[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d50b2caa-4d3a-4c1d-986c-6fb3c0a2f850@ijzerbout.nl>
Date: Fri, 22 Nov 2024 22:29:27 +0100
From: Kees Bakker <kees@...erbout.nl>
To: Manikanta Guntupalli <manikanta.guntupalli@....com>, git@....com,
michal.simek@....com, andi.shyti@...nel.org,
linux-arm-kernel@...ts.infradead.org, linux-i2c@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: radhey.shyam.pandey@....com, srinivas.goud@....com,
shubhrajyoti.datta@....com, manion05gk@...il.com
Subject: Re: [PATCH 2/2] i2c: xiic: Add atomic transfer support
Op 11-10-2024 om 12:41 schreef Manikanta Guntupalli:
> Rework the read and write code paths in the driver to support operation
> in atomic contexts.
>
> Similar changes have been implemented in other drivers, including:
> commit 3a5ee18d2a32 ("i2c: imx: implement master_xfer_atomic callback")
> commit 445094c8a9fb ("i2c: exynos5: add support for atomic transfers")
> commit ede2299f7101 ("i2c: tegra: Support atomic transfers")
> commit fe402bd09049 ("i2c: meson: implement the master_xfer_atomic
> callback")
>
> Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@....com>
> ---
> drivers/i2c/busses/i2c-xiic.c | 245 +++++++++++++++++++++++++++++-----
> 1 file changed, 212 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 052bae4fc664..e5e9a4993bd4 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> [...]
> +static void xiic_recv_atomic(struct xiic_i2c *i2c)
> +{
> + while (xiic_rx_space(i2c)) {
Let's remind what xiic_rx_space is
#define xiic_rx_space(i2c) ((i2c)->rx_msg->len - (i2c)->rx_pos)
> + if (xiic_getreg32(i2c, XIIC_IISR_OFFSET) & XIIC_INTR_RX_FULL_MASK) {
> + if (!i2c->rx_msg) {
This check is suspicious. If i2c->rx_msg is NULL then the while
above already dereferenced a NULL pointer.
What is going on?
> [...]
> +}
> [...]
> +static void xiic_send_rem_atomic(struct xiic_i2c *i2c)
> +{
> + if (!i2c->tx_msg)
> + goto send_atomic_out;
> + while (xiic_tx_space(i2c)) {
> [...]
> + }
> +send_atomic_out:
> + if (i2c->nmsgs > 1) {
> + i2c->nmsgs--;
> + i2c->tx_msg++;
We can get here with i2c->tx_msg being NULL. See the first if
in the function.
> + __xiic_start_xfer(i2c);
> + } else {
> + xiic_irq_dis(i2c, XIIC_INTR_TX_HALF_MASK);
> + }
> }
> [...]
Powered by blists - more mailing lists