Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support

Andre Przywara
2023-06-12T20:11:41Z

On Fri, 9 Jun 2023 16:16:43 -0600
Sam Edwards <cfsworks@gmail.com> wrote:

Hi Sam,

> On 12/5/22 17:45, Andre Przywara wrote:
> > Please let me know if you have any opinions!
>
> I believe I promised you last month I'd let you know once I had a build
> I'm happy with, and I'm pleased to say that I think I've reached that
> point. I'm running quite rapidly out of sharp edges to sand down, too.

Thanks for the update and the list! Can you confirm where you
still needed code changes compared to say my github branch plus the
changes we already discussed? Trying some guesses below, please confirm
or deny:

> I have a build of U-Boot for my target, complete with:
> - UART3 initialized correctly

this is problematic because of the other pinmux used on your board,
which cannot easily be encoded alongside the existing UART3 pinmux?

> - DRAM coming up correctly
> - SPL sets configured boot clock correctly

This should work as per github?

> - SPI-NAND support (SPL and U-Boot proper)

This is with Icenow's series? Any D1 specific changes needed there?

> - MMC support (SPL and U-Boot proper)
> - SPL boot from FEL

again worked already in github?

> - USB gadget support

So with the fixed SUNXI_SRAMC_BASE you said it worked? What about the
USB PHY? That needs at least wiring in the compatible string? If you
have such a patch, can you please rebase it on top of my v2 USB PHY
series and post that?

> - Ethernet MAC+PHY support

Anything surprising here? Is that using an already supported external
PHY?

> - I²C support *
> - GPIO support (LEDs, buttons, misc. board management)

again should work out of the box, minus your board specific
configuration?

> - `reset` working (requries CONFIG_SYSRESET unset, WDT key)

Isn't "CONFIG_SYSRESET unset" a hack? I dimly remember we had this for
some other SoC initially, but later got rid of it?
For the WDT key: it seems like Linux got a nice patch to integrate this
neatly into the driver without quirking this too much, do you have
something ready for U-Boot as well? Would love to see it on the list
then ;-)

> - PSCI, nonsec

ah yeah, owe you some reviews on this one ...

> - Able to boot Linux ;)
>
> * Requires nonzero `MVTWSI_CONTROL_CLEAR_IFLG` for NCAT2, and a patch to
> the pinctrl driver to configure the proper mux function for my necessary
> pins.

Are those pinmuxes straight forward to add to the pinctrl driver? Or
are there conflicts similar to UART3?

> I figured I'd share this list as a sort of checklist for your own work,
> too. The remainder of my efforts now will probably be focused on
> mainlining this stuff (let me know how else I can be of help), and then
> I'm afraid I'll have to disappear back downstream to the Turing Pi 2
> development effort, but maybe our paths will cross again in the kernel
> lists. :)

Yeah, as you may know, the DT has to go through the kernel list. DT
patches can be tedious to upstream, there is now much attention to
every detail. Running checkpatch and dtbs_check should reveal most
issues beforehand, though.

Cheers,
Andre

>
> Thank you greatly,
> Sam
>
> P.S. I figure the reason there aren't I²C function defs in the d1
> pinctrl table already is because Allwinner tends to kick around the I²C
> mux values a lot and we would need a per-pin lookup table that would eat
> up too much valuable image space?
>
> In an entirely JUST FOR FUN exercise to give myself a break from staring
> at datasheets/patches and do a "pure CS" coding challenge for a change,
> I came up with a terse encoding scheme for this table. Here is the size
> (in bits) for a selection of D1's functions (pin assignments harvested
> from Linux):
>
> 'emac': 50,
> 'i2c0': 101,
> 'i2c1': 64,
> 'i2c2': 109,
> 'i2c3': 91,
> 'mmc0': 23,
> 'mmc1': 23,
> 'mmc2': 20,
> 'spi0': 41,
> 'spi1': 48,
> 'uart0': 78,
> 'uart1': 87,
> 'uart2': 88,
> 'uart3': 102,
> 'uart4': 68,
> 'uart5': 66,
>
> ...and yes, it also identifies invalid pin assignments! I'd be willing
> to contribute something like this if there's big interest, but I figure
> needing to compress this at build-time might be a bit too complicated
> for the U-Boot project's liking.