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

On Mon Aug 17, 2026 at 5:18 PM +08, Yixun Lan wrote:
> 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..
Reviewed-by: Troy Mitchell <[email protected]>


-- 
Troy Mitchell

-----BEGIN PGP SIGNATURE-----

iIMEABYKACsWIQSL4Ay2cExaPXAQcU2YCe+A+TM0LwUCaoOvfg0caUB0cm95LXku
b3JnAAoJEJgJ74D5MzQvwg4BAIS1uUyCjrHVjapCPXvliuQ9ZAlWKc3jQItF3pVY
WC4aAQDH2QpwIG2VSgxz8oIN8kEWVqZPBtRRVqcrbOLnVwMnDg==
=ZXt0
-----END PGP SIGNATURE-----