linux-kernel - Re: [PATCH] i2c: cadence: Clear HOLD bit before xfer

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHTX3dLA1oF2v6FRgeL9kMExMXxGz=7LJBM3oo9Zf1CA2HfQFA@mail.gmail.com>
Date:   Wed, 23 Jun 2021 10:46:54 +0200
From:   Michal Simek <monstr@...str.eu>
To:     Raviteja Narayanam <raviteja.narayanam@...inx.com>
Cc:     linux-i2c@...r.kernel.org, git <git@...inx.com>,
        linux-arm <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c: cadence: Clear HOLD bit before xfer_size register
 rolls over

út 24. 11. 2020 v 8:48 odesílatel Raviteja Narayanam
<raviteja.narayanam@...inx.com> napsal:
>
> On Xilinx zynq SOC if the delay between address register write and
> control register write in cdns_mrecv function is more, the xfer size
> register rolls over and controller is stuck. This is an IP bug and

and the controller

> is resolved in later versions of IP.
>
> To avoid this scenario, disable the interrupts on the current processor
> core between the two register writes and enable them later. This can
> help achieve the timing constraint.
>
> Signed-off-by: Raviteja Narayanam <raviteja.narayanam@...inx.com>
> ---
>  drivers/i2c/busses/i2c-cadence.c | 48 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
> index e4b7f2a..81b7c45 100644
> --- a/drivers/i2c/busses/i2c-cadence.c
> +++ b/drivers/i2c/busses/i2c-cadence.c
> @@ -578,6 +578,11 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>  {
>         unsigned int ctrl_reg;
>         unsigned int isr_status;
> +       unsigned long flags;
> +       bool hold_clear = false;
> +       bool irq_save = false;
> +
> +       u32 addr;
>
>         id->p_recv_buf = id->p_msg->buf;
>         id->recv_count = id->p_msg->len;
> @@ -618,14 +623,43 @@ static void cdns_i2c_mrecv(struct cdns_i2c *id)
>                 cdns_i2c_writereg(id->recv_count, CDNS_I2C_XFER_SIZE_OFFSET);
>         }
>
> -       /* Set the slave address in address register - triggers operation */
> -       cdns_i2c_writereg(id->p_msg->addr & CDNS_I2C_ADDR_MASK,
> -                                               CDNS_I2C_ADDR_OFFSET);
> -       /* Clear the bus hold flag if bytes to receive is less than FIFO size */
> +       /* Determine hold_clear based on number of bytes to receive and hold flag */
>         if (!id->bus_hold_flag &&
> -               ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
> -               (id->recv_count <= CDNS_I2C_FIFO_DEPTH))
> -                       cdns_i2c_clear_bus_hold(id);
> +           ((id->p_msg->flags & I2C_M_RECV_LEN) != I2C_M_RECV_LEN) &&
> +           (id->recv_count <= CDNS_I2C_FIFO_DEPTH)) {
> +               if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HOLD) {
> +                       hold_clear = true;
> +                       if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT)
> +                               irq_save = true;
> +               }
> +       }
> +
> +       addr = id->p_msg->addr;
> +       addr &= CDNS_I2C_ADDR_MASK;
> +
> +       if (hold_clear) {
> +               ctrl_reg = cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & ~CDNS_I2C_CR_HOLD;
> +               /*
> +                * In case of Xilinx Zynq SOC, clear the HOLD bit before transfer size
> +                * register reaches '0'. This is an IP bug which causes transfer size
> +                * register overflow to 0xFF. To satisfy this timing requirement,
> +                * disable the interrupts on current processor core between register
> +                * writes to slave address register and control register.
> +                */
> +               if (irq_save)
> +                       local_irq_save(flags);
> +
> +               cdns_i2c_writereg(addr, CDNS_I2C_ADDR_OFFSET);
> +               cdns_i2c_writereg(ctrl_reg, CDNS_I2C_CR_OFFSET);
> +               /* Read it back to avoid bufferring and make sure write happens */
> +               cdns_i2c_readreg(CDNS_I2C_CR_OFFSET);
> +
> +               if (irq_save)
> +                       local_irq_restore(flags);
> +       } else {
> +               cdns_i2c_writereg(addr, CDNS_I2C_ADDR_OFFSET);
> +       }
> +
>         cdns_i2c_writereg(CDNS_I2C_ENABLED_INTR_MASK, CDNS_I2C_IER_OFFSET);
>  }

Unfortunately we can't do anything with this on the Zynq platform. It
is better than nothing.
ZynqMP is not affected.

Acked-by: Michal Simek <michal.simek@...inx.com>

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs