Re: [PATCH v4] i2c: busses: i2c-bcm2835: limits cdiv to allowed values

[Silvan Wicki <linux_wi-ADq4ffItWIY@public.gmane.org>]

On Mon, Jun 08, 2015 at 07:48:25PM -0600, Stephen Warren wrote:
> On 06/06/2015 12:18 PM, Silvan Wicki wrote:
> > Adds: make sure bits 16-31 of DIV register are always 0
> > Adds: assume minimal divider of 2 if divider resulted in 0
> >       (bcm2835 sets divider to 32768 if cdiv is set to 0)
> > 
> > See page 33/34 of BCM2835-ARM-Peripherals.pdf for the DIV register.
> > https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> 
> > diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
> 
> > @@ -252,12 +255,28 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
> 
> > +	if (divider > BCM2835_I2C_CDIV_MAX) {
> > +		dev_warn(&pdev->dev,
> > +			 "Clock too slow, falling back to min clock speed\n");
> > +		divider = BCM2835_I2C_CDIV_MAX;
> > +	}
> >  	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
> 
> Shouldn't this be an error? i.e. dev_err() not dev_warn(), and return an
> error code.
> 
> If the divider that's required to reach the requested clock is higher
> than the HW can implement, that means the actual clock will be faster
> than the requested clock. Presumably there's a reason the specific slow
> rate was requested, and if we exceed it, there will be problems.

i searched for similar things in other i2c drivers and found that
i2c-bcm-iproc, i2c-digicolor do a dev_err. so it seems like dev_err
is the way it should be implemented. i'll update that.