On Mon, 12 Jun 2023 15:18:17 -0600
Sam Edwards <cfswo...@gmail.com> wrote:
Hi Sam,
something regarding "reset" below ...
> On 6/11/23 18:20, Andre Przywara wrote:
> > 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:
>
> Preeeeetttttyyy much everything I've changed locally has been submitted
> to the list or discussed in the relevant patchset. Have you updated your
> GitHub branch recently (past couple of weeks)? I haven't been watching
> it but I can pull it again and see which of my local changes are still
> required.
>
> >> 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?
>
> Actually no, my board's UART3 is on PB6/PB7, nice and standard.
>
> >> - DRAM coming up correctly
> >> - SPL sets configured boot clock correctly
> >
> > This should work as per github?
>
> Yep, everything was working satisfactorily once I figured out I needed
> to set CONFIG_SYS_CLK_FREQ to get acceptable boot speeds.
>
> >> - SPI-NAND support (SPL and U-Boot proper)
> >
> > This is with Icenow's series? Any D1 specific changes needed there?
>
> Yes, with Icenowy's series (322733).
>
> I learned that the BROM sets the boot medium code to 0x04 when it's an
> SPI-*NAND* (while older chips used 0x03 for "it's either SPI-NOR or
> SPI-NAND, good luck figuring out which"). Since `env_get_location`
> assumes BOOT_DEVICE_SPI is NOR (and my target needs to load env from UBI
> iff booting from NAND), I'm hoping I can convince Icenowy to separate
> out the SPI-NAND and SPI-NOR load methods entirely (vs. her current
> try-NAND-then-NOR approach) with the aid of some disambiguation logic to
> probe for an SPI-NAND on the older chips known to report these as 0x03.
>
> I also needed Maksim's patch series (355747) to support the D1 SPI master.
>
> >> - MMC support (SPL and U-Boot proper)
> >> - SPL boot from FEL
> >
> > again worked already in github?
>
> Yes and yes. I was just confirming they're good; no local work needed
> from me here.
>
> >> - 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?
>
> Yes, with corrected SUNXI_SRAMC_BASE -- though I also needed my patches
> to make musb-new/sunxi.c use the UDC gadget model in DM (series 358842),
> as I don't think there's another way to init the controller at runtime.
>
> I can't say whether the endpoint limit is correct or that mUSB *host*
> operation works.
>
> The USB PHY only required that CONFIG_PHY_SUN4I_USB be enabled. The
> correct compatible is already wired up. It does look like your PHY
> series drops the explicit requirement to set PHY_SUN4I_USB so that's
> better than what I was doing (adding a `select` directive under R528).
>
> I can test your series if you want but I doubt it intersects my work
> here in any significant way.
>
> >> - Ethernet MAC+PHY support
> >
> > Anything surprising here? Is that using an already supported external
> > PHY?
>
> The only surprise was this was how I noticed that CONFIG_CLK_SUN20I_D1
> was not being implicitly enabled. Enabling that was then how I found
> that the clock driver wasn't compatible with current upstream (which I
> already mentioned).
>
> The PHY is external and already supported, adding it to my DT required
> very little work.
>
> >> - I²C support *
> >> - GPIO support (LEDs, buttons, misc. board management)
> >
> > again should work out of the box, minus your board specific
> > configuration?
>
> GPIO is completely OotB, yes. I²C is OotB once MVTWSI_CONTROL_CLEAR_IFLG
> is set correctly. (The pinctrl requirements for it are a little harder,
> more on that below.)
>
> >> - `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?
>
> Unsetting it is required for reset_cpu() to be defined. Your patchset
> updates that function (albeit without adding the WDT key, so the current
> patch is broken) to support NCAT2 already. U-Boot has no driver for
> "allwinner,sun20i-d1-wdt-reset", so this is the only way for `reset` to
> work.
So I had a look at this, and it's a bit surprising:
The watchdog driver already supports "allwinner,sun20i-d1-wdt" for a
while. We don't need to list the "-reset" version, because the normal
compatible name acts as a fallback string. However the DT node in the base
.dtsi sets: status = "reserved"; for this (and the other) watchdog, so
U-Boot's DM (correctly!) ignores those devices. Trying to figure out why.
Adding:
&wdt {
status = "okay";
};
to sun8i-t113s.dtsi fixes that for me, now the reset command works.
> > 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 ;-)
>
> I just hacked the correct value into the function; nothing really
> suitable for the list, sorry.
CONFIG_SYSRESET is only applicable for U-Boot proper, so we need
reset_cpu for the SPL still. That is not as critical, since the SPL resets
the board only in case of an error, but should be fixed nevertheless. I
will have a stab at this.
Cheers,
Andre
> >> - PSCI, nonsec
> >
> > ah yeah, owe you some reviews on this one ...
>
> It occurs to me that my work *might* support H6 as well (they both use
> CPUX blocks, right?) so perhaps it would be better if I de-RFC'd my
> series and instead worked to upstream it chasing H6, for you to come
> along later and tack on NCAT2 support with your R528 patchset?
>
> >> - 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?
>
> The conflict is that I'm on i2c2 + muxval 2. I suspect this one's going
> to be a downstream patch to add the necessary line:
> { "i2c2", 2 }, /* PE12-PE13 */
>
> ...and since no other assignment for i2c2 uses muxval 2, the only hope
> for this to be supported upstream would be for the pinctrl driver to
> include the full pin->muxval LUT.
>
> >> 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.
>
> At this time I have no interest in upstreaming the DT. That might change
> in the future, but for now it's very much meant to be out-of-tree.
>
> > Cheers,
> > Andre
>
> Likewise,
> Sam
