| Message ID | 20230117221050.630746-1-trini@konsulko.com |
|---|---|
| State | Accepted |
| Commit | 5bbf9c927a14d24e782bd5fdbbbb3fcaa0594679 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | None | expand |
On Tue, 17 Jan 2023 17:10:50 -0500 Tom Rini <trini@konsulko.com> wrote: > Prior to commit 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to > Kconfig") we had defined CONFIG_SYS_NS16550_REG_SIZE to -1 with > DM_SERIAL such that we would then have a size 0 character array. This > resulted in functionally no padding. The confusion on my part came from > dealing with the constraints around platforms that do not use DM_SERIAL > in SPL/TPL. After Andre Przywara reported that sunxi was broken, I've > re-read the code and comments again and thought on this harder. What we > want I believe is what this patch does now. > > If DM_SERIAL is defined for this stage, regardless of > CONFIG_SYS_NS16550_REG_SIZE then we will dynamically handle reg shifts > and 'struct ns16550' needs no padding (which is functionally what > unsigned char foo[0] provides). This is the same case as NS16550_DYNAMIC > and DEBUG_UART. Expand the existing comment here slightly. > > Otherwise, we will have CONFIG_SYS_NS16550_REG_SIZE set to a non-zero > value, and handle padding within the struct. > > Cc: Simon Glass <sjg@chromium.org> > Cc: Sergei Antonov <saproj@gmail.com> > Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com> > Cc: Andre Przywara <andre.przywara@arm.com> > Fixes: 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig") > Signed-off-by: Tom Rini <trini@konsulko.com> Thanks, that indeed fixes the issue for me, booted on LicheePi Nano and Pine64-LTS: Reviewed-by: Andre Przywara <andre.przywara@arm.com> Tested-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > include/ns16550.h | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/include/ns16550.h b/include/ns16550.h > index 243226fc3d94..e7e68663d030 100644 > --- a/include/ns16550.h > +++ b/include/ns16550.h > @@ -26,15 +26,13 @@ > > #include <linux/types.h> > > -#if CONFIG_IS_ENABLED(DM_SERIAL) && !defined(CONFIG_SYS_NS16550_REG_SIZE) > +#if CONFIG_IS_ENABLED(DM_SERIAL) || defined(CONFIG_NS16550_DYNAMIC) || \ > + defined(CONFIG_DEBUG_UART) > /* > * For driver model we always use one byte per register, and sort out the > - * differences in the driver > + * differences in the driver. In the case of CONFIG_NS16550_DYNAMIC we do > + * similar, and CONFIG_DEBUG_UART is responsible for shifts in its own manner. > */ > -#define CONFIG_SYS_NS16550_REG_SIZE (-1) > -#endif > - > -#if defined(CONFIG_NS16550_DYNAMIC) || defined(CONFIG_DEBUG_UART) > #define UART_REG(x) unsigned char x > #else > #if !defined(CONFIG_SYS_NS16550_REG_SIZE) || (CONFIG_SYS_NS16550_REG_SIZE == 0)
Hi Tom, On 1/17/23 23:10, Tom Rini wrote: > Prior to commit 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to > Kconfig") we had defined CONFIG_SYS_NS16550_REG_SIZE to -1 with > DM_SERIAL such that we would then have a size 0 character array. This > resulted in functionally no padding. The confusion on my part came from > dealing with the constraints around platforms that do not use DM_SERIAL > in SPL/TPL. After Andre Przywara reported that sunxi was broken, I've > re-read the code and comments again and thought on this harder. What we > want I believe is what this patch does now. > > If DM_SERIAL is defined for this stage, regardless of > CONFIG_SYS_NS16550_REG_SIZE then we will dynamically handle reg shifts > and 'struct ns16550' needs no padding (which is functionally what > unsigned char foo[0] provides). This is the same case as NS16550_DYNAMIC > and DEBUG_UART. Expand the existing comment here slightly. > > Otherwise, we will have CONFIG_SYS_NS16550_REG_SIZE set to a non-zero > value, and handle padding within the struct. > > Cc: Simon Glass <sjg@chromium.org> > Cc: Sergei Antonov <saproj@gmail.com> > Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com> > Cc: Andre Przywara <andre.przywara@arm.com> > Fixes: 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig") > Signed-off-by: Tom Rini <trini@konsulko.com> Tested-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> #Ringneck PX30, Puma RK3399 Thanks, Quentin
diff --git a/include/ns16550.h b/include/ns16550.h index 243226fc3d94..e7e68663d030 100644 --- a/include/ns16550.h +++ b/include/ns16550.h @@ -26,15 +26,13 @@ #include <linux/types.h> -#if CONFIG_IS_ENABLED(DM_SERIAL) && !defined(CONFIG_SYS_NS16550_REG_SIZE) +#if CONFIG_IS_ENABLED(DM_SERIAL) || defined(CONFIG_NS16550_DYNAMIC) || \ + defined(CONFIG_DEBUG_UART) /* * For driver model we always use one byte per register, and sort out the - * differences in the driver + * differences in the driver. In the case of CONFIG_NS16550_DYNAMIC we do + * similar, and CONFIG_DEBUG_UART is responsible for shifts in its own manner. */ -#define CONFIG_SYS_NS16550_REG_SIZE (-1) -#endif - -#if defined(CONFIG_NS16550_DYNAMIC) || defined(CONFIG_DEBUG_UART) #define UART_REG(x) unsigned char x #else #if !defined(CONFIG_SYS_NS16550_REG_SIZE) || (CONFIG_SYS_NS16550_REG_SIZE == 0)
Prior to commit 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig") we had defined CONFIG_SYS_NS16550_REG_SIZE to -1 with DM_SERIAL such that we would then have a size 0 character array. This resulted in functionally no padding. The confusion on my part came from dealing with the constraints around platforms that do not use DM_SERIAL in SPL/TPL. After Andre Przywara reported that sunxi was broken, I've re-read the code and comments again and thought on this harder. What we want I believe is what this patch does now. If DM_SERIAL is defined for this stage, regardless of CONFIG_SYS_NS16550_REG_SIZE then we will dynamically handle reg shifts and 'struct ns16550' needs no padding (which is functionally what unsigned char foo[0] provides). This is the same case as NS16550_DYNAMIC and DEBUG_UART. Expand the existing comment here slightly. Otherwise, we will have CONFIG_SYS_NS16550_REG_SIZE set to a non-zero value, and handle padding within the struct. Cc: Simon Glass <sjg@chromium.org> Cc: Sergei Antonov <saproj@gmail.com> Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com> Cc: Andre Przywara <andre.przywara@arm.com> Fixes: 9591b63531fa ("Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig") Signed-off-by: Tom Rini <trini@konsulko.com> --- include/ns16550.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)