| Message ID | 20250409192213.1130181-3-elder@riscstar.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | serial: 8250_of: support an optional bus clock | expand |
Hi Alex, On 17:47 Wed 09 Apr , Alex Elder wrote: > On 4/9/25 4:43 PM, Yixun Lan wrote: > > Hi Alex, > > > > On 14:22 Wed 09 Apr , Alex Elder wrote: > >> The SpacemiT UART requires a bus clock to be enabled, in addition to > >> it's "normal" core clock. Look up the optional bus clock by name, > >> and if that's found, look up the core clock using the name "core". > >> > >> Supplying a bus clock is optional. If no bus clock is needed, the > >> the first/only clock is used for the core clock. > >> > >> Signed-off-by: Alex Elder <elder@riscstar.com> > >> --- > >> v2: Update logic to more check for the optional bus clock first > >> > >> drivers/tty/serial/8250/8250_of.c | 11 ++++++++++- > >> 1 file changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c > >> index 11c860ea80f60..a90a5462aa72a 100644 > >> --- a/drivers/tty/serial/8250/8250_of.c > >> +++ b/drivers/tty/serial/8250/8250_of.c > >> @@ -123,7 +123,16 @@ static int of_platform_serial_setup(struct platform_device *ofdev, > >> > >> /* Get clk rate through clk driver if present */ > >> if (!port->uartclk) { > >> - info->clk = devm_clk_get_enabled(dev, NULL); > >> + struct clk *bus_clk; > > we also need to handle clk in suspend/resume procedure, so > > I think you need to put bus_clk inside struct of_serial_info.. > > OK, I didn't do anything for that in previous versions of the > series. > > I think that means we'd call clk_disable_unprepare() on > the bus clock after doing so for the function clock. And > clk_prepare_enable() on the bus clock before doing that for > the function clock in of_serial_resume(). That's easy. > right, pretty much similar to info->clk > Is there anything further you think is required? There is > no clock rate associated with the bus clock that I know of, > so even if the function clock rate changes, the bus clock > can remain as-is. > no further info I know of, my best guess, the rate doesn't really matter, a wide clk range should just work fine. > > > >> + > >> + bus_clk = devm_clk_get_optional_enabled(dev, "bus"); > > for the 'optional', we can interpret it's optional for other vendor > > UART, but a must required clk for SpacemiT's k1 UART controller > > > > would it better to guard this inside a compatible test or even introduce > > a flag in compatible data? > > I don't personally think so. We could, but the DT binding is going > out of its way to define when the bus clock is required. This is > simpler, and generic. > I would personally choose the way of introducing a flag of compatible data, but it's a lot change of the code (may not worth the effort).. anyway, I'm fine with your current version, and yes, it's generic thanks for doing this..
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c index 11c860ea80f60..a90a5462aa72a 100644 --- a/drivers/tty/serial/8250/8250_of.c +++ b/drivers/tty/serial/8250/8250_of.c @@ -123,7 +123,16 @@ static int of_platform_serial_setup(struct platform_device *ofdev, /* Get clk rate through clk driver if present */ if (!port->uartclk) { - info->clk = devm_clk_get_enabled(dev, NULL); + struct clk *bus_clk; + + bus_clk = devm_clk_get_optional_enabled(dev, "bus"); + if (IS_ERR(bus_clk)) { + ret = dev_err_probe(dev, PTR_ERR(bus_clk), "failed to get bus clock\n"); + goto err_pmruntime; + } + + /* If the bus clock is required, core clock must be named */ + info->clk = devm_clk_get_enabled(dev, bus_clk ? "core" : NULL); if (IS_ERR(info->clk)) { ret = dev_err_probe(dev, PTR_ERR(info->clk), "failed to get clock\n"); goto err_pmruntime;
The SpacemiT UART requires a bus clock to be enabled, in addition to it's "normal" core clock. Look up the optional bus clock by name, and if that's found, look up the core clock using the name "core". Supplying a bus clock is optional. If no bus clock is needed, the the first/only clock is used for the core clock. Signed-off-by: Alex Elder <elder@riscstar.com> --- v2: Update logic to more check for the optional bus clock first drivers/tty/serial/8250/8250_of.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)