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

Carlos Song carlos.song at nxp.com
Wed Nov 27 02:48:55 PST 2024



> -----Original Message-----
> From: Marc Kleine-Budde <mkl at pengutronix.de>
> Sent: Wednesday, November 27, 2024 4:43 PM
> To: Carlos Song <carlos.song at nxp.com>
> Cc: Frank Li <frank.li at nxp.com>; o.rempel at pengutronix.de;
> kernel at pengutronix.de; andi.shyti at kernel.org; shawnguo at kernel.org;
> s.hauer at pengutronix.de; festevam at gmail.com; linux-i2c at vger.kernel.org;
> imx at lists.linux.dev; linux-arm-kernel at lists.infradead.org;
> linux-kernel at vger.kernel.org
> Subject: [EXT] Re: [PATCH v4] i2c: imx: support DMA defer probing
> 
> On 27.11.2024 16:38:18, carlos.song at nxp.com wrote:
> > From: Carlos Song <carlos.song at nxp.com>
> >
> > 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 at nxp.com>
> > Signed-off-by: Clark Wang <xiaoning.wang at nxp.com>
> > ---
> > 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@pengutronix.de/

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


More information about the linux-arm-kernel mailing list