Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1606203965-31595-1-git-send-email-raviteja.narayanam@xilinx.com>
In-Reply-To: <1606203965-31595-1-git-send-email-raviteja.narayanam@xilinx.com>
From:   Michal Simek <monstr@monstr.eu>
Date:   Wed, 23 Jun 2021 10:46:54 +0200
Message-ID: <CAHTX3dLA1oF2v6FRgeL9kMExMXxGz=7LJBM3oo9Zf1CA2HfQFA@mail.gmail.com>
Subject: Re: [PATCH] i2c: cadence: Clear HOLD bit before xfer_size register
 rolls over
To:     Raviteja Narayanam <raviteja.narayanam@xilinx.com>
Cc:     linux-i2c@vger.kernel.org, git <git@xilinx.com>,
        linux-arm <linux-arm-kernel@lists.infradead.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

=C3=BAt 24. 11. 2020 v 8:48 odes=C3=ADlatel Raviteja Narayanam
<raviteja.narayanam@xilinx.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@xilinx.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-ca=
dence.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 =3D false;
> +       bool irq_save =3D false;
> +
> +       u32 addr;
>
>         id->p_recv_buf =3D id->p_msg->buf;
>         id->recv_count =3D 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_OFFS=
ET);
>         }
>
> -       /* 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 h=
old flag */
>         if (!id->bus_hold_flag &&
> -               ((id->p_msg->flags & I2C_M_RECV_LEN) !=3D I2C_M_RECV_LEN)=
 &&
> -               (id->recv_count <=3D CDNS_I2C_FIFO_DEPTH))
> -                       cdns_i2c_clear_bus_hold(id);
> +           ((id->p_msg->flags & I2C_M_RECV_LEN) !=3D I2C_M_RECV_LEN) &&
> +           (id->recv_count <=3D CDNS_I2C_FIFO_DEPTH)) {
> +               if (cdns_i2c_readreg(CDNS_I2C_CR_OFFSET) & CDNS_I2C_CR_HO=
LD) {
> +                       hold_clear =3D true;
> +                       if (id->quirks & CDNS_I2C_BROKEN_HOLD_BIT)
> +                               irq_save =3D true;
> +               }
> +       }
> +
> +       addr =3D id->p_msg->addr;
> +       addr &=3D CDNS_I2C_ADDR_MASK;
> +
> +       if (hold_clear) {
> +               ctrl_reg =3D 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 t=
ransfer size
> +                * register overflow to 0xFF. To satisfy this timing requ=
irement,
> +                * disable the interrupts on current processor core betwe=
en 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 h=
appens */
> +               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@xilinx.com>

Thanks,
Michal

--=20
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