| Message ID | 1437997641-32575-9-git-send-email-sifan.naeem@imgtec.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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); >
Hi James, > 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? > I'll rework this, when the requested bit rate is larger than the maximum supported, set the bitrate down to the maximum supported before bitrate_khz is calculated. Thanks, Sifan > Cheers > James > > > > > /* Find the prescale that would give us that inc (approx delay = 0) */ > > prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz); > > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; + } /* Find the prescale that would give us that inc (approx delay = 0) */ prescale = SCB_OPT_INC * clk_khz / (256 * 16 * bitrate_khz);
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(+)