| Message ID | 20240804150955.3521296-1-jonas@kwiboo.se |
|---|---|
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | serial: ns16550: Try get serial clock rate from DT before CLK | expand |
Hi Jonas, On 8/4/24 5:09 PM, Jonas Karlman wrote: > Initializing a clock driver to read a known static clock rate can take > some time at U-Boot proper pre-reloc phase. > > Change to first try and read clock rate from DT to speed up boot time, > fall back to getting the clock rate from clock driver. > > This help reduce boot time by around: > - ~35ms on a Radxa ROCK Pi 4 (RK3399) > - ~15ms on a Radxa ZERO 3W (RK3566) > Time that is wasted getting a static rate known at compile time. > I guess this also makes some board perform worse as well, the ones without a clock-frequency set? The change seems sound to me, therefore Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> Thanks! Quentin
Hi Quentin, On 2024-08-05 09:19, Quentin Schulz wrote: > Hi Jonas, > > On 8/4/24 5:09 PM, Jonas Karlman wrote: >> Initializing a clock driver to read a known static clock rate can take >> some time at U-Boot proper pre-reloc phase. >> >> Change to first try and read clock rate from DT to speed up boot time, >> fall back to getting the clock rate from clock driver. >> >> This help reduce boot time by around: >> - ~35ms on a Radxa ROCK Pi 4 (RK3399) >> - ~15ms on a Radxa ZERO 3W (RK3566) >> Time that is wasted getting a static rate known at compile time. >> > > I guess this also makes some board perform worse as well, the ones > without a clock-frequency set? I do not expect that this change will add anything to existing boot time, however in some circumstances it may reduce the boot time. Before this change the rate was retrieved using first successful of: 1. clk_get_rate() 2. clock-frequency prop from DT 3. CFG_SYS_NS16550_CLK This patch swap 1. and 2. so driver first will try to use any rate that may have been defined in DT, thus possible skip a probe of the clock. Boot time is only saved/reduced if the serial driver was the only device that needed something from clock device in current phase. As long as the DT contain correct rate in clock-frequency prop or no such prop this patch is not expected to change anything. Regards, Jonas > > The change seems sound to me, therefore > Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> > > Thanks! > Quentin
Hi Jonas, On 8/5/24 11:02 AM, Jonas Karlman wrote: > Hi Quentin, > > On 2024-08-05 09:19, Quentin Schulz wrote: >> Hi Jonas, >> >> On 8/4/24 5:09 PM, Jonas Karlman wrote: >>> Initializing a clock driver to read a known static clock rate can take >>> some time at U-Boot proper pre-reloc phase. >>> >>> Change to first try and read clock rate from DT to speed up boot time, >>> fall back to getting the clock rate from clock driver. >>> >>> This help reduce boot time by around: >>> - ~35ms on a Radxa ROCK Pi 4 (RK3399) >>> - ~15ms on a Radxa ZERO 3W (RK3566) >>> Time that is wasted getting a static rate known at compile time. >>> >> >> I guess this also makes some board perform worse as well, the ones >> without a clock-frequency set? > > I do not expect that this change will add anything to existing boot > time, however in some circumstances it may reduce the boot time. > > Before this change the rate was retrieved using first successful of: > > 1. clk_get_rate() > 2. clock-frequency prop from DT > 3. CFG_SYS_NS16550_CLK > > This patch swap 1. and 2. so driver first will try to use any rate that > may have been defined in DT, thus possible skip a probe of the clock. > > Boot time is only saved/reduced if the serial driver was the only device > that needed something from clock device in current phase. > > As long as the DT contain correct rate in clock-frequency prop or no > such prop this patch is not expected to change anything. > Well yes, that's what I meant. If there's a board without DT or without clock-frequency in the DT, then this board's boot would be slowed down. The ns16550 being used for many many different SoCs, I don't think we can necessarily assume everyone is using DM. All of this is merely me highlighting this fact, not arguing whether this patch should be merged or not :) Cheers, Quentin
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 6fcb5b523acb..07f9ac00e113 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -565,19 +565,19 @@ int ns16550_serial_of_to_plat(struct udevice *dev) plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0); plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1); - err = clk_get_by_index(dev, 0, &clk); - if (!err) { - err = clk_get_rate(&clk); - if (!IS_ERR_VALUE(err)) - plat->clock = err; - } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) { - debug("ns16550 failed to get clock\n"); - return err; - } - if (!plat->clock) - plat->clock = dev_read_u32_default(dev, "clock-frequency", - CFG_SYS_NS16550_CLK); + plat->clock = dev_read_u32_default(dev, "clock-frequency", 0); + if (!plat->clock) { + err = clk_get_by_index(dev, 0, &clk); + if (!err) { + err = clk_get_rate(&clk); + if (!IS_ERR_VALUE(err)) + plat->clock = err; + } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) { + debug("ns16550 failed to get clock\n"); + return err; + } + } if (!plat->clock) plat->clock = CFG_SYS_NS16550_CLK; if (!plat->clock) {
Initializing a clock driver to read a known static clock rate can take some time at U-Boot proper pre-reloc phase. Change to first try and read clock rate from DT to speed up boot time, fall back to getting the clock rate from clock driver. This help reduce boot time by around: - ~35ms on a Radxa ROCK Pi 4 (RK3399) - ~15ms on a Radxa ZERO 3W (RK3566) Time that is wasted getting a static rate known at compile time. Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- drivers/serial/ns16550.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)