On Tue, Jan 17, 2023 at 08:15:18AM -0500, Tom Rini wrote:
> On Tue, Jan 17, 2023 at 12:09:38PM +0000, Andre Przywara wrote:
>
> > Commit 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig")
> > moved some NS16550 configuration variables into Kconfig.
> > Among those there is CONFIG_SYS_NS16550_REG_SIZE, which used to be only
> > defined for SPL build runs, but now is always set (thanks for Kconfig).
> > However this breaks the gating logic in ns16550.h, where we *override*
> > this variable for DM build, as we learn this setting from the DT instead.
> >
> > As a consequence, we did the register shift twice: once when building
> > the register struct (as required for non-DM SPL builds), but then also
> > again in the driver after we parsed the reg-shift DT property.
> >
> > Change the logic to match what the comment says: only observe
> > CONFIG_SYS_NS16550_REG_SIZE when not using DM, and ignore it otherwise.
> >
> > This fixes U-Boot proper for all sunxi boards, since they are relying
> > on this driver being build non-DM for the SPL, but DM for U-Boot proper.
> >
> > Fixes: 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig")
> > Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
> > ---
> > Hi,
> >
> > this is admittedly a quick fix, to get sunxi booting again. This whole
> > code around the register access looks somewhat bonkers, to be honest,
> > but at the moment I don't have time to rework this.
> > Another possible "quicker fix" would be use a separate variable for the
> > register shift, so that we don't have to redefine a Kconfig variable.
> > Another idea would be to get rid of the struct for the registers
> > altogether, to remove the hacks about when to shift.
> >
> > Cheers,
> > Andre
> >
> > include/ns16550.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/ns16550.h b/include/ns16550.h
> > index 243226fc3d9..e9e2aeedd16 100644
> > --- a/include/ns16550.h
> > +++ b/include/ns16550.h
> > @@ -26,11 +26,12 @@
> >
> > #include <linux/types.h>
> >
> > -#if CONFIG_IS_ENABLED(DM_SERIAL) && !defined(CONFIG_SYS_NS16550_REG_SIZE)
> > +#if CONFIG_IS_ENABLED(DM_SERIAL)
> > /*
> > * For driver model we always use one byte per register, and sort out the
> > * differences in the driver
> > */
> > +#undef CONFIG_SYS_NS16550_REG_SIZE
> > #define CONFIG_SYS_NS16550_REG_SIZE (-1)
> > #endif
>
> Does:
> https://patchwork.ozlabs.org/project/uboot/patch/20230110161946.3816866-5-tr...@konsulko.com/
> work for you as well?This got me to double check why my pine64 wasn't in CI, again, and now I see that no, this patch doesn't solve that case. -- Tom
signature.asc
Description: PGP signature
