Hi all,
I see the following crash on a imx8mm board implementation. I cannot
find a reason for this at the moment. This can be reproduced with the
imx8mm-kontron-n801x-s dtb.Has anyone seen the same issue?
The position in the code is here:
https://elixir.bootlin.com/linux/latest/source/drivers/usb/chipidea/usbmisc_imx.c#L819
[ 1.489344] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000098
[ 1.498170] Mem abort info:
[ 1.500966] ESR = 0x96000044
[ 1.504030] EC = 0x25: DABT (current EL), IL = 32 bits
[ 1.509356] SET = 0, FnV = 0
[ 1.512416] EA = 0, S1PTW = 0
[ 1.515569] FSC = 0x04: level 0 translation fault
[ 1.520458] Data abort info:
[ 1.523349] ISV = 0, ISS = 0x00000044
[ 1.527196] CM = 0, WnR = 1
[ 1.530176] [0000000000000098] user address but active_mm is swapper
[ 1.536544] Internal error: Oops: 96000044 [#1] PREEMPT SMP
[ 1.542125] Modules linked in:
[ 1.545190] CPU: 3 PID: 7 Comm: kworker/u8:0 Not tainted 5.14.0-dirty #3
[ 1.551901] Hardware name: Kontron i.MX8MM N801X S (DT)
[ 1.557133] Workqueue: events_unbound deferred_probe_work_func
[ 1.562984] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[ 1.568998] pc : imx7d_charger_detection+0x3f0/0x510
[ 1.573973] lr : imx7d_charger_detection+0x22c/0x510
[ 1.578947] sp : ffff800011f1b5d0
[ 1.582266] x29: ffff800011f1b5d0 x28: 0000000000000000 x27: 0000000000000000
[ 1.589416] x26: ffff000000294c10 x25: ffff800011a6d678 x24: ffff000002ff3680
[ 1.596567] x23: ffff000002ff3688 x22: ffff000002ff3680 x21: 0000000000000000
[ 1.603715] x20: 0000000000000000 x19: ffff000003118a80 x18: 0000000000000001
[ 1.610867] x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000000
[ 1.618019] x14: ffff0000000d44c0 x13: ffff80002e7ed000 x12: 0000000034d4d91d
[ 1.625172] x11: 0000000000000000 x10: 0000000000000960 x9 : ffff800011f1b460
[ 1.632321] x8 : ffff00003fdb1d40 x7 : ffff00003fdb7f00 x6 : 0000000004ce61f4
[ 1.639475] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[ 1.646625] x2 : 11246917acc78900 x1 : 0000000000000000 x0 : 0000000000000001
[ 1.653779] Call trace:
[ 1.656230] imx7d_charger_detection+0x3f0/0x510
[ 1.660854] imx_usbmisc_charger_detection+0x40/0xc8
[ 1.665825] ci_hdrc_imx_notify_event+0xec/0x120
[ 1.670455] ci_udc_vbus_session+0x70/0xa0
[ 1.674560] usb_gadget_vbus_connect+0x20/0x38
[ 1.679013] ci_handle_vbus_change+0x64/0x70
[ 1.683290] ci_hdrc_probe+0x4e0/0x838
[ 1.687047] platform_probe+0x68/0xd8
[ 1.690717] really_probe+0xb0/0x318
[ 1.694300] __driver_probe_device+0x78/0xe0
[ 1.698576] driver_probe_device+0xb0/0x110
[ 1.702767] __device_attach_driver+0x90/0xe0
[ 1.707131] bus_for_each_drv+0x7c/0xd0
[ 1.710974] __device_attach+0xe8/0x148
[ 1.714816] device_initial_probe+0x14/0x20
[ 1.719009] bus_probe_device+0x9c/0xa8
[ 1.722852] device_add+0x3f0/0x840
[ 1.726349] platform_device_add+0x110/0x238
[ 1.730626] ci_hdrc_add_device+0x538/0x5a8
[ 1.734818] ci_hdrc_imx_probe+0x3fc/0x810
[ 1.738927] platform_probe+0x68/0xd8
[ 1.742598] really_probe+0xb0/0x318
[ 1.746178] __driver_probe_device+0x78/0xe0
[ 1.750458] driver_probe_device+0xb0/0x110
[ 1.754651] __device_attach_driver+0x90/0xe0
[ 1.759013] bus_for_each_drv+0x7c/0xd0
[ 1.762858] __device_attach+0xe8/0x148
[ 1.766703] device_initial_probe+0x14/0x20
[ 1.770896] bus_probe_device+0x9c/0xa8
[ 1.774739] deferred_probe_work_func+0x88/0xc0
[ 1.779278] process_one_work+0x1e8/0x360
[ 1.783295] worker_thread+0x210/0x480
[ 1.787049] kthread+0x150/0x160
[ 1.790284] ret_from_fork+0x10/0x18
[ 1.793873] Code: a8c47bfd d50323bf d65f03c0 52800020 (b9009aa0)
[ 1.799975] ---[ end trace d43a19cb0f4746b6 ]---
If anyone has a hint that would be great.
--
Heiko
Hi all,
Am So., 19. Sept. 2021 um 08:58 Uhr schrieb Heiko Thiery
<[email protected]>:
>
> Hi all,
>
> I see the following crash on a imx8mm board implementation. I cannot
> find a reason for this at the moment. This can be reproduced with the
> imx8mm-kontron-n801x-s dtb.Has anyone seen the same issue?
>
> The position in the code is here:
> https://elixir.bootlin.com/linux/latest/source/drivers/usb/chipidea/usbmisc_imx.c#L819
>
> [ 1.489344] Unable to handle kernel NULL pointer dereference at
> virtual address 0000000000000098
> [ 1.498170] Mem abort info:
> [ 1.500966] ESR = 0x96000044
> [ 1.504030] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 1.509356] SET = 0, FnV = 0
> [ 1.512416] EA = 0, S1PTW = 0
> [ 1.515569] FSC = 0x04: level 0 translation fault
> [ 1.520458] Data abort info:
> [ 1.523349] ISV = 0, ISS = 0x00000044
> [ 1.527196] CM = 0, WnR = 1
> [ 1.530176] [0000000000000098] user address but active_mm is swapper
> [ 1.536544] Internal error: Oops: 96000044 [#1] PREEMPT SMP
> [ 1.542125] Modules linked in:
> [ 1.545190] CPU: 3 PID: 7 Comm: kworker/u8:0 Not tainted 5.14.0-dirty #3
> [ 1.551901] Hardware name: Kontron i.MX8MM N801X S (DT)
> [ 1.557133] Workqueue: events_unbound deferred_probe_work_func
> [ 1.562984] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> [ 1.568998] pc : imx7d_charger_detection+0x3f0/0x510
> [ 1.573973] lr : imx7d_charger_detection+0x22c/0x510
> [ 1.578947] sp : ffff800011f1b5d0
> [ 1.582266] x29: ffff800011f1b5d0 x28: 0000000000000000 x27: 0000000000000000
> [ 1.589416] x26: ffff000000294c10 x25: ffff800011a6d678 x24: ffff000002ff3680
> [ 1.596567] x23: ffff000002ff3688 x22: ffff000002ff3680 x21: 0000000000000000
> [ 1.603715] x20: 0000000000000000 x19: ffff000003118a80 x18: 0000000000000001
> [ 1.610867] x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000000
> [ 1.618019] x14: ffff0000000d44c0 x13: ffff80002e7ed000 x12: 0000000034d4d91d
> [ 1.625172] x11: 0000000000000000 x10: 0000000000000960 x9 : ffff800011f1b460
> [ 1.632321] x8 : ffff00003fdb1d40 x7 : ffff00003fdb7f00 x6 : 0000000004ce61f4
> [ 1.639475] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [ 1.646625] x2 : 11246917acc78900 x1 : 0000000000000000 x0 : 0000000000000001
> [ 1.653779] Call trace:
> [ 1.656230] imx7d_charger_detection+0x3f0/0x510
> [ 1.660854] imx_usbmisc_charger_detection+0x40/0xc8
> [ 1.665825] ci_hdrc_imx_notify_event+0xec/0x120
> [ 1.670455] ci_udc_vbus_session+0x70/0xa0
> [ 1.674560] usb_gadget_vbus_connect+0x20/0x38
> [ 1.679013] ci_handle_vbus_change+0x64/0x70
> [ 1.683290] ci_hdrc_probe+0x4e0/0x838
> [ 1.687047] platform_probe+0x68/0xd8
> [ 1.690717] really_probe+0xb0/0x318
> [ 1.694300] __driver_probe_device+0x78/0xe0
> [ 1.698576] driver_probe_device+0xb0/0x110
> [ 1.702767] __device_attach_driver+0x90/0xe0
> [ 1.707131] bus_for_each_drv+0x7c/0xd0
> [ 1.710974] __device_attach+0xe8/0x148
> [ 1.714816] device_initial_probe+0x14/0x20
> [ 1.719009] bus_probe_device+0x9c/0xa8
> [ 1.722852] device_add+0x3f0/0x840
> [ 1.726349] platform_device_add+0x110/0x238
> [ 1.730626] ci_hdrc_add_device+0x538/0x5a8
> [ 1.734818] ci_hdrc_imx_probe+0x3fc/0x810
> [ 1.738927] platform_probe+0x68/0xd8
> [ 1.742598] really_probe+0xb0/0x318
> [ 1.746178] __driver_probe_device+0x78/0xe0
> [ 1.750458] driver_probe_device+0xb0/0x110
> [ 1.754651] __device_attach_driver+0x90/0xe0
> [ 1.759013] bus_for_each_drv+0x7c/0xd0
> [ 1.762858] __device_attach+0xe8/0x148
> [ 1.766703] device_initial_probe+0x14/0x20
> [ 1.770896] bus_probe_device+0x9c/0xa8
> [ 1.774739] deferred_probe_work_func+0x88/0xc0
> [ 1.779278] process_one_work+0x1e8/0x360
> [ 1.783295] worker_thread+0x210/0x480
> [ 1.787049] kthread+0x150/0x160
> [ 1.790284] ret_from_fork+0x10/0x18
> [ 1.793873] Code: a8c47bfd d50323bf d65f03c0 52800020 (b9009aa0)
> [ 1.799975] ---[ end trace d43a19cb0f4746b6 ]---
Meanwhile I figured out that in ci_hdrc_imx_probe() the "fsl,usbphy"
node is not found [1]. But I do not understand why.
The following failure leads to the return code of -19 (ENODEV) and
sets the pyh to NULL:
"failed to get fsl,usbphy phandle in /soc@0/bus@32c00000/usb@32e40000 node"
[1] https://elixir.bootlin.com/linux/latest/source/drivers/usb/chipidea/ci_hdrc_imx.c#L420
--
Heiko
Hi Heiko,
On Sun, Sep 19, 2021 at 7:10 AM Heiko Thiery <[email protected]> wrote:
> Meanwhile I figured out that in ci_hdrc_imx_probe() the "fsl,usbphy"
> node is not found [1]. But I do not understand why.
>
> The following failure leads to the return code of -19 (ENODEV) and
> sets the pyh to NULL:
> "failed to get fsl,usbphy phandle in /soc@0/bus@32c00000/usb@32e40000 node"
Since commit:
commit 78e80c4b4238c1f5642b975859664fced4f9c69e
Author: Marek Vasut <[email protected]>
Date: Wed Jul 21 18:39:55 2021 +0200
arm64: dts: imx8m: Replace deprecated fsl,usbphy DT props with phys
The fsl,usbphy DT property is deprecated, replace it with phys DT
property and specify #phy-cells. No functional change.
Signed-off-by: Marek Vasut <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: Shawn Guo <[email protected]>
To: [email protected]
Signed-off-by: Shawn Guo <[email protected]>
Don't we need to search for 'phys' too?
Does this patch help?
https://pastebin.com/raw/yZKz1huL
Hi Fabio,
Am So., 19. Sept. 2021 um 15:08 Uhr schrieb Fabio Estevam <[email protected]>:
>
> Hi Heiko,
>
> On Sun, Sep 19, 2021 at 7:10 AM Heiko Thiery <[email protected]> wrote:
>
> > Meanwhile I figured out that in ci_hdrc_imx_probe() the "fsl,usbphy"
> > node is not found [1]. But I do not understand why.
> >
> > The following failure leads to the return code of -19 (ENODEV) and
> > sets the pyh to NULL:
> > "failed to get fsl,usbphy phandle in /soc@0/bus@32c00000/usb@32e40000 node"
>
> Since commit:
> commit 78e80c4b4238c1f5642b975859664fced4f9c69e
> Author: Marek Vasut <[email protected]>
> Date: Wed Jul 21 18:39:55 2021 +0200
>
> arm64: dts: imx8m: Replace deprecated fsl,usbphy DT props with phys
>
> The fsl,usbphy DT property is deprecated, replace it with phys DT
> property and specify #phy-cells. No functional change.
>
> Signed-off-by: Marek Vasut <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: NXP Linux Team <[email protected]>
> Cc: Shawn Guo <[email protected]>
> To: [email protected]
> Signed-off-by: Shawn Guo <[email protected]>
>
> Don't we need to search for 'phys' too?
>
> Does this patch help?
> https://pastebin.com/raw/yZKz1huL
I can confirm that on the next-20210915 (that includes commit
78e80c4b4238c1f5642b975859664fced4f9c69e) your provided patch solves
the problem.
But is it explainable that in the version before the commit
78e80c4b4238c1f5642b975859664fced4f9c69e the problem occurs in the
form I reported?
--
Heiko
Hi Heiko,
On Sun, Sep 19, 2021 at 4:33 PM Heiko Thiery <[email protected]> wrote:
> > Does this patch help?
> > https://pastebin.com/raw/yZKz1huL
>
> I can confirm that on the next-20210915 (that includes commit
> 78e80c4b4238c1f5642b975859664fced4f9c69e) your provided patch solves
> the problem.
Thanks for testing it.
> But is it explainable that in the version before the commit
> 78e80c4b4238c1f5642b975859664fced4f9c69e the problem occurs in the
> form I reported?
I don't understand this problem either. I would suggest bisecting it.
Hi Fabio,
Am So., 19. Sept. 2021 um 23:44 Uhr schrieb Fabio Estevam <[email protected]>:
>
> Hi Heiko,
>
> On Sun, Sep 19, 2021 at 4:33 PM Heiko Thiery <[email protected]> wrote:
>
> > > Does this patch help?
> > > https://pastebin.com/raw/yZKz1huL
> >
> > I can confirm that on the next-20210915 (that includes commit
> > 78e80c4b4238c1f5642b975859664fced4f9c69e) your provided patch solves
> > the problem.
>
> Thanks for testing it.
>
> > But is it explainable that in the version before the commit
> > 78e80c4b4238c1f5642b975859664fced4f9c69e the problem occurs in the
> > form I reported?
>
> I don't understand this problem either. I would suggest bisecting it.
Now it is clear to me. I used the dtb for my board that had already
changed the phy node and tried to boot the "old" kernel 5.14. Thus no
phy could be found. Nevertheless the kernel should not crash in case
no phy was found.
So I made a beginner's mistake here. But this also means that you can
no longer start an old kernel with the changed dtb. This comes into
play when you e.g. a standard distribution where the embedded dtb is
passed from the uboot via EFI boot.
--
Heiko
Hi Heiko,
On Mon, Sep 20, 2021 at 6:17 AM Heiko Thiery <[email protected]> wrote:
> Now it is clear to me. I used the dtb for my board that had already
> changed the phy node and tried to boot the "old" kernel 5.14. Thus no
> phy could be found. Nevertheless the kernel should not crash in case
> no phy was found.
Agreed. The patch I proposed earlier should fix the problem, correct?
https://pastebin.com/raw/yZKz1huL
Hi,
Am Mo., 20. Sept. 2021 um 12:52 Uhr schrieb Fabio Estevam <[email protected]>:
>
> Hi Heiko,
>
> On Mon, Sep 20, 2021 at 6:17 AM Heiko Thiery <[email protected]> wrote:
>
> > Now it is clear to me. I used the dtb for my board that had already
> > changed the phy node and tried to boot the "old" kernel 5.14. Thus no
> > phy could be found. Nevertheless the kernel should not crash in case
> > no phy was found.
>
> Agreed. The patch I proposed earlier should fix the problem, correct?
> https://pastebin.com/raw/yZKz1huL
Yes this should do it.
--
Heiko
On 20.09.21 13:05, Heiko Thiery wrote:
> Hi,
>
> Am Mo., 20. Sept. 2021 um 12:52 Uhr schrieb Fabio Estevam <[email protected]>:
>>
>> Hi Heiko,
>>
>> On Mon, Sep 20, 2021 at 6:17 AM Heiko Thiery <[email protected]> wrote:
>>
>>> Now it is clear to me. I used the dtb for my board that had already
>>> changed the phy node and tried to boot the "old" kernel 5.14. Thus no
>>> phy could be found. Nevertheless the kernel should not crash in case
>>> no phy was found.
>>
>> Agreed. The patch I proposed earlier should fix the problem, correct?
>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpastebin.com%2Fraw%2FyZKz1huL&data=04%7C01%7Cfrieder.schrempf%40kontron.de%7C7f02c93eae364cf2fbc608d97c269eb6%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637677327580221365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=HMFJKptc6UBzfnpPcPF%2BSdP0g1s%2Fc0OSRfFliD6HibY%3D&reserved=0
>
> Yes this should do it.
Commit ed5a419bb019 ("usb: chipidea: imx: "fsl,usbphy" phandle is not
mandatory now") explains, that the core driver already covers reading
the "phys" property (see ci_hdrc_probe()):
Since the chipidea common code support get the USB PHY phandle from
"phys", the glue layer is not mandatory to get the "fsl,usbphy"
phandle any more.
This seems to be the reason why ci_hdrc_imx_probe() doesn't return any
error in case "fsl,usbphy" is not set. It expects that the core will
handle data->phy = NULL and already checks for a "phys" property.
Therefore I'm not sure Fabio's fix is the right way to go. Could it be
that the ci_hdrc_imx driver expects that it will be probed before the
ci_hdrc core and this isn't true in Heiko's case?
Hi Frieder,
On Mon, Sep 20, 2021 at 11:26 AM Frieder Schrempf
<[email protected]> wrote:
> Commit ed5a419bb019 ("usb: chipidea: imx: "fsl,usbphy" phandle is not
> mandatory now") explains, that the core driver already covers reading
> the "phys" property (see ci_hdrc_probe()):
>
> Since the chipidea common code support get the USB PHY phandle from
> "phys", the glue layer is not mandatory to get the "fsl,usbphy"
> phandle any more.
>
> This seems to be the reason why ci_hdrc_imx_probe() doesn't return any
> error in case "fsl,usbphy" is not set. It expects that the core will
> handle data->phy = NULL and already checks for a "phys" property.
chipidea core populates ci->usb_phy when phys is used.
The charger detection function only checks data->usb_phy, so they
don't see ci->usb_phy that was populated by the chipidea core.
We could change the signature of all charger detection functions to
receive struct ci_hdrc *ci, but that will be a huge patch that will
probably not meet
the stable requirements, so I think to minimally fix stable we should
go with the proposed fix I suggested.
>
> Therefore I'm not sure Fabio's fix is the right way to go. Could it be
> that the ci_hdrc_imx driver expects that it will be probed before the
> ci_hdrc core and this isn't true in Heiko's case?
Hi Fabio,
On 20.09.21 22:50, Fabio Estevam wrote:
> Hi Frieder,
>
> On Mon, Sep 20, 2021 at 11:26 AM Frieder Schrempf
> <[email protected]> wrote:
>
>> Commit ed5a419bb019 ("usb: chipidea: imx: "fsl,usbphy" phandle is not
>> mandatory now") explains, that the core driver already covers reading
>> the "phys" property (see ci_hdrc_probe()):
>>
>> Since the chipidea common code support get the USB PHY phandle from
>> "phys", the glue layer is not mandatory to get the "fsl,usbphy"
>> phandle any more.
>>
>> This seems to be the reason why ci_hdrc_imx_probe() doesn't return any
>> error in case "fsl,usbphy" is not set. It expects that the core will
>> handle data->phy = NULL and already checks for a "phys" property.
>
> chipidea core populates ci->usb_phy when phys is used.
>
> The charger detection function only checks data->usb_phy, so they
> don't see ci->usb_phy that was populated by the chipidea core.
>
> We could change the signature of all charger detection functions to
> receive struct ci_hdrc *ci, but that will be a huge patch that will
> probably not meet
> the stable requirements, so I think to minimally fix stable we should
> go with the proposed fix I suggested.
Ok, thanks for the explanation. I agree. Would you mind sending the fix
as formal patch?
Thanks
Frieder
On 21.09.21 08:42, Frieder Schrempf wrote:
> Hi Fabio,
>
> On 20.09.21 22:50, Fabio Estevam wrote:
>> Hi Frieder,
>>
>> On Mon, Sep 20, 2021 at 11:26 AM Frieder Schrempf
>> <[email protected]> wrote:
>>
>>> Commit ed5a419bb019 ("usb: chipidea: imx: "fsl,usbphy" phandle is not
>>> mandatory now") explains, that the core driver already covers reading
>>> the "phys" property (see ci_hdrc_probe()):
>>>
>>> Since the chipidea common code support get the USB PHY phandle from
>>> "phys", the glue layer is not mandatory to get the "fsl,usbphy"
>>> phandle any more.
>>>
>>> This seems to be the reason why ci_hdrc_imx_probe() doesn't return any
>>> error in case "fsl,usbphy" is not set. It expects that the core will
>>> handle data->phy = NULL and already checks for a "phys" property.
>>
>> chipidea core populates ci->usb_phy when phys is used.
>>
>> The charger detection function only checks data->usb_phy, so they
>> don't see ci->usb_phy that was populated by the chipidea core.
>>
>> We could change the signature of all charger detection functions to
>> receive struct ci_hdrc *ci, but that will be a huge patch that will
>> probably not meet
>> the stable requirements, so I think to minimally fix stable we should
>> go with the proposed fix I suggested.
>
> Ok, thanks for the explanation. I agree. Would you mind sending the fix
> as formal patch?
Sorry, I just saw you already did.