Re: [PATCH v4] i2c: imx: support DMA defer probing

From: Carlos Song
Date: Wed Nov 27 2024 - 05:49:26 EST

Next message: Sasha Finkelstein: "Re: [PATCH 1/4] dt-bindings: input: touchscreen: Add Z2 controller"
Previous message: Parthiban.Veerasooran: "Re: [PATCH net v2 2/2] net: ethernet: oa_tc6: fix tx skb race condition between reference pointers"
In reply to: Marc Kleine-Budde: "Re: [PATCH v4] i2c: imx: support DMA defer probing"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Sent: Wednesday, November 27, 2024 4:43 PM
> To: Carlos Song <carlos.song@xxxxxxx>
> Cc: Frank Li <frank.li@xxxxxxx>; o.rempel@xxxxxxxxxxxxxx;
> kernel@xxxxxxxxxxxxxx; andi.shyti@xxxxxxxxxx; shawnguo@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: [EXT] Re: [PATCH v4] i2c: imx: support DMA defer probing
>
> On 27.11.2024 16:38:18, carlos.song@xxxxxxx wrote:
> > From: Carlos Song <carlos.song@xxxxxxx>
> >
> > Return -EPROBE_DEFER when dma_request_slave_channel() because DMA
> > driver have not ready yet.
> >
> > Move i2c_imx_dma_request() before registering I2C adapter to avoid
> > infinite loop of .probe() calls to the same driver, see "e8c220fac415
> > Revert "i2c: imx: improve the error handling in i2c_imx_dma_request()""
> > and "Documentation/driver-api/driver-model/driver.rst".
> >
> > Use CPU mode to avoid stuck registering i2c adapter when DMA resources
> > are unavailable.
> >
> > Signed-off-by: Carlos Song <carlos.song@xxxxxxx>
> > Signed-off-by: Clark Wang <xiaoning.wang@xxxxxxx>
> > ---
> > Change for V4:
> > - Output "Only use PIO mode" log in debug level when no DMA configure.
> > Change for V3:
> > - According to Marc's comment, remove error print when defer probe.
> > Add info log when DMA has not been enabled and add error log when
> > DMA error, both won't stuck the i2c adapter register, just for reminding,
> > i2c adapter is working only in PIO mode.
> > Change for V2:
> > - According to Frank's comments, wrap at 75 char and Simplify fix code
> > at i2c_imx_dma_request().
> > - Use strict patch check, fix alignment warning at
> > i2c_imx_dma_request()
> > ---
> > drivers/i2c/busses/i2c-imx.c | 31 +++++++++++++++++++++++--------
> > 1 file changed, 23 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx.c
> > b/drivers/i2c/busses/i2c-imx.c index 5ed4cb61e262..b11d66d56c55 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -397,17 +397,16 @@ static void i2c_imx_reset_regs(struct
> > imx_i2c_struct *i2c_imx) }
> >
> > /* Functions for DMA support */
> > -static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
> > - dma_addr_t phy_addr)
> > +static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
> > +dma_addr_t phy_addr)
> > {
> > struct imx_i2c_dma *dma;
> > struct dma_slave_config dma_sconfig;
> > - struct device *dev = &i2c_imx->adapter.dev;
> > + struct device *dev = i2c_imx->adapter.dev.parent;
>
> Why is this change needed? What's the purpose?
>
Because i2c_imx_dma_request has been moved to before i2c_add_numbered_adapter(&i2c_imx->adapter);
If we use &i2c_imx->adapter.dev, we will meet NULL pointer at here (arrow points to)

/* Functions for DMA support */
static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, dma_addr_t phy_addr)
{
struct imx_i2c_dma *dma;
struct dma_slave_config dma_sconfig;
struct device *dev = i2c_imx->adapter.dev.parent;
int ret;

----> dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
if (!dma)
return -ENOMEM;
log is that:

[ 1.159261] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 1.168089] Mem abort info:
[ 1.170889] ESR = 0x0000000096000044
[ 1.174649] EC = 0x25: DABT (current EL), IL = 32 bits
[ 1.179983] SET = 0, FnV = 0
[ 1.183044] EA = 0, S1PTW = 0
[ 1.186193] FSC = 0x04: level 0 translation fault
[ 1.191089] Data abort info:
[ 1.193975] ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
[ 1.199483] CM = 0, WnR = 1, TnD = 0, TagAccess = 0
[ 1.204554] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 1.209888] [0000000000000000] user address but active_mm is swapper
[ 1.216270] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP
[ 1.222566] Modules linked in:
[ 1.225634] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-06331-g5ddb25243333-dirty #1
[ 1.234464] Hardware name: LS1043A RDB Board (DT)
[ 1.239186] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 1.246182] pc : devres_add+0x54/0x7c
[ 1.249864] lr : devres_add+0x28/0x7c
[ 1.253541] sp : ffff80008003ba90
[ 1.256864] x29: ffff80008003ba90 x28: 0000000000000000 x27: 0000000000000013
[ 1.264039] x26: ffff1638043d1080 x25: ffff1638043d14b8 x24: ffff1638043d10f0
[ 1.271214] x23: 0000000000000000 x22: ffff163804267180 x21: ffff1638043d139c
[ 1.278388] x20: ffff163804267180 x19: ffff1638043d13a0 x18: 0000000000000006
[ 1.285563] x17: 0000000000000001 x16: 0000000000000000 x15: ffff80008003b6df
[ 1.292737] x14: ffffa378172b5f20 x13: 747365757165725f x12: 616d645f786d695f
[ 1.299912] x11: ffffa378172b5f20 x10: 000000000000009d x9 : 0000000000000000
[ 1.307087] x8 : ffff163804267200 x7 : 0000000000000000 x6 : 000000000000003f
[ 1.314261] x5 : 0000000000000040 x4 : ffff1638043d10f0 x3 : 0000000000000000
[ 1.321435] x2 : ffff163804267100 x1 : 0000000000000000 x0 : ffff1638043d139c
[ 1.328609] Call trace:
[ 1.331059] devres_add+0x54/0x7c
[ 1.334387] devm_kmalloc+0x90/0x100
[ 1.337977] i2c_imx_probe+0x48c/0x714
[ 1.341743] platform_probe+0x68/0xdc
[ 1.345419] really_probe+0xbc/0x2bc
[ 1.349007] __driver_probe_device+0x78/0x120
[ 1.353381] driver_probe_device+0x3c/0x154
[ 1.357581] __driver_attach+0x90/0x1a0
[ 1.361431] bus_for_each_dev+0x7c/0xe0
[ 1.365284] driver_attach+0x24/0x30
[ 1.368875] bus_add_driver+0xe4/0x208
[ 1.372640] driver_register+0x68/0x124
[ 1.376490] __platform_driver_register+0x24/0x30
[ 1.381214] i2c_adap_imx_init+0x1c/0x28
[ 1.385155] do_one_initcall+0x60/0x1d4
[ 1.389007] kernel_init_freeable+0x214/0x278
[ 1.393384] kernel_init+0x20/0x140
[ 1.396887] ret_from_fork+0x10/0x20
[ 1.400479] Code: aa1503e0 f9415c83 f9015c82 a9380e93 (f9000062)
[ 1.406600] ---[ end trace 0000000000000000 ]---
[ 1.411235] note: swapper/0[1] exited with irqs disabled
[ 1.416582] note: swapper/0[1] exited with preempt_count 1
[ 1.422102] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 1.429795] SMP: stopping secondary CPUs

[1] https://lore.kernel.org/all/20241127-attentive-orthodox-tuna-8f8af0-mkl@xxxxxxxxxxxxxx/

> > int ret;
> >
> > dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
>
> This means you bind the allocated memory to the parent device, not the i2c
> device anymore.
>
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung Nürnberg | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |

Next message: Sasha Finkelstein: "Re: [PATCH 1/4] dt-bindings: input: touchscreen: Add Z2 controller"
Previous message: Parthiban.Veerasooran: "Re: [PATCH net v2 2/2] net: ethernet: oa_tc6: fix tx skb race condition between reference pointers"
In reply to: Marc Kleine-Budde: "Re: [PATCH v4] i2c: imx: support DMA defer probing"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]