| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 27 Mar 2019 13:43:17 +0000
From: Laurentiu Tudor <laurentiu.tudor@....com>
To: Steven Price <steven.price@....com>,
"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
Ying Zhang <ying.zhang22455@....com>
CC: "upstream-release@...ux.nxdi.nxp.com"
<upstream-release@...ux.nxdi.nxp.com>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Leo Li <leoyang.li@....com>, dl-linux-imx <linux-imx@....com>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RESEND] i2c: imx: defer probing on dma channel request
Hello,
Just FYI, I'm still seeing issues with the dma driver compiled _out_,
trying to test i2c without dma support. I get the crash below in generic
driver code later in the boot process, debug is in progress.
P.S. This is seen on an NXP LS1043A chip.
[ 5.152697] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000010
[ 5.161483] Mem abort info:
[ 5.164311] ESR = 0x96000004
[ 5.167407] Exception class = DABT (current EL), IL = 32 bits
[ 5.173391] device_initialize: dev = ffff800027756808
[ 5.178446] usb 4-1: new SuperSpeed Gen 1 USB device number 2 using
xhci-hcd
[ 5.185489] SET = 0, FnV = 0
[ 5.188532] EA = 0, S1PTW = 0
[ 5.191676] Data abort info:
[ 5.194599] ISV = 0, ISS = 0x00000004
[ 5.198476] CM = 0, WnR = 0
[ 5.201485] [0000000000000010] user address but active_mm is swapper
[ 5.207894] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 5.213455] Modules linked in:
[ 5.216502] CPU: 0 PID: 18 Comm: kworker/0:1 Not tainted
5.1.0-rc2-next-20190327-00021-g7b1a4c075381-dirty #15
[ 5.226489] Hardware name: LS1043A RDB Board (DT)
[ 5.231189] Workqueue: events deferred_probe_work_func
[ 5.236317] pstate: a0000005 (NzCv daif -PAN -UAO)
[ 5.241098] pc : device_reorder_to_tail+0x13c/0x1b8
[ 5.245965] lr : device_reorder_to_tail+0xd8/0x1b8
[ 5.250743] sp : ffff000011823c70
[ 5.254046] x29: ffff000011823c70 x28: 0000000000000000
[ 5.259347] x27: ffff0000117abcd8 x26: ffff000011293a20
[ 5.264649] x25: ffff00001102a000 x24: ffff00001102a708
[ 5.269950] x23: ffff00001102a700 x22: ffff000010d28000
[ 5.275251] x21: ffff80007393b9a0 x20: fffffffffffffff8
[ 5.280552] x19: ffff80007393b8f0 x18: ffffffffffffffff
[ 5.285853] x17: 0000000000000002 x16: 0000000000000000
[ 5.291154] x15: ffff00001127d6c8 x14: 000000000000017e
[ 5.296454] x13: 0000000000000001 x12: 0000000000000000
[ 5.301755] x11: 0000000000000000 x10: 0000000000000980
[ 5.307055] x9 : ffff0000118238c0 x8 : ffff800073872560
[ 5.312356] x7 : ffff800073871d00 x6 : 0000000000000058
[ 5.317656] x5 : 000000000000000f x4 : 0000000000000100
[ 5.322957] x3 : 000080006a351000 x2 : 0991ec05b7534200
[ 5.328257] x1 : fffffffffffffff8 x0 : 0000000000000000
[ 5.333560] Process kworker/0:1 (pid: 18, stack limit =
0x(____ptrval____))
[ 5.340509] Call trace:
[ 5.342945] device_reorder_to_tail+0x13c/0x1b8
[ 5.347466] device_for_each_child+0x50/0xa8
[ 5.351725] device_reorder_to_tail+0xc4/0x1b8
[ 5.356157] device_pm_move_to_tail+0x34/0x50
[ 5.360502] deferred_probe_work_func+0x64/0xa0
[ 5.365023] process_one_work+0x1c8/0x320
[ 5.369021] worker_thread+0x234/0x428
[ 5.372761] kthread+0xf4/0x120
[ 5.375892] ret_from_fork+0x10/0x18
[ 5.379458] Code: 911c2318 911c02f7 d0004e19 b4000274 (f9400e84)
[ 5.385541] ---[ end trace ab4b151d346a8d41 ]---
---
Best Regards, Laurentiu
On 25.03.2019 19:12, Steven Price wrote:
> On 25/03/2019 15:30, laurentiu.tudor@....com wrote:
>> From: Laurentiu Tudor <laurentiu.tudor@....com>
>>
>> If the dma controller is not yet probed, defer i2c probe.
>> The error path in probe was slightly modified (no functional change)
>
> There is a functional change for cases like:
>
>> ret = pm_runtime_get_sync(&pdev->dev);
>> if (ret < 0)
>> goto rpm_disable;
>
> ...as rpm_disable will no longer fall through to the call of
> clk_disable_unprepare().
>
>> to avoid triggering this WARN_ON():
>> "cg-pll0-div1 already disabled
>> WARNING: CPU: 1 PID: 1 at drivers/clk/clk.c:828 clk_core_disable+0xa8/0xb0"
>
> I'm also not sure how this WARN_ON was hit. i2c_imx_probe() calls
> clk_prepare_enable() which should increment the reference count. So it
> should always be possible to decrememt the enable_count. What am I missing?
>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@....com>
>> ---
>> drivers/i2c/busses/i2c-imx.c | 21 +++++++++++++--------
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
>> index 42fed40198a0..4e34b1572756 100644
>> --- a/drivers/i2c/busses/i2c-imx.c
>> +++ b/drivers/i2c/busses/i2c-imx.c
>> @@ -1111,7 +1111,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
>> pdev->name, i2c_imx);
>> if (ret) {
>> dev_err(&pdev->dev, "can't claim irq %d\n", irq);
>> - goto clk_disable;
>> + clk_disable_unprepare(i2c_imx->clk);
>> + return ret;
>> }
>>
>> /* Init queue */
>> @@ -1161,19 +1162,25 @@ static int i2c_imx_probe(struct platform_device *pdev)
>> pm_runtime_mark_last_busy(&pdev->dev);
>> pm_runtime_put_autosuspend(&pdev->dev);
>>
>> + /* Init DMA config if supported */
>> + ret = i2c_imx_dma_request(i2c_imx, phy_addr);
>> + if (ret) {
>> + if (ret != -EPROBE_DEFER)
>> + dev_info(&pdev->dev, "can't use DMA, using PIO instead.\n");
>> + else
>> + goto del_adapter;
>> + }
>> +
>
> This can be simplified by reversing the condition:
>
> if (ret) {
> if (ret == -EPROBE_DEFER)
> goto del_adapter;
> dev_info();
> }
>
> or even:
>
> if (ret == -EPROBE_DEFER)
> goto del_adapter;
> else if (ret)
> dev_info();
>
>> dev_dbg(&i2c_imx->adapter.dev, "claimed irq %d\n", irq);
>> dev_dbg(&i2c_imx->adapter.dev, "device resources: %pR\n", res);
>> dev_dbg(&i2c_imx->adapter.dev, "adapter name: \"%s\"\n",
>> i2c_imx->adapter.name);
>>
>> - /* Init DMA config if supported */
>> - ret = i2c_imx_dma_request(i2c_imx, phy_addr);
>> - if (ret < 0)
>> - goto clk_notifier_unregister;
>> -
>> dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
>> return 0; /* Return OK */
>>
>> +del_adapter:
>> + i2c_del_adapter(&i2c_imx->adapter);
>
> This looks like a separate fix (previously the call to
> i2c_add_numbered_adapter() was not undone in case of later errors). It
> worth spelling this out in the commit message.
>
> Thanks,
>
> Steve
>
>> clk_notifier_unregister:
>> clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
>> rpm_disable:
>> @@ -1182,8 +1189,6 @@ static int i2c_imx_probe(struct platform_device *pdev)
>> pm_runtime_set_suspended(&pdev->dev);
>> pm_runtime_dont_use_autosuspend(&pdev->dev);
>>
>> -clk_disable:
>> - clk_disable_unprepare(i2c_imx->clk);
>> return ret;
>> }
>>
>>
>
Powered by blists - more mailing lists