From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V3 01/20] Add functions for use with i.mx6 otg udc
Date: Sun, 4 Aug 2013 00:46:08 +0200 [thread overview]
Message-ID: <201308040046.08246.marex@denx.de> (raw)
In-Reply-To: <51FD5A61.6020603@boundarydevices.com>
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
next prev parent reply other threads:[~2013-08-03 22:46 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
2013-08-03 22:46 ` Marek Vasut [this message]
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=201308040046.08246.marex@denx.de \
--to=marex@denx.de \
--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