| Message ID | 20220117102504.90585-1-iivanov@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: bcm2835: Set clock-stretch timeout to 35ms | expand |
On 01-17 11:25, Ivan T. Ivanov wrote: > Date: Mon, 17 Jan 2022 11:25:04 +0100 > From: "Ivan T. Ivanov" <iivanov@suse.de> > To: Florian Fainelli <f.fainelli@gmail.com>, Ray Jui <rjui@broadcom.com>, > Scott Branden <sbranden@broadcom.com>, Nicolas Saenz Julienne > <nsaenz@kernel.org> > Cc: linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Phil > Elwell <phil@raspberrypi.org>, "Ivan T . Ivanov" <iivanov@suse.de> > Subject: [PATCH] i2c: bcm2835: Set clock-stretch timeout to 35ms > Message-Id: <20220117102504.90585-1-iivanov@suse.de> Tags: all arm i2c linux me ring rpi sent Hi, > > From: Phil Elwell <phil@raspberrypi.org> > > The BCM2835 I2C blocks have a register to set the clock-stretch > timeout - how long the device is allowed to hold SCL low - in bus > cycles. The current driver doesn't write to the register, therefore > the default value of 64 cycles is being used for all devices. > > Set the timeout to the value recommended for SMBus - 35ms. > > See: https://github.com/raspberrypi/linux/issues/3064 It will be nice if we have this fix upstream. Regards, Ivan > > Signed-off-by: Phil Elwell <phil@raspberrypi.org> > [iivanov@suse.de fix small coding style issue] > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de> > --- > drivers/i2c/busses/i2c-bcm2835.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c > index 37443edbf754..9cce0e15da32 100644 > --- a/drivers/i2c/busses/i2c-bcm2835.c > +++ b/drivers/i2c/busses/i2c-bcm2835.c > @@ -106,6 +106,7 @@ static int clk_bcm2835_i2c_set_rate(struct clk_hw *hw, unsigned long rate, > { > struct clk_bcm2835_i2c *div = to_clk_bcm2835_i2c(hw); > u32 redl, fedl; > + u32 clk_tout; > u32 divider = clk_bcm2835_i2c_calc_divider(rate, parent_rate); > > if (divider == -EINVAL) > @@ -129,6 +130,17 @@ static int clk_bcm2835_i2c_set_rate(struct clk_hw *hw, unsigned long rate, > bcm2835_i2c_writel(div->i2c_dev, BCM2835_I2C_DEL, > (fedl << BCM2835_I2C_FEDL_SHIFT) | > (redl << BCM2835_I2C_REDL_SHIFT)); > + > + /* > + * Set the clock stretch timeout to the SMBUs-recommended 35ms. > + */ > + if (rate > 0xffff * 1000 / 35) > + clk_tout = 0xffff; > + else > + clk_tout = 35 * rate / 1000; > + > + bcm2835_i2c_writel(div->i2c_dev, BCM2835_I2C_CLKT, clk_tout); > + > return 0; > } > > -- > 2.26.2 >
Hi Ivan, > The BCM2835 I2C blocks have a register to set the clock-stretch > timeout - how long the device is allowed to hold SCL low - in bus > cycles. The current driver doesn't write to the register, therefore > the default value of 64 cycles is being used for all devices. > > Set the timeout to the value recommended for SMBus - 35ms. By default, busses are I2C and not SMBus, so it looks like we should finally apply this patch which disables CLKT: http://patchwork.ozlabs.org/project/linux-i2c/patch/1519422151-6218-1-git-send-email-stefan.wahren@i2se.com/ If you really want the SMBus timeout applied, you can check for a "smbus" property in DT and then set CLKT accordingly in a seperate patch. Kind regards, Wolfram
> By default, busses are I2C and not SMBus, so it looks like we should > finally apply this patch which disables CLKT: > > http://patchwork.ozlabs.org/project/linux-i2c/patch/1519422151-6218-1-git-send-email-stefan.wahren@i2se.com/ I applied this now to for-current and added stable. That should avoid unwanted timeouts. > If you really want the SMBus timeout applied, you can check for a > "smbus" property in DT and then set CLKT accordingly in a seperate > patch. This still holds.
On 02-07 16:15, Wolfram Sang wrote: > Date: Mon, 7 Feb 2022 16:15:24 +0100 > From: Wolfram Sang <wsa@kernel.org> > To: "Ivan T. Ivanov" <iivanov@suse.de>, Florian Fainelli > <f.fainelli@gmail.com>, Ray Jui <rjui@broadcom.com>, Scott Branden > <sbranden@broadcom.com>, Nicolas Saenz Julienne <nsaenz@kernel.org>, > linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Phil > Elwell <phil@raspberrypi.org> > Subject: Re: [PATCH] i2c: bcm2835: Set clock-stretch timeout to 35ms > Message-ID: <YgE3jMkzisFZFHb5@shikoro> Tags: all arm i2c linux me ring rpi > > > > By default, busses are I2C and not SMBus, so it looks like we should > > finally apply this patch which disables CLKT: > > > > http://patchwork.ozlabs.org/project/linux-i2c/patch/1519422151-6218-1-git-send-email-stefan.wahren@i2se.com/ > > I applied this now to for-current and added stable. That should avoid > unwanted timeouts. Thanks! Unfortunately reporter went silent after I have prepared for him kernel package with _this_ patch [1], so it will be difficult to verify above change on my side. > > > If you really want the SMBus timeout applied, you can check for a > > "smbus" property in DT and then set CLKT accordingly in a seperate > > patch. > > This still holds. Sure, will keep it in mind. Regards, Ivan [1] https://bugzilla.suse.com/show_bug.cgi?id=1192976
diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c index 37443edbf754..9cce0e15da32 100644 --- a/drivers/i2c/busses/i2c-bcm2835.c +++ b/drivers/i2c/busses/i2c-bcm2835.c @@ -106,6 +106,7 @@ static int clk_bcm2835_i2c_set_rate(struct clk_hw *hw, unsigned long rate, { struct clk_bcm2835_i2c *div = to_clk_bcm2835_i2c(hw); u32 redl, fedl; + u32 clk_tout; u32 divider = clk_bcm2835_i2c_calc_divider(rate, parent_rate); if (divider == -EINVAL) @@ -129,6 +130,17 @@ static int clk_bcm2835_i2c_set_rate(struct clk_hw *hw, unsigned long rate, bcm2835_i2c_writel(div->i2c_dev, BCM2835_I2C_DEL, (fedl << BCM2835_I2C_FEDL_SHIFT) | (redl << BCM2835_I2C_REDL_SHIFT)); + + /* + * Set the clock stretch timeout to the SMBUs-recommended 35ms. + */ + if (rate > 0xffff * 1000 / 35) + clk_tout = 0xffff; + else + clk_tout = 35 * rate / 1000; + + bcm2835_i2c_writel(div->i2c_dev, BCM2835_I2C_CLKT, clk_tout); + return 0; }