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

[Andre Przywara]

On Tue, 20 Jun 2023 16:11:48 -0600
Sam Edwards <cfsworks at gmail.com> wrote:

Hi Sam,

pleasure to write with you ;-)

> On 6/20/23 06:42, Andre Przywara wrote:
> > So yeah, the request of a "Enter FEL" command came up multiple times, but
> > so far no one could be bothered to implement this properly. The idea would
> > be to have a generic command (more like "fel-reset" than efex), and
> > allow each SoC (family) to implement this differently, as every
> > SoC requires something a bit different here (32-bit vs. 64-bit, having an
> > RTC vs not, etc).
> > 
> > If you could post your solution somewhere, we could start this effort.
> > There was some patch for the H3 already, and it's relatively
> > straight-forward on the newer SoCs (H616, IIRC), so if at least two
> > popular, but different SoCs would be supported, we could make sure to have
> > the right abstractions in place.  
> 
> I already have a "go_to_fel()" that does the right thing to enter FEL 
> from the SPL; I would pretty much just need to introduce the following 
> per-SoC(-family) functions:
> - bool sunxi_fel_flag_test(void)
> - void sunxi_fel_flag_clear(void)
> - void sunxi_fel_flag_set(void)

Well, so this is actually the fallback implementation which should
somewhat work on most SoCs: set a flag, reset, and catch the flag in
the SPL. For modern SoCs with CPU hotplug support (the H616 is one one
of those, and it looks like the T113s is as well), there is actually a more
direct route:
We put some magic and the FEL entry address into some special memory
locations, then just reset. Now the *BootROM* will do the check already,
and branch to the provided entry point, which would be the FEL routine.
This doesn't rely on a prepared SPL to be loaded, so works without a
boot device with mainline U-Boot around.
Refer to section 3.4.2.3 and 3.4.2.4 of the T113-S3 user manual (v1.1).
According to this, the magic would be 0xfa50392f, the magic's address is
0x070005C0, and CPU0's entry point address would be in 0x070005C4. I had a
proof of concept implementation for the H616 using this method. The only
problem left would be that someone needs to clean the magic afterwards,
otherwise any follow-up reset would trigger FEL mode again.

