Re: [PATCH v3 2/2] i2c: Add Qualcomm CCI I2C driver

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Mon 06 Aug 21:30 PDT 2018, Vinod wrote:

> On 06-08-18, 11:45, Bjorn Andersson wrote:
> > On Mon 06 Aug 04:04 PDT 2018, Vinod Koul wrote:
> > [..]
> > > diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> > [..]
[..]
> > > +struct cci {
> > > +	struct device *dev;
> > > +	void __iomem *base;
> > > +	u32 irq;
> > 
> > Use unsigned int rather than a type of specific size
> 
> Any specific reason for that preference?
> 

Just that unsigned int is the type used in the irq api.

[..]
> > 
> > > +
> > > +	ret = cci_clock_set_rate(cci->nclocks, cci->clock,
> > > +				 cci->clock_freq, dev);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	ret = clk_bulk_prepare_enable(cci->nclocks, cci->clock);
> > 
> > It seems a little bit excessive to keep the clocks on while the driver
> > is probed, but this could be improved in a follow up patch...
> 
> okay but are they required for controller to be probed, I will check
> that part and update
> 

Right. So doing something similar to how we deal with clocks in i2c-qup
would probably make sense; we enable them during probe and then toggle
them using pm_runtime from there on.

Regards,
Bjorn