diff mbox

[v2,7/7] i2c: img-scb: verify support for requested bit rate

Message ID 1439567424-8094-8-git-send-email-sifan.naeem@imgtec.com
State Superseded
Headers show

Commit Message

Sifan Naeem Aug. 14, 2015, 3:50 p.m. UTC
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.

If the requested bit rate is larger than the maximum supported by the
driver, set the bitrate to the maximum supported before bitrate_khz is
calculated.

Maximum speed supported by the driver can be increased to 1Mhz by
adding support for "fast plus mode" in the future.

Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
Reviewed-by: James Hartley <james.hartley@imgtec.com>
---
 drivers/i2c/busses/i2c-img-scb.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

James Hogan Aug. 19, 2015, 9:17 p.m. UTC | #1
On Fri, Aug 14, 2015 at 04:50:24PM +0100, 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.
> 
> If the requested bit rate is larger than the maximum supported by the
> driver, set the bitrate to the maximum supported before bitrate_khz is
> calculated.
> 
> Maximum speed supported by the driver can be increased to 1Mhz by
> adding support for "fast plus mode" in the future.
> 
> Fixes: commit 27bce457d588 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
> Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
> Reviewed-by: James Hartley <james.hartley@imgtec.com>
> ---
>  drivers/i2c/busses/i2c-img-scb.c |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> index 6c21a7bd9a66..c16caaccc363 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -1138,9 +1138,6 @@ static int img_i2c_init(struct img_i2c *i2c)
>  	/* Fencing enabled by default. */
>  	i2c->need_wr_rd_fence = true;
>  
> -	bitrate_khz = i2c->bitrate / 1000;
> -	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
> -
>  	/* Determine what mode we're in from the bitrate */
>  	timing = timings[0];
>  	for (i = 0; i < ARRAY_SIZE(timings); i++) {
> @@ -1149,6 +1146,17 @@ static int img_i2c_init(struct img_i2c *i2c)
>  			break;
>  		}
>  	}
> +	if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
> +		dev_warn(i2c->adap.dev.parent,
> +			 "requested bitrate (%d) is higher than the max bitrate supported (%d)\n",

Technically both i2c->bitrate and timings[].max_bitrate are unsigned, so
%u would be more correct for both.

Otherwise:
Acked-by: James Hogan <james.hogan@imgtec.com>

Wolfram: are you happy to fix the %u thing when applying it, or would
you prefer me to submit a fixed patch (Sifan is off for a few weeks I
believe).

(Still need to look at the other changes...)

Cheers
James

> +			 i2c->bitrate,
> +			 timings[ARRAY_SIZE(timings) - 1].max_bitrate);
> +		timing = timings[ARRAY_SIZE(timings) - 1];
> +		i2c->bitrate = timing.max_bitrate;
> +	}
> +
> +	bitrate_khz = i2c->bitrate / 1000;
> +	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
>  
>  	/* Find the prescale that would give us that inc (approx delay = 0) */
>  	prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
> -- 
> 1.7.9.5
>
Wolfram Sang Aug. 20, 2015, 8:28 p.m. UTC | #2
> Wolfram: are you happy to fix the %u thing when applying it, or would
> you prefer me to submit a fixed patch (Sifan is off for a few weeks I
> believe).

If this is the only thing to be fixed, I can do it. If more things
accumulate, I'd prefer a new version.

Thanks,

   Wolfram
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 6c21a7bd9a66..c16caaccc363 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1138,9 +1138,6 @@  static int img_i2c_init(struct img_i2c *i2c)
 	/* Fencing enabled by default. */
 	i2c->need_wr_rd_fence = true;
 
-	bitrate_khz = i2c->bitrate / 1000;
-	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
-
 	/* Determine what mode we're in from the bitrate */
 	timing = timings[0];
 	for (i = 0; i < ARRAY_SIZE(timings); i++) {
@@ -1149,6 +1146,17 @@  static int img_i2c_init(struct img_i2c *i2c)
 			break;
 		}
 	}
+	if (i2c->bitrate > timings[ARRAY_SIZE(timings) - 1].max_bitrate) {
+		dev_warn(i2c->adap.dev.parent,
+			 "requested bitrate (%d) is higher than the max bitrate supported (%d)\n",
+			 i2c->bitrate,
+			 timings[ARRAY_SIZE(timings) - 1].max_bitrate);
+		timing = timings[ARRAY_SIZE(timings) - 1];
+		i2c->bitrate = timing.max_bitrate;
+	}
+
+	bitrate_khz = i2c->bitrate / 1000;
+	clk_khz = clk_get_rate(i2c->scb_clk) / 1000;
 
 	/* Find the prescale that would give us that inc (approx delay = 0) */
 	prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);