> The "fel-reset" command (which is easier to type than what I have, "run 
> fel_do_enter") would then call sunxi_fel_flag_set() and initiate a 
> reset, and the SPL's early init just has to do sunxi_fel_flag_test() -> 
> sunxi_fel_flag_clear() -> go_to_fel(). Seems easy enough.
> 
> Could you recommend to me a sufficiently different chip to test my 
> abstractions against? Something ARMv8 and *without* RTC?

I think all ARMv8 parts have an RTC, so your generic approach might work
there as well. The complication is that the SPL switches to AArch64 very
early, in hand-stitched AArch32 assembly, check out
arch/arm/include/asm/arch-sunxi/boot0.h.
The check would need to be coded like this, then.

> I can then send 
> in a series adding FEL support for that. (Also, did that H3 patch 
> actually land? I didn't see anything but want to know if I should be 
> refactoring my approach to extend what that H3 patch does or not.)

https://patchwork.ozlabs.org/project/uboot/patch/c211aa414c59e7275fef82bf6f0035276d6e29f3.1656875482.git.msuchanek@suse.de/

Another generic approach for ARMv7 parts would be to reset the peripherals
as much as possible, then configure the core in a BROM compatible way
(MMU off, etc) and just branch to the BROM FEL entry address. This idea is
already somewhat used in our return-to-FEL code in the SPL, although we
don't change too much of the core setup in the SPL.

> > Ah, depending on the BSP kernel is indeed quite bad. I wonder what
> > features of the kernel you rely on that upstream does not have? Or is it
> > more about the BMC userland parts that are married to the Allwinner kernel
> > and its own interfaces?  
> 
> I don't fully know; getting the kernel back on mainline is, as I said, a 
> push for another day. I'm very much making a point of not looking into 
> it before the bootloader can be upgraded to something that isn't a 
> crashy, hard-to-update, failure-prone mess. (I'm working in "biggest 
> fire, first out" order.)

Fair enough, from a mainlining point of view you need to funnel the
board .dts through the Linux tree first, though.
Also, with the right DT, a mainline kernel would run on the board
already, maybe just not with the full functionality you'd expect from
it.

What I mean to say: you can surely continue using Tina Linux for the
BMC functionality on the board right now, but still upstream the board
.dts. As mentioned, the DT just describes the hardware, so it doesn't
dictate what to do with it. One might abuse the board as a T113s dev
board, maybe ;-) Does it work without any of the modules populated?

> That said, the first such dependent feature that leaps to mind is the 
> AWNAND driver's CONFIG_AW_SPINAND_SIMULATE_MULTIPLANE, which logically 
> interleaves pages of the NAND in a different ordering vs. what the 
> physical NAND (and mainline's spi-nand driver) does. Alas this is a 
> feature we're dependent on not because it provides benefits to our users 
> (it does not, and in downstream discussions I've been soapboxing about 
> how it's likely wearing down people's NANDs) but because the boards are 
> flashed at the factory with this flag enabled so we need it set for the 
> NAND to be accessible. We've experimented with reflashing the board with 
> that flag disabled, but that has so far only resulted in corrupted flash.
> 
> Hope is not lost, though, for I have a half-written tool which shows 
> some promise in being able to "unscramble the egg" and migrate existing 
> NANDs over to the correct layout. That should be sufficient to get 
> mainline U-Boot (and Tina Linux with the flag disabled) working, but I 
> have no idea about mainline Linux still: this would only peel back one 
> layer of the onion, and I don't know whether the next obstacle will be 
> easier, harder, or about the same difficulty.
> 
> But it does mean that, for now, we're stuck with Tina Linux.
> 
> > Final DT is a noble goal, but in reality there will always be room for
> > improvement and additions. So what we typically do is to start with a
> > simple .dts for the kernel tree, describing the basic peripherals, and
> > everything that already works and is not subject to debate. If in doubt,
> > include a node, and we will comment. Could you prepare such a patch?  
> 
> The peripheral-describing .dts that I have is for Tina Linux, and uses 
> incompatible compatibles (ha), includes, dt-bindings, temporary hacks 
> while better driver support can be developed, and would otherwise not 
> fly upstream.

Sure, you keep that nasty piece downstream, and load it in U-Boot into
$fdt_addr_r, then take it from there.
That doesn't affect the mainline Linux and U-Boot DT, though, which
could be upstreamed independently.
Then you can use the mainline DT (as used by U-Boot), using
$fdtcontroladdr, or any loaded DT, via $fdt_addr_r.

> I can send it in *anyway* if for some reason you think 
> that's a good idea, but I really don't see that as being anything other 
> than a waste of time.

There is indeed no point in sending a DT which only works with the
Allwinner BSP kernel.

> As well, I can't write a fresh .dts for mainline (one more likely to be 
> accepted on the list).

Yes, please!

> A mainline kernel has never been booted on this 
> board, so I would do no better at this than a kernel contributor 
> selected at random.

... having a board. So far you are the one contributor with access to
the hardware, so: thanks for volunteering! ;-)

> The best I can do now is write something that *looks* like the correct .dts.

Yes, and that would be the right .dts, if it passes the kernel review.

> As I keep saying, that may change in the future. But the answer today is 
> still "no, I cannot."

So to summarise what I am trying to say: You create a simple .dts file,
basically #include "sun8i-t113s.dtsi" and declaring the UART. Then add the
devices that already work (Ethernet?, USB-OTG) and see how far you get.
You could just load a kernel and initrd via FEL, and use the mainline
kernel just via serial like this. Granted, this is not really useful in
the BMC context, but would be a start.

This is actually similar to what Chris just did [1] for the RG Nano: this
initial DT for this mini handheld gaming console has no display support, so
is pretty useless in its original context, but starting mainline support
is the important thing here.

Plus: this exposes the problems you face (PHY config?) to a wider range of
people, who can help with the solution.

