| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <758551c7-7fb3-4e10-32a7-7dd48a93b064@broadcom.com>
Date: Thu, 3 Jun 2021 10:06:10 -0700
From: Ray Jui <ray.jui@...adcom.com>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>, wsa@...nel.org,
rjui@...adcom.com, sbranden@...adcom.com,
bcm-kernel-feedback-list@...adcom.com
Cc: linux-i2c@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
Richard Laing <richard.laing@...iedtelesis.co.nz>
Subject: Re: [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
On 6/2/2021 10:25 PM, Chris Packham wrote:
> From: Richard Laing <richard.laing@...iedtelesis.co.nz>
>
> The bcm-iproc controller can put the SDA/SCL lines into bit-bang mode,
> make use of this to support i2c bus recovery.
>
> Signed-off-by: Richard Laing <richard.laing@...iedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ---
> Richard did most of the work on this. I'm just cleaning it up to get it
> upstream.
>
> Changes in v3:
> - drop const from bcm_iproc_recovery_info due to build error from the
> kernel test bot
>
> Changes in v2:
> - Incorporate feedback from Ray Jui
> - Move bcm_iproc_i2c_resume so it can be re-used to return the i2c bus
> to normal operation at the correct speed after a recovery.
> - Add iproc_i2c_lockup_recover() helper to only trigger recovery if sda
> is actually stuck
> - Use usleep_range() instead of udelay()
> - Cosmetic changes to register bit definitions
>
> drivers/i2c/busses/i2c-bcm-iproc.c | 176 +++++++++++++++++++++++++----
> 1 file changed, 151 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index cceaf69279a9..fdbb98a848e3 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -25,6 +25,7 @@
> #define CFG_OFFSET 0x00
> #define CFG_RESET_SHIFT 31
> #define CFG_EN_SHIFT 30
> +#define CFG_BIT_BANG_SHIFT 29
> #define CFG_SLAVE_ADDR_0_SHIFT 28
> #define CFG_M_RETRY_CNT_SHIFT 16
> #define CFG_M_RETRY_CNT_MASK 0x0f
> @@ -66,6 +67,12 @@
> #define S_FIFO_RX_THLD_SHIFT 8
> #define S_FIFO_RX_THLD_MASK 0x3f
>
> +#define M_BB_CTRL_OFFSET 0x14
> +#define M_BB_SMBCLK_IN_SHIFT 31
> +#define M_BB_SMBCLK_OUT_EN_SHIFT 30
> +#define M_BB_SMBDAT_IN_SHIFT 29
> +#define M_BB_SMBDAT_OUT_EN_SHIFT 28
> +
> #define M_CMD_OFFSET 0x30
> #define M_CMD_START_BUSY_SHIFT 31
> #define M_CMD_STATUS_SHIFT 25
> @@ -713,6 +720,147 @@ static void bcm_iproc_i2c_enable_disable(struct bcm_iproc_i2c_dev *iproc_i2c,
> iproc_i2c_wr_reg(iproc_i2c, CFG_OFFSET, val);
> }
>
> +static int bcm_iproc_i2c_resume(struct device *dev)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
> + int ret;
> + u32 val;
> +
> + /*
> + * Power domain could have been shut off completely in system deep
> + * sleep, so re-initialize the block here
> + */
> + ret = bcm_iproc_i2c_init(iproc_i2c);
> + if (ret)
> + return ret;
> +
> + /* configure to the desired bus speed */
> + val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
> + val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
> + val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
> + iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
> +
> + bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> +
> + return 0;
> +}
> +
> +static void bcm_iproc_i2c_prepare_recovery(struct i2c_adapter *adap)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> + u32 tmp;
> +
> + dev_dbg(iproc_i2c->device, "Prepare recovery\n");
> +
> + /* Disable interrupts */
> + writel(0, iproc_i2c->base + IE_OFFSET);
> + readl(iproc_i2c->base + IE_OFFSET);
> + synchronize_irq(iproc_i2c->irq);
> +
> + /* Place controller in reset */
> + tmp = readl(iproc_i2c->base + CFG_OFFSET);
> + tmp |= BIT(CFG_RESET_SHIFT);
> + writel(tmp, iproc_i2c->base + CFG_OFFSET);
> + usleep_range(100, 200);
> +
> + /* Switch to bit-bang mode */
> + tmp = readl(iproc_i2c->base + CFG_OFFSET);
> + tmp |= BIT(CFG_BIT_BANG_SHIFT);
> + writel(tmp, iproc_i2c->base + CFG_OFFSET);
> + usleep_range(100, 200);
> +}
> +
> +static void bcm_iproc_i2c_unprepare_recovery(struct i2c_adapter *adap)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> + u32 tmp;
> +
> + /* Switch to normal mode */
> + tmp = readl(iproc_i2c->base + CFG_OFFSET);
> + tmp &= ~BIT(CFG_BIT_BANG_SHIFT);
> + writel(tmp, iproc_i2c->base + CFG_OFFSET);
> + usleep_range(100, 200);
> +
> + bcm_iproc_i2c_resume(iproc_i2c->device);
> +
> + dev_dbg(iproc_i2c->device, "Recovery complete\n");
> +}
> +
> +static int bcm_iproc_i2c_get_scl(struct i2c_adapter *adap)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> + u32 tmp;
> +
> + tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +
> + return !!(tmp & BIT(M_BB_SMBCLK_IN_SHIFT));
> +}
> +
> +static void bcm_iproc_i2c_set_scl(struct i2c_adapter *adap, int val)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> + u32 tmp;
> +
> + tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> + if (val)
> + tmp |= BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
> + else
> + tmp &= ~BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
> +
> + writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
> +}
> +
> +static void bcm_iproc_i2c_set_sda(struct i2c_adapter *adap, int val)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> + u32 tmp;
> +
> + tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> + if (val)
> + tmp |= BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
> + else
> + tmp &= ~BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
> +
> + writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
> +}
> +
> +static int bcm_iproc_i2c_get_sda(struct i2c_adapter *adap)
> +{
> + struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> + u32 tmp;
> +
> + tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +
> + return !!(tmp & BIT(M_BB_SMBDAT_IN_SHIFT));
> +}
> +
> +/* Check if bus lockup occurred, and invoke recovery if so. */
> +static void iproc_i2c_lockup_recover(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> + /*
> + * assume bus lockup if SDA line is low;
> + * note that there is no need to switch to
> + * bit-bang mode for this check.
> + */
> + if (!bcm_iproc_i2c_get_sda(&iproc_i2c->adapter)) {
> + /* locked up - invoke i2c bus recovery. */
> + int ret = i2c_recover_bus(&iproc_i2c->adapter);
> +
> + if (ret)
> + dev_err(iproc_i2c->device, "bus recovery: error %d\n", ret);
> + }
> +}
> +
> +static struct i2c_bus_recovery_info bcm_iproc_recovery_info = {
> + .recover_bus = i2c_generic_scl_recovery,
> + .prepare_recovery = bcm_iproc_i2c_prepare_recovery,
> + .unprepare_recovery = bcm_iproc_i2c_unprepare_recovery,
> + .set_scl = bcm_iproc_i2c_set_scl,
> + .get_scl = bcm_iproc_i2c_get_scl,
> + .set_sda = bcm_iproc_i2c_set_sda,
> + .get_sda = bcm_iproc_i2c_get_sda,
> +};
> +
> static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
> struct i2c_msg *msg)
> {
> @@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
> /* flush both TX/RX FIFOs */
> val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
> iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> + iproc_i2c_lockup_recover(iproc_i2c);
> return -ETIMEDOUT;
> }
>
> @@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
> /* flush both TX/RX FIFOs */
> val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
> iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> + iproc_i2c_lockup_recover(iproc_i2c);
> return ret;
> }
>
> @@ -1111,6 +1261,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
> of_node_full_name(iproc_i2c->device->of_node));
> adap->algo = &bcm_iproc_algo;
> adap->quirks = &bcm_iproc_i2c_quirks;
> + adap->bus_recovery_info = &bcm_iproc_recovery_info;
> adap->dev.parent = &pdev->dev;
> adap->dev.of_node = pdev->dev.of_node;
>
> @@ -1159,31 +1310,6 @@ static int bcm_iproc_i2c_suspend(struct device *dev)
> return 0;
> }
>
> -static int bcm_iproc_i2c_resume(struct device *dev)
> -{
> - struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
> - int ret;
> - u32 val;
> -
> - /*
> - * Power domain could have been shut off completely in system deep
> - * sleep, so re-initialize the block here
> - */
> - ret = bcm_iproc_i2c_init(iproc_i2c);
> - if (ret)
> - return ret;
> -
> - /* configure to the desired bus speed */
> - val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
> - val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
> - val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
> - iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
> -
> - bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> -
> - return 0;
> -}
> -
> static const struct dev_pm_ops bcm_iproc_i2c_pm_ops = {
> .suspend_late = &bcm_iproc_i2c_suspend,
> .resume_early = &bcm_iproc_i2c_resume
>
This latest change looks good to me. Thanks a lot!
Acked-by: Ray Jui <ray.jui@...adcom.com>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4194 bytes)
Powered by blists - more mailing lists