RE: [PATCH 1/2] usbnet: changes for upcoming cdc_ncm driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

> > -		skb_queue_tail (&dev->done, skb);
> > +	if (skb->len) {
> > +		/* all data was already cloned inside NCM driver */
> 
> Fix this comment.  NCM isn't the only framing policy which un-batches
> RX packets ... RNDIS has done so for a number of years already, and
> more recently EEM needs it too ... plus at least one hardware driver.

Will do.

> Also, check pending patches, since I seem to recall one that supports
> some hardware (SMSC?) that batches, and needed to update the calling
> convention you're using here (i.e. the original one).

Patch was based on linux-next git and I don't see any difference from
net-next. Any particular git you'd like me to look at pending patches?

> 
> 
> > +		if (dev->driver_info->flags & FLAG_MULTI_PACKET)
> 
> except ... you documented this flag as affecting TX paths not RX...

This flag carries two purposes:
- help to correct statistic (Tx/Rx counting)
- handle NCM feature to avoid sending short packets in certain
conditions: cdc_ncm driver has to send short packet (or ZLP) only
if data size is not equal to negotiated data size (dwNtbOutMaxSize).

The other option (instead of dedicated flag) would be to add
additional field in the usbnet context structure, which will keep
dwNtbOutMaxSize value (it still would be better to use
dedicated flag in addition to the value). Thus usbnet would possess
the knowledge when ZLP or short packet should be skipped.
By doing this approach we will eliminate a need to generate short
packet inside the driver and use generic code in usbnet.

Could you comment, what would be the preferable way to get it in the
next version of this patch?

Regards,
Alexey

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux