Thread (7 messages) 7 messages, 3 authors, 2017-01-27

Re: [PATCH 1/3] net: bgmac: allocate struct bgmac just once & don't copy it

From: David Miller <davem@davemloft.net>
Date: 2017-01-27 16:26:46

From: Felix Fietkau <nbd@nbd.name>
Date: Fri, 27 Jan 2017 17:02:33 +0100
On 2017-01-27 10:20, Rafał Miłecki wrote:
quoted
From: Rafał Miłecki <rafal@milecki.pl>

To share as much code as possible in bgmac we call alloc_etherdev from
bgmac.c which is used by both: platform and bcma code. The easiest
solution was to use it for allocating whole struct bgmac but it doesn't
work well as we already get early-filled struct bgmac as an argument.

So far we were solving this by copying received struct into newly
allocated one. The problem is it means storing 2 allocated structs,
using only 1 of them and non-shared code not having access to it.

This patch solves it by using alloc_etherdev to allocate *pointer* for
the already allocated struct. The only downside of this is we have to be
careful when using netdev_priv.

Another solution was to call alloc_etherdev in platform/bcma specific
code but Jon advised against it due to sharing less code that way.
How does that lead to sharing less code?
I find this pointer indirection rather ugly and uncommon, and I think it
would be much cleaner to split the probe into bgmac_enet_alloc and
bgmac_enet_probe (with bgmac_enet_alloc calling alloc_etherdev and doing
basic setup).
I agree, it would be so much better if bgmac_probe() and friends
initialized a real bgmac object which was the private of a netdev
struct, then passed that down into bgmac_enet_probe().
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help