
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