[U-Boot] [PATCH 2/2] mx5: Use default pin initializers

[Benoît Thébaudeau]

Dear Matt Sealey,

> On Thu, Aug 16, 2012 at 3:09 PM, Benoît Thébaudeau
> <benoit.thebaudeau at advansee.com> wrote:
> > Dear Matt Sealey,
> >
> >> I'm gonna NACK this because most of these pin settings are
> >> actually
> >> the POR defaults anyway (UART1 is
> >> the example I can easily pick out).
> >>
> >> .. unless someone's setting them up wrong between warn reboots, or
> >> changing a UART into something else
> >> later in boot.. which is unfathomable.. we don't need to even
> >> touch
> >> the iomux controller to get a working UART
> >> on MX51, Efika MX or anything.
> >>
> >> Why bother touching them?
> >
> > I agree, but the purpose of this patch is not to add this code,
> > only to move it
> > to a common place... Why didn't you NAK the addition of these
> > boards because of
> > that?
> 
> Why bother changing it in the first place if it's already doing the
> same thing? :)

To improve code quality... And because I have local boards that also needed the
same functions, so it avoided more duplications.

> I have a MAJOR nit that is going to ruin your day; this is from
> mx51_uart1_init_pins, just one line:
> 
>  mxc_request_iomux(MX51_PIN_UART1_RXD, IOMUX_CONFIG_ALT0);
> 
> Why are you using the old mxc_request_iomux model, when MX6 and
> Sabrelite for example uses the newer iomux_v3_configure_pad model?

Simply because the current mx51 pin definitions are made for this API.

> Surely someone should be thinking of putting that into place for MX51
> and MX53 first so these pin init things can be done on the newer one.
> I already ported 2012.07 for Efika MX and have an include (Troy
> Kisky's got a copy of it too, we sent it there hoping for a little
> review but he doesn't have our board to test IIRC) and a working
> tree.
> 
> What's stopping us submitting this already? Churn. Also, we thought
> we
> could get away with U-Boot doing the grunt work of configuring pins
> where needed, and the Linux DT guys vetoed it in favor of needlessly
> reconfiguring the pins U-Boot already set up by redefining them for
> reconfiguration in the DT anyway - I would have hoped we could avoid
> that, and as such our U-Boot tree internally does a LOT of iomux
> setup
> that U-Boot doesn't even drive (audio clock enables, amplifier,
> i2c..)
> on our board. I need to cut that down a little bit.
> 
> > This kind of code is present in all i.MX board files. If it is
> > useful, it's
> > better to have it merged rather than duplicated.
> 
> See above :)
> 
> For i.MX we have two iomux models and function sets available for
> doing identical stuff. Isn't that kind of wasteful?

There's probably room for improvement on iomux, but we have to start cleanup
with something. If you want, you can submit patches and rebase after mine. That
would avoid you to duplicate your patch code for all boards. These are two
different topics, and my change comes logically first.

> > If you NAK this, following your rationale, it means that you prefer
> > to have
> > useless duplicated code everywhere rather than in a single
> > location.
> 
> I'd prefer to have useless code removed, rather than just moved.

It's not useless. See below. :)

> > doesn't make any sense. Or are you suggesting to remove this code
> > from all
> > boards instead? Was there a good reason for this code to be there
> > in the first
> > place?
> 
> No, there was never a good reason for it. It's there because whoever
> wrote the very, very original code in the first place (Pegatron)
> didn't read the docs properly and just set everything up regardless
> of
> whether it was already set up, then whoever put the Efika MX support
> in mainline didn't read the docs either and just reformatted what was
> there in our source release and/or read the schematics without fact
> checking.
> 
> It's a hideous legacy being perpetuated here. I am fully in favor of
> you leaving Efika MX out to languish in the deep dark recesses of
> boards that might compile but are basically doing weird stuff, and
> nobody uses anyway, until we work out the situation to actually
> support the board 100% in the most efficient way possible.
> 
> I'll throw a patchset out to add the iomux-mx51.h file (with all the
> pins Efika MX needs at least. I had a little chat with Troy at BD and
> he suggested people should add pins per board where needed and not
> just copy the entire set from the Linux code if possible) sometime
> end
> of week or next week, and maybe we can revisit these patches. I've
> been concentrating on the Linux side and didn't think this new driver
> model/code cleanup nightmare would happen at the same time as the
> Linux board removal/wholesale DT switch.
> 
> Marek; I realise it's just an opinion, which I feel as the maintainer
> of this platform (not for U-Boot in terms of custodian status, but in
> terms of who at Genesi has the responsibility to keep track of this
> stuff) I should express and I feel it would be remiss of me if I did
> not. Who's the custodian of the Efika MX stuff anyway? Does it just
> default to Stefano, or is it you as the committer?

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=MAINTAINERS;h=c5a6f2f297353b5883f751af9780de440bc3b4d7;hb=HEAD
is here to tell you just that.

> As for problems during reboots, well, unsupported configurations are
> unsupported configurations. If your kernel muxes out DIFFERENT pins
> to
> U-Boot, expect U-Boot to burn the board. As it stands, the old
> "linux-legacy" kernels don't make too heavy a set of changes (maybe
> just drive strength and hysteresis stuff that is totally pointless).
> If you boot U-Boot from POR and then boot a DT linux kernel, it'll
> all
> be exactly the same on every reboot. Boot an old kernel and it'll
> still work. You can't be too careful here, and by that I mean, you
> can't keep reconfiguring the chip just because you think there may be
> the possibility of some idiot doing something wrong. This isn't
> something that is burned into the chip and configured as such on boot
> without intervention, after all, it's all going to revert to power on
> defaults the moment you yank the power. At that point, if you're
> running a new U-Boot, and any Linux kernel published in the last few
> years for Efika MX, nothing is going to explode.

As a general rule, you should not rely on the expected post-reset state. You
really can't know for sure what may have happened before your code starts. Weird
things could have occurred, e.g. you may have launched your code from a JTAG
probe that put your binary right into RAM instead of using normal reset + ROM
bootloader.

Best regards,
Benoît