public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Troy Kisky <troy.kisky@boundarydevices.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V3 01/20] Add functions for use with i.mx6 otg udc
Date: Sat, 03 Aug 2013 12:30:41 -0700	[thread overview]
Message-ID: <51FD5A61.6020603@boundarydevices.com> (raw)
In-Reply-To: <201308031847.26751.marex@denx.de>

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

  reply	other threads:[~2013-08-03 19:30 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-01 23:27 [U-Boot] [PATCH V3 00/20] Make mv_udc work for i.mx6 Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 01/20] Add functions for use with i.mx6 otg udc Troy Kisky
2013-08-02 10:48   ` Marek Vasut
2013-08-02 19:44     ` Troy Kisky
2013-08-02 22:10       ` Marek Vasut
2013-08-02 23:41         ` Troy Kisky
2013-08-03  1:45           ` Marek Vasut
2013-08-03  2:24             ` Troy Kisky
2013-08-03 16:47               ` Marek Vasut
2013-08-03 19:30                 ` Troy Kisky [this message]
2013-08-03 22:46                   ` Marek Vasut
2013-08-15 21:36                   ` Marek Vasut
2013-08-16 18:20                     ` Troy Kisky
2013-08-04 20:15   ` Wolfgang Denk
2013-08-05 20:15     ` Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 02/20] mx6: iomux: add GPR1 defines Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 03/20] nitrogen6x: add otg usb ethernet gadget support Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 04/20] nitrogen6x: add CONFIG_MV_UDC Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 05/20] arch-mxs/sys_proto.h: include regs-common.h Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 06/20] usb: gadget: mv_udc: fix hardware udc address for i.MX6 Troy Kisky
2013-08-02 10:53   ` Marek Vasut
2013-08-01 23:27 ` [U-Boot] [PATCH V3 07/20] usb: gadget: config: fix unaligned access issues Troy Kisky
2013-08-02 10:57   ` Marek Vasut
2013-08-01 23:27 ` [U-Boot] [PATCH V3 08/20] usb: gadget: mv_udc: add MX6Q specific reset Troy Kisky
2013-08-02 10:58   ` Marek Vasut
2013-08-01 23:27 ` [U-Boot] [PATCH V3 09/20] usb: gadget: ether set wMaxPacketSize Troy Kisky
2013-08-02 10:59   ` Marek Vasut
2013-08-01 23:27 ` [U-Boot] [PATCH V3 10/20] usb: gadget: ether: return error from rx_submit if no request Troy Kisky
2013-08-02 11:07   ` Marek Vasut
2013-08-01 23:27 ` [U-Boot] [PATCH V3 11/20] usb: gadget: mv_udc: split mv_udc.h file Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 12/20] usb: udc: add udc.h include file Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 13/20] usb: gadget: mv_udc: fix typo in error message Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 14/20] usb: gadget: mv_udc: set is_dualspeed = 1 Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 15/20] usb: gadget: mv_udc: fix full speed connections Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 16/20] usb: gadget: mv_udc: optimize bounce Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 17/20] usb: gadget: mv_udc: flush item before head Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 18/20] usb: gadget: mv_udc: optimize ep_enable Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 19/20] usb: gadget: mv_udc: zero transfer descriptor memory on probe Troy Kisky
2013-08-01 23:27 ` [U-Boot] [PATCH V3 20/20] usb: gadget: mv_udc: clear desc upon ep_disable Troy Kisky
2013-08-02 11:14 ` [U-Boot] [PATCH V3 00/20] Make mv_udc work for i.mx6 Marek Vasut
2013-08-02 14:43 ` Stefano Babic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51FD5A61.6020603@boundarydevices.com \
    --to=troy.kisky@boundarydevices.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox