| Message ID | 20221128054834.34950-1-samuel@sholland.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | serial: ns16550: Enable clocks during probe | expand |
On 11/28/22 06:48, Samuel Holland wrote: > If the UART bus or baud clock has a gate, it must be enabled before the > UART can be used. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > drivers/serial/ns16550.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c > index 7592979cab5..785fb520062 100644 > --- a/drivers/serial/ns16550.c > +++ b/drivers/serial/ns16550.c > @@ -506,6 +506,7 @@ int ns16550_serial_probe(struct udevice *dev) > struct ns16550_plat *plat = dev_get_plat(dev); > struct ns16550 *const com_port = dev_get_priv(dev); > struct reset_ctl_bulk reset_bulk; > + struct clk_bulk clk_bulk; > fdt_addr_t addr; > int ret; > > @@ -524,6 +525,10 @@ int ns16550_serial_probe(struct udevice *dev) > if (!ret) > reset_deassert_bulk(&reset_bulk); > > + ret = clk_get_bulk(dev, &clk_bulk); > + if (!ret) > + clk_enable_bulk(&clk_bulk); > + > com_port->plat = dev_get_plat(dev); > ns16550_init(com_port, -1); > Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
On Sun, Nov 27, 2022 at 11:48:34PM -0600, Samuel Holland wrote: > If the UART bus or baud clock has a gate, it must be enabled before the > UART can be used. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > Reviewed-by: Stefan Roese <sr@denx.de> This breaks building on phycore-rk3288
On 12/12/22 12:54, Tom Rini wrote: > On Sun, Nov 27, 2022 at 11:48:34PM -0600, Samuel Holland wrote: > >> If the UART bus or baud clock has a gate, it must be enabled before the >> UART can be used. >> >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> Reviewed-by: Stefan Roese <sr@denx.de> > > This breaks building on phycore-rk3288 I get: binman: Error 1 running 'mkimage -d ./mkimage.simple-bin.mkimage -n rk3288 -T rksd ./idbloader.img': Error: SPL image is too large (size 0x8800 than 0x8000) Before applying this patch: $ ls -l spl/u-boot-spl.bin -rw-r--r-- 1 samuel samuel 32704 Dec 12 19:35 spl/u-boot-spl.bin So the board was quite close to its SPL size limit already. I was trying to be general with this patch, but I suppose for my immediate purposes (Allwinner D1), I only care about the first clock. If I use clk_get_by_index() instead of clk_get_bulk(), the phycore-rk3288 build passes with 4 bytes to spare: $ ls -l spl/u-boot-spl.bin -rw-r--r-- 1 samuel samuel 32760 Dec 12 19:36 spl/u-boot-spl.bin I will send a v2, but I imagine some other unsuspecting patch will run into this limit again before long. Regards, Samuel
Hi Samuel, On 12/13/22 02:46, Samuel Holland wrote: > On 12/12/22 12:54, Tom Rini wrote: >> On Sun, Nov 27, 2022 at 11:48:34PM -0600, Samuel Holland wrote: >> >>> If the UART bus or baud clock has a gate, it must be enabled before the >>> UART can be used. >>> >>> Signed-off-by: Samuel Holland <samuel@sholland.org> >>> Reviewed-by: Stefan Roese <sr@denx.de> >> >> This breaks building on phycore-rk3288 > > I get: > > binman: Error 1 running 'mkimage -d ./mkimage.simple-bin.mkimage -n > rk3288 -T rksd ./idbloader.img': Error: SPL image is too large (size > 0x8800 than 0x8000) > > Before applying this patch: > > $ ls -l spl/u-boot-spl.bin > -rw-r--r-- 1 samuel samuel 32704 Dec 12 19:35 spl/u-boot-spl.bin > > So the board was quite close to its SPL size limit already. > > I was trying to be general with this patch, but I suppose for my > immediate purposes (Allwinner D1), I only care about the first clock. If > I use clk_get_by_index() instead of clk_get_bulk(), the phycore-rk3288 > build passes with 4 bytes to spare: > > $ ls -l spl/u-boot-spl.bin > -rw-r--r-- 1 samuel samuel 32760 Dec 12 19:36 spl/u-boot-spl.bin > > I will send a v2, but I imagine some other unsuspecting patch will run > into this limit again before long. Why not enable LTO to save more space. I just checked this on this platform: w/o LTO: spl/u-boot-spl.bin 32604 with LTO enabled: spl/u-boot-spl.bin 30016 Not tested though. Thanks, Stefan
Hi Stefan, Am 13.12.22 um 07:14 schrieb Stefan Roese: > Hi Samuel, > > On 12/13/22 02:46, Samuel Holland wrote: >> On 12/12/22 12:54, Tom Rini wrote: >>> On Sun, Nov 27, 2022 at 11:48:34PM -0600, Samuel Holland wrote: >>> >>>> If the UART bus or baud clock has a gate, it must be enabled before the >>>> UART can be used. >>>> >>>> Signed-off-by: Samuel Holland <samuel@sholland.org> >>>> Reviewed-by: Stefan Roese <sr@denx.de> >>> >>> This breaks building on phycore-rk3288 >> >> I get: >> >> binman: Error 1 running 'mkimage -d ./mkimage.simple-bin.mkimage -n >> rk3288 -T rksd ./idbloader.img': Error: SPL image is too large (size >> 0x8800 than 0x8000) >> >> Before applying this patch: >> >> $ ls -l spl/u-boot-spl.bin >> -rw-r--r-- 1 samuel samuel 32704 Dec 12 19:35 spl/u-boot-spl.bin >> >> So the board was quite close to its SPL size limit already. >> >> I was trying to be general with this patch, but I suppose for my >> immediate purposes (Allwinner D1), I only care about the first clock. If >> I use clk_get_by_index() instead of clk_get_bulk(), the phycore-rk3288 >> build passes with 4 bytes to spare: >> >> $ ls -l spl/u-boot-spl.bin >> -rw-r--r-- 1 samuel samuel 32760 Dec 12 19:36 spl/u-boot-spl.bin >> >> I will send a v2, but I imagine some other unsuspecting patch will run >> into this limit again before long. > > Why not enable LTO to save more space. I just checked this on this > platform: > > w/o LTO: > spl/u-boot-spl.bin 32604 > > with LTO enabled: > spl/u-boot-spl.bin 30016 > > Not tested though. Thanks for the hint with the LTO. I just send out a patch to enable it for the phycore-rk3288 https://lists.denx.de/pipermail/u-boot/2022-December/502125.html Regards, Wadim > > Thanks, > Stefan
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 7592979cab5..785fb520062 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -506,6 +506,7 @@ int ns16550_serial_probe(struct udevice *dev) struct ns16550_plat *plat = dev_get_plat(dev); struct ns16550 *const com_port = dev_get_priv(dev); struct reset_ctl_bulk reset_bulk; + struct clk_bulk clk_bulk; fdt_addr_t addr; int ret; @@ -524,6 +525,10 @@ int ns16550_serial_probe(struct udevice *dev) if (!ret) reset_deassert_bulk(&reset_bulk); + ret = clk_get_bulk(dev, &clk_bulk); + if (!ret) + clk_enable_bulk(&clk_bulk); + com_port->plat = dev_get_plat(dev); ns16550_init(com_port, -1);
If the UART bus or baud clock has a gate, it must be enabled before the UART can be used. Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/serial/ns16550.c | 5 +++++ 1 file changed, 5 insertions(+)