Re: [PATCH 8/8] i2c: img-scb: verify support for requested bit rate

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

On 27/07/15 12:47, Sifan Naeem wrote:
> The requested bit rate can be outside the range supported by the driver.
> The maximum bit rate this driver supports at the moment is 400Khz.
> 
> Return -EINVAL if the bit rate is larger than 400khz.
> 
> Maximum speed supported by the driver can be increased to 1Mhz by
> adding support for "fast plus mode" in the future.
> 
> Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
> Cc: Stable kernel (v3.19+) <stable@vger.kernel.org>
> ---
>  drivers/i2c/busses/i2c-img-scb.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> index bbfee33..07a039c 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -1157,6 +1157,12 @@ static int img_i2c_init(struct img_i2c *i2c)
>  			break;
>  		}
>  	}
> +	if (i2c->bitrate > timing.max_bitrate) {
> +		dev_err(i2c->adap.dev.parent,
> +			"requested bitrate (%d) is higher than the max bitrate supported (%d)\n",
> +			 i2c->bitrate, timing.max_bitrate);
> +		return -EINVAL;
> +	}

The timing is only chosen if i2c->bitrate <= timing.max_bitrate, so
you'd only hit this case if none of the timings were valid, in which
case timing == timings[0], so when you print timing.max_bitrate it won't
be the max bitrate supported, it'll be the max bitrate of the first
timing in the array.

Anyway, I think the original intent of the DT provided clock-frequency
was more as a maximum bitrate. This was used with TZ1090 as a way to
limit the bitrate of the bus if some devices on the bus don't support
the full speed, e.g. we had an HDMI chip that would get confused at 400khz.

So would it be acceptable to change it to just clamp the bitrate to the
maximum rate supported, before the bitrate_khz calculation, such that if
you specified 1MHz in DT, it could safely fall back to 400KHz until the
driver supports the faster mode?

Cheers
James

>  
>  	/* Find the prescale that would give us that inc (approx delay = 0) */
>  	prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]