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

Troy Kisky
2013-08-03T19:30:41Z

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,)
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.

Troy