[U-Boot] [PATCH V3 01/20] Add functions for use with i.mx6 otg udc

Marek Vasut
2013-08-03T22:46:08Z

Dear Troy Kisky,

> On 8/3/2013 9:47 AM, Marek Vasut wrote:
> > Dear Troy Kisky,
> >
> > [...]
> >
> >>>> Why is there a union ? It looks to me like you just want to access the
> >>>> same variable
> >>>> with 2 naming strategies.
> >>>
> >>> That is correct. I can either pass it further into functions as the
> >>> struct mxs_register_32 name_reg or I can directly access it as
> >>> name_set/_clr/_tog . Works just fine.
> >>
> >> I never said it didn't work, obviously it does.
> >>
> >>> Best regards,
> >>> Marek Vasut
> >>
> >> There may be code that you can point at that would make this useful, but
> >> I have
> >> a hard time envisioning it. The code I added, I know doesn't need a
> >> union, and I bet most
> >> of the other variable accesses don't need a union. That's why I asked if
> >> you'd like
> >> me to attempt to clean it up (always access thru struct, ie replace
> >> name_set with name.set).
> >
> > No, I want to keep this as-is. Especially because the MX28 has the
> > registers named exactly by this scheme.
>
> Exactly matching documentation is a good advantage.
>
> >> I don't want to change the code I added to use this.
> >
> > Please do, I do not want a duplicit implementation of these register
> > structures in the tree.
> >
> >> I can see a small advantage in consistency with the mx28.
> >
> > THe MX6 uses many IP blocks from MX28 -- APBH DMA, NAND, USB -- to name a
> > few. Keeping mx28 and mx6 aligned is more than helpful.
> >
> > Best regards,
> > Marek Vasut
>
> Marek, I really appreciate your willingness to explain your reasoning.
> Though I wouldn't do this
> without prompting, perhaps my dislike of unions is a bit irrational. But
> since there are also many
> other places where this change could be made (grep _tog in
> arch-mx6/crm_regs.h, imx-regs.h,)

This clearly means that someone didn't look around before coding this stuff.
Brief look indicates the OCOTP is taken from MX28 too and so is ANATOP.

> I'd like Stefano to say he is OK with using mxs_reg_32 and doesn't share
> my opinion and doesn't
> want to rename it to something else. After all, you're talking about
> removing a structure with only
> 4 members, not a great amount of duplication.

I think I lost you here, sorry. To put down what I am talking about:

- Use what already is in imx-common/regs-common.h
- imx-common/regs-common.h does not need change

I did not yet hear any reasonable argument to change the imx-common/regs-
common.h file. Unions being ugly in your opinion is not a valid argument.

Note that MX28 was in the tree much earlier than MX6 and if MX6 did reinvent the
wheel, it should be fixed before this duplication spreads. Especially so the MX6
and MX28 do not diverge and can share much of the code.

Best regards,
Marek Vasut