| Message ID | 20241127083818.2108201-1-carlos.song@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] i2c: imx: support DMA defer probing | expand |
Hello Carlos, On 27.11.24 09:38, carlos.song@nxp.com wrote: > From: Carlos Song <carlos.song@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. Please try to address open questions before sending new versions of the patch set. Otherwise, it's difficult to follow the conversation. Did you see my question[1] on your v2: | Wouldn't this break probe for all i2c-imx users who have CONFIG_IMX_SDMA | disabled? | | Also I am wondering on what kernel version and what configuration | (CONFIG_I2C_IMX=?, CONFIG_IMX_SDMA=?) you have that made you run into | this situation. | | I'd have expected that with fw_devlink enabled, the I2C controller wouldn't | be probed before the DMA provider is available. [1]: https://lore.kernel.org/all/19a43db4-db5c-4638-9778-d94fb571a206@pengutronix.de/ Thanks, Ahma > > Signed-off-by: Carlos Song <carlos.song@nxp.com> > Signed-off-by: Clark Wang <xiaoning.wang@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; > int ret; > > dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); > if (!dma) > - return; > + return -ENOMEM; > > dma->chan_tx = dma_request_chan(dev, "tx"); > if (IS_ERR(dma->chan_tx)) { > @@ -452,7 +451,7 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, > dev_info(dev, "using %s (tx) and %s (rx) for DMA transfers\n", > dma_chan_name(dma->chan_tx), dma_chan_name(dma->chan_rx)); > > - return; > + return 0; > > fail_rx: > dma_release_channel(dma->chan_rx); > @@ -460,6 +459,8 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, > dma_release_channel(dma->chan_tx); > fail_al: > devm_kfree(dev, dma); > + > + return ret; > } > > static void i2c_imx_dma_callback(void *arg) > @@ -1803,6 +1804,23 @@ static int i2c_imx_probe(struct platform_device *pdev) > if (ret == -EPROBE_DEFER) > goto clk_notifier_unregister; > > + /* > + * Init DMA config if supported, -ENODEV means DMA not enabled at > + * this platform, that is not a real error, so just remind "only > + * PIO mode is used". If DMA is enabled, but meet error when request > + * DMA channel, error should be showed in probe error log. PIO mode > + * should be available regardless of DMA. > + */ > + ret = i2c_imx_dma_request(i2c_imx, phy_addr); > + if (ret) { > + if (ret == -EPROBE_DEFER) > + goto clk_notifier_unregister; > + else if (ret == -ENODEV) > + dev_dbg(&pdev->dev, "Only use PIO mode\n"); > + else > + dev_err_probe(&pdev->dev, ret, "Failed to setup DMA, only use PIO mode\n"); > + } > + > /* Add I2C adapter */ > ret = i2c_add_numbered_adapter(&i2c_imx->adapter); > if (ret < 0) > @@ -1817,9 +1835,6 @@ static int i2c_imx_probe(struct platform_device *pdev) > i2c_imx->adapter.name); > dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n"); > > - /* Init DMA config if supported */ > - i2c_imx_dma_request(i2c_imx, phy_addr); > - > return 0; /* Return OK */ > > clk_notifier_unregister:
On 27.11.2024 16:38:18, carlos.song@nxp.com wrote: > From: Carlos Song <carlos.song@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@nxp.com> > Signed-off-by: Clark Wang <xiaoning.wang@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? > 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
> -----Original Message----- > From: Ahmad Fatoum <a.fatoum@pengutronix.de> > Sent: Wednesday, November 27, 2024 4:38 PM > To: Carlos Song <carlos.song@nxp.com>; mkl@pengutronix.de; Frank Li > <frank.li@nxp.com>; o.rempel@pengutronix.de; kernel@pengutronix.de; > andi.shyti@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; > festevam@gmail.com > Cc: imx@lists.linux.dev; linux-i2c@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: [EXT] Re: [PATCH v4] i2c: imx: support DMA defer probing > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > Hello Carlos, > > On 27.11.24 09:38, carlos.song@nxp.com wrote: > > From: Carlos Song <carlos.song@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. > > Please try to address open questions before sending new versions of the patch > set. Otherwise, it's difficult to follow the conversation. > > Did you see my question[1] on your v2: > Hi, thank you so much! So sorry about it... I missed it yesterday. I will answer your question[1] in this mail. > | Wouldn't this break probe for all i2c-imx users who have > | CONFIG_IMX_SDMA disabled? > | I have tested i2c probe at IMX and LS platform when DMA disabled, it won't break i2c-imx probe. When require DMA channel in i2c_imx_dma_request, find no devices and return -ENODEV, as you see at V4 patch, it will continue to probe and work in PIO mode. I2C adapter should keep available whatever DMA mode is or isn't enabled. > | Also I am wondering on what kernel version and what configuration > | (CONFIG_I2C_IMX=?, CONFIG_IMX_SDMA=?) you have that made you run into > | this situation. > | I want to correct something, these code about DMA in i2c-imx.c is for eDMA not for SDMA. For eDMA mode, I have tested this patch at layerscape-1043 platform. My patch is based on cfba9f07a1d6 (tag: next-20241122, origin/master, origin/HEAD). Test log is : No apply this patch: CONFIG_I2C_IMX=y CONFIG_FSL_EDMA=y root@ls1043ardb:~# dmesg | grep i2c [ 1.162053] i2c i2c-0: IMX I2C adapter registered [ 1.166826] i2c i2c-0: using dma0chan16 (tx) and dma0chan17 (rx) for DMA transfers [ 4.722057] i2c_dev: i2c /dev entries driver Not apply the patch: CONFIG_I2C_IMX=y CONFIG_FSL_EDMA=m root@ls1043ardb:~# dmesg | grep i2c [ 1.166381] i2c i2c-0: IMX I2C adapter registered [ 4.719226] i2c_dev: i2c /dev entries driver (result shows i2c not enabled the eDMA mode) root@ls1043ardb:~# i2cdetect -y -l i2c-0 i2c 2180000.i2c I2C adapter root@ls1043ardb:~# i2cdetect -y 0 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 08 -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: UU -- -- -- -- -- -- -- -- -- -- -- UU -- -- -- 50: -- UU UU UU -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- 69 -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- After apply the patch: CONFIG_I2C_IMX=y CONFIG_FSL_EDMA=m root@ls1043ardb:~# root@ls1043ardb:~# dmesg | grep i2c [ 4.697046] i2c_dev: i2c /dev entries driver [ 7.304142] imx-i2c 2180000.i2c: using dma0chan16 (tx) and dma0chan17 (rx) for DMA transfers [ 7.313532] i2c i2c-0: IMX I2C adapter registered (result shows i2c probed after eDMA module installed) root@ls1043ardb:~# root@ls1043ardb:~# i2cdetect -y -l i2c-0 i2c 2180000.i2c I2C adapter root@ls1043ardb:~# i2cdetect -y 0 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 08 -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: UU -- -- -- -- -- -- -- -- -- -- -- UU -- -- -- 50: -- UU UU UU -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- 69 -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- > | I'd have expected that with fw_devlink enabled, the I2C controller > | wouldn't be probed before the DMA provider is available. > This is a legacy patch, it has been in our local tree for a long time. The related history is relatively vague. I reproduced the problem and found this patch is effective, so I referred the community patch and legacy patch to rewrite the commit log(I am not sure if this would happened in some cases so I kept this information). Now it seems that these descriptions are redundant. I should completely removed this in the commit log: 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". [1]: https://lore.kernel.org/all/19a43db4-db5c-4638-9778-d94fb571a206@pengutronix.de/ [2]:https://lore.kernel.org/all/153e8e36-7b0e-4379-9cc3-6dacb5d705be@pengutronix.de/ > > > > Signed-off-by: Carlos Song <carlos.song@nxp.com> > > Signed-off-by: Clark Wang <xiaoning.wang@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; > > int ret; > > > > dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); > > if (!dma) > > - return; > > + return -ENOMEM; > > > > dma->chan_tx = dma_request_chan(dev, "tx"); > > if (IS_ERR(dma->chan_tx)) { > > @@ -452,7 +451,7 @@ static void i2c_imx_dma_request(struct > imx_i2c_struct *i2c_imx, > > dev_info(dev, "using %s (tx) and %s (rx) for DMA transfers\n", > > dma_chan_name(dma->chan_tx), > > dma_chan_name(dma->chan_rx)); > > > > - return; > > + return 0; > > > > fail_rx: > > dma_release_channel(dma->chan_rx); > > @@ -460,6 +459,8 @@ static void i2c_imx_dma_request(struct > imx_i2c_struct *i2c_imx, > > dma_release_channel(dma->chan_tx); > > fail_al: > > devm_kfree(dev, dma); > > + > > + return ret; > > } > > > > static void i2c_imx_dma_callback(void *arg) @@ -1803,6 +1804,23 @@ > > static int i2c_imx_probe(struct platform_device *pdev) > > if (ret == -EPROBE_DEFER) > > goto clk_notifier_unregister; > > > > + /* > > + * Init DMA config if supported, -ENODEV means DMA not enabled at > > + * this platform, that is not a real error, so just remind "only > > + * PIO mode is used". If DMA is enabled, but meet error when request > > + * DMA channel, error should be showed in probe error log. PIO mode > > + * should be available regardless of DMA. > > + */ > > + ret = i2c_imx_dma_request(i2c_imx, phy_addr); > > + if (ret) { > > + if (ret == -EPROBE_DEFER) > > + goto clk_notifier_unregister; > > + else if (ret == -ENODEV) > > + dev_dbg(&pdev->dev, "Only use PIO mode\n"); > > + else > > + dev_err_probe(&pdev->dev, ret, "Failed to setup > DMA, only use PIO mode\n"); > > + } > > + > > /* Add I2C adapter */ > > ret = i2c_add_numbered_adapter(&i2c_imx->adapter); > > if (ret < 0) > > @@ -1817,9 +1835,6 @@ static int i2c_imx_probe(struct platform_device > *pdev) > > i2c_imx->adapter.name); > > dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n"); > > > > - /* Init DMA config if supported */ > > - i2c_imx_dma_request(i2c_imx, phy_addr); > > - > > return 0; /* Return OK */ > > > > clk_notifier_unregister: > > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > http://www.pen/ > gutronix.de%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C1acf840d499f > 49a7872408dd0ebedc39%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > %7C638682935131084746%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&s > data=Y9Qn9XEk15yu4CespwsNu6hl3%2FqfNTvEeOn4ZvnGxbo%3D&reserved=0 > | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
> -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: Wednesday, November 27, 2024 4:43 PM > To: Carlos Song <carlos.song@nxp.com> > Cc: Frank Li <frank.li@nxp.com>; o.rempel@pengutronix.de; > kernel@pengutronix.de; andi.shyti@kernel.org; shawnguo@kernel.org; > s.hauer@pengutronix.de; festevam@gmail.com; linux-i2c@vger.kernel.org; > imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org > Subject: [EXT] Re: [PATCH v4] i2c: imx: support DMA defer probing > > On 27.11.2024 16:38:18, carlos.song@nxp.com wrote: > > From: Carlos Song <carlos.song@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@nxp.com> > > Signed-off-by: Clark Wang <xiaoning.wang@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 |
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; int ret; dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); if (!dma) - return; + return -ENOMEM; dma->chan_tx = dma_request_chan(dev, "tx"); if (IS_ERR(dma->chan_tx)) { @@ -452,7 +451,7 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, dev_info(dev, "using %s (tx) and %s (rx) for DMA transfers\n", dma_chan_name(dma->chan_tx), dma_chan_name(dma->chan_rx)); - return; + return 0; fail_rx: dma_release_channel(dma->chan_rx); @@ -460,6 +459,8 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, dma_release_channel(dma->chan_tx); fail_al: devm_kfree(dev, dma); + + return ret; } static void i2c_imx_dma_callback(void *arg) @@ -1803,6 +1804,23 @@ static int i2c_imx_probe(struct platform_device *pdev) if (ret == -EPROBE_DEFER) goto clk_notifier_unregister; + /* + * Init DMA config if supported, -ENODEV means DMA not enabled at + * this platform, that is not a real error, so just remind "only + * PIO mode is used". If DMA is enabled, but meet error when request + * DMA channel, error should be showed in probe error log. PIO mode + * should be available regardless of DMA. + */ + ret = i2c_imx_dma_request(i2c_imx, phy_addr); + if (ret) { + if (ret == -EPROBE_DEFER) + goto clk_notifier_unregister; + else if (ret == -ENODEV) + dev_dbg(&pdev->dev, "Only use PIO mode\n"); + else + dev_err_probe(&pdev->dev, ret, "Failed to setup DMA, only use PIO mode\n"); + } + /* Add I2C adapter */ ret = i2c_add_numbered_adapter(&i2c_imx->adapter); if (ret < 0) @@ -1817,9 +1835,6 @@ static int i2c_imx_probe(struct platform_device *pdev) i2c_imx->adapter.name); dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n"); - /* Init DMA config if supported */ - i2c_imx_dma_request(i2c_imx, phy_addr); - return 0; /* Return OK */ clk_notifier_unregister: