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

[Vlad Zakharov]

Hello,

I found that this commit breaks serial ns16550 driver on ARC.
I have investigated the issue and found that it is because ARC timer node in device tree doesn't refer to any clock
devices. 

In such case clk_get_by_index() returns -ENOENT error, but neither -ENODEV nor -ENOSYS (as CONFIG_CLK is enabled for
ARC). So the error is not ignored and we exit with error instead of trying to get "clock-frequency" property from the
device tree.

Thus I wonder why we ignore only ENOSYS and ENODEV errors and exit if any other error code appears?
As shown in my example such behavior can lead to breakages. 
What should we do if ENOENT occurs? 

Thanks.

On Thu, 2016-09-08 at 07:47 +0100, Paul Burton 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 at 10000000 {
>     compatible = "ns16550a";
>     reg = <0x10000000 0x1000>;
>     clocks = <&sys_clk>;
>   };
> 
>   uart1: uart at 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 at imgtec.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> ---
> 
> Changes in v7:
> - Check clk_get_rate return for error values
> 
> 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 | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 88fca15..3f6ea4d 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,21 @@ 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) {
> +		err = clk_get_rate(&clk);
> +		if (!IS_ERR_VALUE(err))
> +			plat->clock = err;
> +	} 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;
-- 
Best regards,
Vlad Zakharov <vzakhar at synopsys.com>