[U-Boot] [PATCH] usb: musb: add support for Blackfin MUSB

[Mike Frysinger]

On Sunday 21 June 2009 15:54:13 Remy Bohmer wrote:
> Hello Mike,
>
> I got a remark about this patch:
>
> 2009/6/16 Mike Frysinger <vapier at gentoo.org>:
> > From: Bryan Wu <bryan.wu at analog.com>
> >
> > Signed-off-by: Bryan Wu <bryan.wu at analog.com>
> > Signed-off-by: Mike Frysinger <vapier at gentoo.org>
> > ---
> >  drivers/usb/musb/Makefile                   |    1 +
> >  drivers/usb/musb/blackfin_usb.c             |  152 +++++++++++++++
> >  drivers/usb/musb/blackfin_usb.h             |   27 +++
> >  drivers/usb/musb/musb_core.h                |   73 +++++++
> >  include/asm-blackfin/mach-common/bits/usb.h |  280
> > +++++++++++++++++++++++++++ include/usb.h                               |
> >    3 +-
> >  6 files changed, 535 insertions(+), 1 deletions(-)
> >
> > --- a/drivers/usb/musb/musb_core.h
> > +++ b/drivers/usb/musb/musb_core.h
> > @@ -42,6 +42,68 @@
> >
> >  /* Mentor USB core register overlay structure */
> >  struct musb_regs {
> > +#if defined(CONFIG_USB_BLACKFIN)
> > +       /* Every register is 32bit aligned, but only 16bits in size */
> > +# define ureg(name) u16 name; u16 __pad_##name;
> > +       /* common registers */
> > +       ureg(faddr)
> > +       ureg(power)
> > +       ureg(intrtx)
> > +       ureg(intrrx)
> > +       ureg(intrtxe)
> > +       ureg(intrrxe)
> > +       ureg(intrusb)
> > +       ureg(intrusbe)
> > +       ureg(frame)
> > +       ureg(index)
> > +       ureg(testmode)
> > +       ureg(globintr)
> > +       ureg(global_ctl)
> > +       u32     reserved0[3];
> > +       /* indexed registers */
> > +       ureg(txmaxp)
> > +       ureg(txcsr)
> > +       ureg(rxmaxp)
> > +       ureg(rxcsr)
> > +       ureg(rxcount)
> > +       ureg(txtype)
> > +       ureg(txinterval)
> > +       ureg(rxtype)
> > +       ureg(rxinterval)
> > +       u32     reserved1;
> > +       ureg(txcount)
> > +       u32     reserved2[5];
> > +       /* fifo */
> > +       u16     fifox[32];
> > +       /* OTG, dynamic FIFO, version & vendor registers */
> > +       u32     reserved3[16];
> > +       ureg(devctl)
> > +       ureg(vbus_irq)
> > +       ureg(vbus_mask)
> > +       u32 reserved4[15];
> > +       ureg(linkinfo)
> > +       ureg(vplen)
> > +       ureg(hseof1)
> > +       ureg(fseof1)
> > +       ureg(lseof1)
> > +       u32 reserved5[41];
> > +       /* target address registers */
> > +       struct musb_tar_regs {
> > +               ureg(txmaxp)
> > +               ureg(txcsr)
> > +               ureg(rxmaxp)
> > +               ureg(rxcsr)
> > +               ureg(rxcount)
> > +               ureg(txtype)
> > +               ureg(txinternal)
> > +               ureg(rxtype)
> > +               ureg(rxinternal)
> > +               u32     reserved6;
> > +               ureg(txcount)
> > +               u32 reserved7[5];
> > +       } tar[8];
> > +# undef ureg
> > +#else
> >        /* common registers */
> >        u8      faddr;
> >        u8      power;
> > @@ -97,6 +159,7 @@ struct musb_regs {
> >                u8      rxhubaddr;
> >                u8      rxhubport;
> >        } tar[16];
> > +#endif
>
> Can both branches of this ifdef be merged somehow, such that the ifdef
> is much smaller?
>
> Both branches seem to contain the same elements, only type differs (
> which could maybe be handled with ureg() constructions also)

the layout isnt exactly the same, the sizes arent uniform enough for ureg(), 
and the registers available dont match either.  we'd still have ifdefs, but 
interleaved, and i think that'd be much more likely to lead to breakage for 
architectures invovled.

> >  } __attribute__((aligned(32)));
>
> Shouldn't this be '__attribute__((packed))' too?

i didnt add that, but adding packed would probably make sense
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090621/14e8dc34/attachment-0001.pgp