On Thu, Apr 08, 2010 at 11:11:57AM +0300, Yauheni Kaliuta wrote:
>
> Hi!
>
> >>>>> "eGK" == ext Greg KH writes:
>
> > On Wed, Apr 07, 2010 at 08:40:54PM +0300, yauheni.kaliuta@xxxxxxxxx wrote:
> > > From: Yauheni Kaliuta <yauheni.kaliuta@xxxxxxxxx>
> > >
> > > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@xxxxxxxxx>
> > > ---
> > > include/linux/usb/ncm.h | 103 +++++++++++++++++++++++++++++++++++++++++++++++
>
> > Who is going to need this file? Both the gadget and device drivers?
>
> yes. Actually, the parser code looks similar, and I could even try to make
> a common API, but do not see nice place, where to put such a shared C file.
Ok, that's fine.
> > > 1 files changed, 103 insertions(+), 0 deletions(-)
> > > create mode 100644 include/linux/usb/ncm.h
> > >
> > > diff --git a/include/linux/usb/ncm.h b/include/linux/usb/ncm.h
> > > new file mode 100644
> > > index 0000000..4ff533d
> > > --- /dev/null
> > > +++ b/include/linux/usb/ncm.h
> > > @@ -0,0 +1,103 @@
> > > +/*
> > > + * USB CDC NCM auxiliary definitions
> > > + */
> > > +
> > > +#ifndef __LINUX_USB_NCM_H
> > > +#define __LINUX_USB_NCM_H
> > > +
> > > +#include <linux/types.h>
> > > +#include <linux/usb/cdc.h>
> > > +#include <asm/unaligned.h>
> > > +
> > > +#define NCM_NTB_MIN_IN_SIZE 2048
> > > +#define NCM_NTB_MIN_OUT_SIZE 2048
> > > +
> > > +/* bmNetworkCapabilities */
> > > +
> > > +#define NCM_NCAP_ETH_FILTER (1 << 0)
> > > +#define NCM_NCAP_NET_ADDRESS (1 << 1)
> > > +#define NCM_NCAP_ENCAP_COMM (1 << 2)
> > > +#define NCM_NCAP_MAX_DGRAM (1 << 3)
> > > +#define NCM_NCAP_CRC_MODE (1 << 4)
> > > +
> > > +struct ndp_parser_opts {
> > > + uint32_t nth_sign;
> > > + uint32_t ndp_sign;
>
> > u32 please.
>
> ok
>
> > > + unsigned nth_size;
> > > + unsigned ndp_size;
> > > + unsigned ndplen_align;
> > > + /* sizes in u16 units */
> > > + unsigned dgram_item_len; /* index or length */
> > > + unsigned block_length;
> > > + unsigned fp_index;
> > > + unsigned reserved1;
> > > + unsigned reserved2;
>
> > reserved for what?
>
> struct usb_cdc_ncm_ndp16 {
> __le32 dwSignature;
> __le16 wLength;
> __le16 wNextFpIndex;
> __u8 data[0];
> } __attribute__ ((packed));
>
> struct usb_cdc_ncm_ndp32 {
> __le32 dwSignature;
> __le16 wLength;
> __le16 wReserved6;
> __le32 dwNextFpIndex;
> __le32 dwReserved12;
> __u8 data[0];
> } __attribute__ ((packed));
>
> reserved by spec, 0 in ndp16 case.
But why not use the structure itself here? And how do you know that
'unsigned' will be the right size? Please use specific variable sizes
if you are relying on them to be a specific size.
> > > + unsigned next_fp_index;
>
> > Care to be specific about these sizes as well?
>
> NextFpIndex size is different for ndp16/32 as well. I'll put a comment.
Why is it different? Why can't it be the same?
> > > +};
> > > +
> > > +#define INIT_NDP16_OPTS { \
> > > + .nth_sign = NCM_NTH16_SIGN, \
> > > + .ndp_sign = NCM_NDP16_NOCRC_SIGN, \
> > > + .nth_size = sizeof(struct usb_cdc_ncm_nth16), \
> > > + .ndp_size = sizeof(struct usb_cdc_ncm_ndp16), \
> > > + .ndplen_align = 4, \
> > > + .dgram_item_len = 1, \
> > > + .block_length = 1, \
> > > + .fp_index = 1, \
> > > + .reserved1 = 0, \
> > > + .reserved2 = 0, \
> > > + .next_fp_index = 1, \
> > > + }
> > > +
> > > +
> > > +#define INIT_NDP32_OPTS { \
> > > + .nth_sign = NCM_NTH32_SIGN, \
> > > + .ndp_sign = NCM_NDP32_NOCRC_SIGN, \
> > > + .nth_size = sizeof(struct usb_cdc_ncm_nth32), \
> > > + .ndp_size = sizeof(struct usb_cdc_ncm_ndp32), \
> > > + .ndplen_align = 8, \
> > > + .dgram_item_len = 2, \
> > > + .block_length = 2, \
> > > + .fp_index = 2, \
> > > + .reserved1 = 1, \
> > > + .reserved2 = 2, \
> > > + .next_fp_index = 2, \
> > > + }
> > > +
> > > +static inline void put_ncm(__le16 **p, unsigned size, unsigned val)
> > > +{
> > > + switch (size) {
> > > + case 1:
> > > + put_unaligned_le16((uint16_t)val, *p);
> > > + break;
> > > + case 2:
> > > + put_unaligned_le32((uint32_t)val, *p);
> > > +
> > > + break;
> > > + default:
> > > + BUG();
> > > + }
> > > +
> > > + *p += size;
> > > +}
>
> > Don't we have functions for this type of thing already?
>
> Thanks a lot, but I could not find it. Could you point?
I have not looked that hard, but it should be somewhere.
thanks,
greg k-h
--
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
