Re: [PATCH] rockchip: rk3568: add support of all UART

2024-01-08 Thread Arseniy Mescheryakov
Hello, you are right, that this patch is not needed for working
console and with property configured dt and config it works. But for
debug (for example, everything that was printed by debug()) it is
needed. Withit this code you will not see this prints.
And yes, via DDR blob you can configure a specific uart and it debug()
will work, but I think u-boot shouldn't depend on some proprietary
blobs.


Re: [PATCH] rockchip: rk3568: add support of all UART

2024-01-01 Thread Arseniy Mescheryakov
Thanks for the answer.
I think developers should have the opportunity to configure UART from
the u-boot directly (via configuration) without any additional
proprietary blobs.

There is 
https://github.com/rockchip-linux/u-boot/blob/next-dev/arch/arm/mach-rockchip/rk3568/rk3568.c
, that has same functionality as my patch, but it has a lot of code
duplication and because of it i add a lot of macroses.
Maybe there is any target that has a lot of UARTs instances and its
configuration going in other way than mine and hasn't a lot of code
duplication.


On Sun, Dec 31, 2023 at 3:31 AM Jonas Karlman  wrote:
>
> Hi Arseniy,
>
> On 2023-12-30 12:20, Arseniy Meshcheryakov wrote:
> > This patch add support of all UARTs for rk3568, which can be used in 
> > console.
>
> Please explain why this is needed, on rk35xx the rockchip ddr init blob
> is required and can setup correct uart iomux with rkbin/tools/ddrbin_tool [1].
>
> And U-Boot SPL and proper can use the serial, gpio and pinctrl drivers
> to configure correct uart iomux for any board not using uart2.
>
> We should be able to remove the current code that configures uart2 M0
> iomux and still have a working debug console.
>
> Please provide more details why and on what board you need this.
>
> [1] 
> https://github.com/rockchip-linux/rkbin/blob/master/tools/ddrbin_tool_user_guide.txt#L72-L75
>
> >
> > Signed-off-by: Arseniy Meshcheryakov 
> > ---
> >  arch/arm/mach-rockchip/rk3568/rk3568.c | 470 -
> >  1 file changed, 452 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c 
> > b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > index 69ef19cc85..f79b47ee5b 100644
> > --- a/arch/arm/mach-rockchip/rk3568/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c
> > @@ -34,26 +34,468 @@
> >  #define CPU_GRF_BASE 0xfdc3
> >  #define GRF_CORE_PVTPLL_CON0 (0x10)
> >
> > -/* PMU_GRF_GPIO0D_IOMUX_L */
> >  enum {
> > + /* PMU_GRF_GPIO0C_IOMUX_L */
> > + GPIO0C1_SHIFT   = 4,
> > + GPIO0C1_MASK= GENMASK(6, 4),
> > + GPIO0C1_GPIO= 0,
> > + GPIO0C1_UART0_M0_TX = 3,
> > +
> > + GPIO0C0_SHIFT   = 0,
> > + GPIO0C0_MASK= GENMASK(2, 0),
> > + GPIO0C0_GPIO= 0,
> > + GPIO0C0_UART0_M0_RX = 3,
> > +
> > + /* PMU_GRF_GPIO0D_IOMUX_L */
> >   GPIO0D1_SHIFT   = 4,
> >   GPIO0D1_MASK= GENMASK(6, 4),
> >   GPIO0D1_GPIO= 0,
> > - GPIO0D1_UART2_TXM0,
> > + GPIO0D1_UART2_M0_TX,
> >
> >   GPIO0D0_SHIFT   = 0,
> >   GPIO0D0_MASK= GENMASK(2, 0),
> >   GPIO0D0_GPIO= 0,
> > - GPIO0D0_UART2_RXM0,
> > -};
> > + GPIO0D0_UART2_M0_RX,
> > +
> > + /* GRF_GPIO1A_IOMUX_L */
> > + GPIO1A1_SHIFT   = 4,
> > + GPIO1A1_MASK= GENMASK(6, 4),
> > + GPIO1A1_GPIO= 0,
> > + GPIO1A1_UART3_M0_TX = 2,
> > +
> > + GPIO1A0_SHIFT   = 0,
> > + GPIO1A0_MASK= GENMASK(2, 0),
> > + GPIO1A0_GPIO= 0,
> > + GPIO1A0_UART3_M0_RX = 2,
> > +
> > + /* GRF_GPIO1A_IOMUX_H */
> > + GPIO1A6_SHIFT   = 8,
> > + GPIO1A6_MASK= GENMASK(10, 8),
> > + GPIO1A6_GPIO= 0,
> > + GPIO1A6_UART4_M0_TX = 2,
> > +
> > +
> > + GPIO1A4_SHIFT   = 0,
> > + GPIO1A4_MASK= GENMASK(2, 0),
> > + GPIO1A4_GPIO= 0,
> > + GPIO1A4_UART4_M0_RX = 2,
> > +
> > + /* GRF_GPIO1D_IOMUX_H */
> > + GPIO1D6_SHIFT   = 8,
> > + GPIO1D6_MASK= GENMASK(10, 8),
> > + GPIO1D6_GPIO= 0,
> > + GPIO1D6_UART2_M1_RX = 2,
> > + GPIO1D6_UART6_M1_RX,
> > +
> > + GPIO1D5_SHIFT   = 4,
> > + GPIO1D5_MASK= GENMASK(6, 4),
> > + GPIO1D5_GPIO= 0,
> > + GPIO1D5_UART2_M1_TX = 2,
> > + GPIO1D5_UART6_M1_TX,
> > +
> > + /* GRF_GPIO2A_IOMUX_L */
> > + GPIO2A3_SHIFT   = 12,
> > + GPIO2A3_MASK= GENMASK(14, 12),
> > + GPIO2A3_GPIO= 0,
> > + GPIO2A3_UART6_M0_RX = 3,
> > +
> > + GPIO2A2_SHIFT   = 8,
> > + GPIO2A2_MASK= GENMASK(10, 8),
> > + GPIO2A2_GPIO= 0,
> > + GPIO2A2_UART5_M0_TX = 3,
> > +
> > + GPIO2A1_SHIFT   = 4,
> > + GPIO2A1_MASK= GENMASK(6, 4),
> > + GPIO2A1_GPIO= 0,
> > + GPIO2A1_UART5_M0_RX = 3,
> > +
> > + /* GRF_GPIO2A_IOMUX_H */
> > + GPIO2A7_SHIFT   = 12,
> > + GPIO2A7_MASK= GENMASK(14, 12),
> > + GPIO2A7_GPIO= 0,
> > + GPIO2A7_UART9_M0_RX = 3,
> > +
> > + GPIO2A6_SHIFT   = 8,
> > + GPIO2A6_MASK= GENMASK(10, 8),
> > + GPIO2A6_GPIO= 0,
> > + GPIO2A6_UART7_M0_TX = 3,
> > +
> > + GPIO2A5_SHIFT   = 4,
> > +