Re: [PATCH v2 3/4] i2c: k1: enable both functional and bus clocks

[Yixun Lan <dlan@kernel.org>]

hi Troy,

On 16:56 Mon 17 Aug     , Troy Mitchell wrote:
> > The K1 I2C controller requires both its functional clock and APB bus
> > clock to operate. The device tree provides them as "func" and "bus", but
> > the driver currently acquires and enables only the first clock.
> >
> > Acquire both clocks by name and enable them during probe. Use explicit
> > named lookups instead of the bulk clock API to align with the K1 Linux
> > driver and keep the roles of the two clocks clear if functional clock
> > rate configuration is needed later.
> 
> [...]
> 
> > @@ -487,15 +487,21 @@ static int k1_i2c_probe(struct udevice *bus)
> >  		return ret;
> >  	}
> >
> > -	ret = clk_get_by_index(bus, 0, &priv->clk);
> > +	ret = clk_get_by_name(bus, "func", &clk);
> >  	if (ret)
> >  		return ret;
> >
> > -	ret = clk_enable(&priv->clk);
> > -	if (ret && ret != -ENOSYS && ret != -EOPNOTSUPP) {
> > -		debug("%s: failed to enable clock\n", __func__);
> > +	ret = clk_enable(&clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_get_by_name(bus, "bus", &clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_enable(&clk);
> > +	if (ret)
> >  		return ret;
> > -	}
> 
> Once the functional clock has been enabled, failure to acquire or enable
> the bus clock returns without disabling the functional clock.
> 
I'd not worry about this, as in uboot it isn't worth the effort to do the
resouce reclamation, instead I'm more concerned about the error handling..


-- 
Yixun Lan (dlan)