> On 25 Feb 2018, at 11:24, Kever Yang <kever.y...@rock-chips.com> wrote:
> 
> Assigned clocks are widely used in kernel, but not in U-Boot yet,
> many U-Boot clock driver do not have the API while dts port from kernel
> have "assigned-clocks" node.
> 
> Just give a warning now instead of a device probe fail.

I strongly disagree on this one: the only reason this can fail is if 
assigned-clock-rates
can not be set (e.g. because a clock that should be assigned to is not known by 
the
clock driver).  However, this should never be ignored silently.

If the clock subsystem does not know all clocks that are being attempted to 
set, then
commits to shared drivers will eventually break: e.g. commit ba1f96672522
("net: designware: add clock support”) recently broke the GMAC for the RK3368 
and
RK3399 because it iterated over all clocks defined in the “clocks” property of 
the
GMAC node.

As long as various drivers perform an unconditional clk_enable and/or an 
unconditional
clk_set_rate, we should enforce this in the core already.  In consequence, 
dedicated
code from the drivers should now start to slowly disappear, as clock-rates can 
now be
set via the DTS.

> 
> Signed-off-by: Kever Yang <kever.y...@rock-chips.com>
> ---
> 
> drivers/core/device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 9d58f44..71872e9 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -407,7 +407,7 @@ int device_probe(struct udevice *dev)
>       /* Process 'assigned-{clocks/clock-parents/clock-rates}' properties */
>       ret = clk_set_defaults(dev);
>       if (ret)
> -             goto fail;
> +             debug("%s clk_set_defaults failed %d\n", ret);

This probably shouldn’t have been a silent failure.
A pr_err() may be more appropriate… I would recommend this to continue failing
though, as it is simple enough to handle the additional clocks in the clock 
drivers
and create a permanent record of “things not needing additional changes, due to
the BROM already having set up things” by returning success.

> 
>       if (drv->probe) {
>               ret = drv->probe(dev);
> -- 
> 1.9.1
> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to