Re: [U-Boot] [PATCH v6 02/12] serial: ns16550: Support clocks via phandle

Hi Paul,

On 4 August 2016 at 04:29, Paul Burton paul.burton@imgtec.com wrote:

Previously ns16550 compatible UARTs probed via device tree have needed their device tree nodes to contain a clock-frequency property. An alternative to this commonly used with Linux is to reference a clock via a phandle. This patch allows U-Boot to support that, retrieving the clock frequency by probing the appropriate clock device.

For example, a system might choose to provide the UART base clock as a reference to a clock common to multiple devices:

sys_clk: clock { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <10000000>; };

uart0: uart@10000000 { compatible = "ns16550a"; reg = <0x10000000 0x1000>; clocks = <&sys_clk>; };

uart1: uart@10000000 { compatible = "ns16550a"; reg = <0x10001000 0x1000>; clocks = <&sys_clk>; };

This removes the need for the frequency information to be duplicated in multiple nodes and allows the device tree to be more descriptive of the system.

Signed-off-by: Paul Burton paul.burton@imgtec.com Reviewed-by: Simon Glass sjg@chromium.org

---

Changes in v6:

• Ignore -ENOSYS from clk_get_by_index too, for systems with CONFIG_CLK=n

Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2:

• Propogate non-ENODEV errors from clk_get_by_index

drivers/serial/ns16550.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 88fca15..af317cb 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -5,6 +5,7 @@ */

#include <common.h> +#include <clk.h> #include <dm.h> #include <errno.h> #include <fdtdec.h> @@ -352,6 +353,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev) { struct ns16550_platdata *plat = dev->platdata; fdt_addr_t addr;

•   struct clk clk;
  

•   int err;
  
    /* try Processor Local Bus device first */
    addr = dev_get_addr(dev);
  

@@ -397,9 +400,19 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev) "reg-offset", 0); plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "reg-shift", 0);

•   plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
  

•                                "clock-frequency",
  

•                                CONFIG_SYS_NS16550_CLK);
  

•   err = clk_get_by_index(dev, 0, &clk);
  

•   if (!err) {
  

•           plat->clock = clk_get_rate(&clk);
  

While I think this is a good change, it breaks boards which have a clock in the device tree but don't support it in their driver.

I think you can fix it by checking the error value from clk_get_rate(), i.e. something like:

if (!err) { err = clk_get_rate(&clk); if (!IS_ERR_VALUE(err)) plat->clock = err; } else if ...

•   } else if (err != -ENODEV && err != -ENOSYS) {
  

•           debug("ns16550 failed to get clock\n");
  

•           return err;
  

•   }
  

•   if (!plat->clock)
  

•           plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
  

•                                        "clock-frequency",
  

•                                        CONFIG_SYS_NS16550_CLK);
    if (!plat->clock) {
            debug("ns16550 clock not defined\n");
            return -EINVAL;
  

-- 2.9.2

Regards, Simon