Re: [RFCv5 05/13] net: usb: use accessor helper for usbnet private data

["Bjørn Mork" <bjorn@mork.no>]

Oliver Neukum <oneukum@suse.com> writes:

> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index dbf01210b0e7..5522dc137d45 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -43,7 +43,7 @@ enum cdc_mbim_flags {
>  /* using a counter to merge subdriver requests with our own into a combined state */
>  static int cdc_mbim_manage_power(struct usbnet *dev, int on)
>  {
> -	struct cdc_mbim_state *info = (void *)&dev->data;
> +	struct cdc_mbim_state *info = usbnet_priv(dev);
>  	int rv = 0;
>  
>  	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);


I'm not sure I'm following all the conversions, but you are aware that
cdc_mbim and cdc_ncm share a few functions struct cdc_ncm_ctx state, the
and therefore have to cooperate wrt the private data layout?

The 'struct cdc_mbim_state' has a ctx field which is supposed to be
managed by the cdc_ncm part of the driver:

struct cdc_mbim_state {
	struct cdc_ncm_ctx *ctx;
	atomic_t pmcount;
	struct usb_driver *subdriver;
	unsigned long _unused;
	unsigned long flags;
};

The cdc_mbim driver currently depends on cdc_ncm_bind_common(), which is
called from cdc_mbim_bind(), to allocate the 'struct cdc_ncm_ctx' data.
It will later call other functions, like cdc_ncm_fill_tx_frame(), which
depend on that 'struct cdc_ncm_ctx *ctx' field.

I don't understand how you make the patch above work in cooperation with
this:


@@ -1203,7 +1187,7 @@ static struct usb_cdc_ncm_ndp32 *cdc_ncm_ndp32(struct cdc_ncm_ctx *ctx, struct s
 struct sk_buff *
 cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct cdc_ncm_ctx *ctx = usbnet_priv(dev);
 	union {
 		struct usb_cdc_ncm_nth16 *nth16;
 		struct usb_cdc_ncm_nth32 *nth32;


Looks like the cdc_mbim driver will use the same usbnet_priv(dev) as
both 'struct cdc_ncm_ctx' and 'struct cdc_mbim_state'.

Yes, I know this too magic, and should be rewritten on so many levels.
But it is what it is.



Bjørn