[1]
https://lore.kernel.org/linux-sunxi/20230620200022.295674-1-macroalpha82@gmail.com/T
> 
> > This
> > should not contradict any DT nodes that U-Boot uses, so it's not a double
> > effort.  
> 
> True, in theory it *shouldn't* but in practice, I've found it does.
> 
> One way I've been bitten is that the sunxi SPI driver in U-Boot doesn't 
> support Quad-SPI, so if the DT says the SPI-NAND is connected with a bus 
> width of 4, the SPI-NAND driver requests Quad-SPI transfers, but the SPI 
> master driver has no idea that it needs to handle the transfer any 
> differently, and we're left with corrupt NAND reads/writes. Without 
> Quad-SPI support in U-Boot's master driver (and/or, better yet, a U-Boot 
> equivalent to Linux commit 83596fbeb5) -- also a push for another day -- 
> I have no choice but to give U-Boot a specially edited version of the DT 
> that omits this property.

The U-Boot build system support some kind of build time DT "overlay"
feature: You put a file with the same name, but ending in
"-u-boot.dtsi" in the arch/arm/dts directory, and it will be included
into the DT which gets embedded into the U-Boot image. See
arch/arm/dts/sun50i-a64-sopine-baseboard-u-boot.dts for an
example, and doc/develop/devicetree/control.rst for the proper
documentation.
So we upstream a minimal, non-controversial and non-contradicting base
.dts into the kernel tree, and can fix things up for the time being
using this method. This hack can then go away if either the mainline
kernel DT gets fixed and/or U-Boot learns the quad-SPI trick.

> > This would mean we have a *second* board DT for the T113s SoC in the
> > kernel, which always helps to improve quality and prevents hacks that just
> > work on the MangoPi. Besides, the TuringPi board is an actually useful
> > application of the SoC, deployed and available, in contrast to just some
> > development board from Chinese websites.
> > And once this is merged, we could just copy this over to U-Boot and add
> > the defconfig and any other support patches there.  
> 
> See below.
> 
> >> ...but for now it's very much meant to be out-of-tree. :)
> >>
> >> (I also do not work for the company that produced this board -- I'm just  
> > 
> > Ah, that would have been a first anyway ;-)  
> 
> Oh? What would have been a first? I could pass it along to my contact at

Someone from the board vendor company actually actively adding upstream
support for their device early. There were some examples in the past
where employees participated in upstreaming, but I cannot remember
seeing this too often when it comes to the initial DT support.

> this company and encourage him to get involved in some way. I'm sure 
> they'd appreciate the opportunity for the good press associated with 
> being the first at something in the F/OSS world, and it might help to 
> get them in the habit of cooperating closely with upstream (to make it 
> less likely that they just fork things the moment upstream doesn't solve 
> some problem they're having).

Yes, I understand the pressure in a product-centric world with release
dates and timelines, but the advantage of vendor-backed contributions
are early access to hardware and documentation, plus access to the
hardware engineers.

> > Yeah, I understand it's not the most grateful job to chase up on doing
> > things properly and stay on with the upstreaming process. Ultimately it's
> > the right thing to do, though, and will save you hassle over time. Plus we
> > (the community) will help you with that, and you'd get a second commit in
> > the kernel ;-)  
> 
> Ideologically-speaking, this is music to my ears, and I think we would 
> even be having this same discussion were our roles reversed: we do both 
> agree fully on the (mutual) benefits of upstream contribution.
> 
> But even more ultimately: the available time on any given day is 
> limited, and I have to choose my battles. There are often things that 

Fair enough, and there is no real pressure in getting the mainline DT
fully functional and merged today. But we should start the process *now*,
as this helps to detect problems early, and allows other people to jump on
this and continue the work or help out.

Cheers,
Andre


> either require less effort, save an even greater hassle over time, or 
> provide more urgently-needed benefits, which (pragmatically speaking) 
> ought to take priority. That doesn't mean the other lower-priority items 
> have no benefit, it just means they should not be done *now.*
> 
> > Cheers,
> > Andre  
> 
> Likewise,
> Sam