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

[Yao Zi <me@ziyao.cc>]

On Tue, Aug 18, 2026 at 06:21:17AM +0000, Yao Zi wrote:
> On Sat, Aug 15, 2026 at 11:40:23AM +0800, Junhui Liu 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.
> > 
> > Fixes: 271546fb8e54 ("i2c: k1: add I2C driver support")
> > Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> > ---
> >  drivers/i2c/k1_i2c.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/i2c/k1_i2c.c b/drivers/i2c/k1_i2c.c
> > index 2c7a1e0d3775..e2e4f9e53447 100644
> > --- a/drivers/i2c/k1_i2c.c
> > +++ b/drivers/i2c/k1_i2c.c
> > @@ -51,7 +51,6 @@ struct k1_i2c {
> >  struct k1_i2c_priv {
> >  	int id;
> >  	void __iomem *base;
> > -	struct clk clk;
> >  };
> >  
> >  /*
> > @@ -465,6 +464,7 @@ static int k1_i2c_probe(struct udevice *bus)
> >  {
> >  	struct k1_i2c_priv *priv = dev_get_priv(bus);
> >  	struct reset_ctl reset;
> > +	struct clk clk;
> >  	u32 speed;
> >  	int ret;
> >  
> > @@ -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)
> 
> I prefer to have something logged when any of these operations fail so
> it's easer to debug. With dev_* or log_* APIs, they should be optimized
> out and taken no extra space if they don't reach the logging level.

Oops, it seems drivers/clk/clk-uclass.c does some sort of logging already.
So please ignore this comment.

Regards,
Yao Zi

> But anyway, we currently have many drivers silently bail out in such
> cases, so this isn't a hard requirement.
> 
> Best regards,
> Yao Zi
> 
> >  		return ret;
> > -	}
> >  
> >  	priv->base = (void *)devfdt_get_addr_ptr(bus);
> >  
> > 
> > -- 
> > 2.55.0
 >