Hi Heiko,
On Sun, Sep 19, 2021 at 7:10 AM Heiko Thiery [off-list ref] 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 [off-list ref]
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 [off-list ref]
Cc: Fabio Estevam [off-list ref]
Cc: NXP Linux Team [off-list ref]
Cc: Shawn Guo [off-list ref]
To: linux-arm-kernel@lists.infradead.org
Signed-off-by: Shawn Guo [off-list ref]
Don't we need to search for 'phys' too?
Does this patch help?
https://pastebin.com/raw/yZKz1huL
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Fabio,
Am So., 19. Sept. 2021 um 15:08 Uhr schrieb Fabio Estevam [off-list ref]:
Hi Heiko,
On Sun, Sep 19, 2021 at 7:10 AM Heiko Thiery [off-list ref] wrote:
quoted
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 [off-list ref]
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 [off-list ref]
Cc: Fabio Estevam [off-list ref]
Cc: NXP Linux Team [off-list ref]
Cc: Shawn Guo [off-list ref]
To: linux-arm-kernel@lists.infradead.org
Signed-off-by: Shawn Guo [off-list ref]
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I can confirm that on the next-20210915 (that includes commit
78e80c4b4238c1f5642b975859664fced4f9c69e) your provided patch solves
the problem.
Thanks for testing it.
quoted
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Heiko,
On Mon, Sep 20, 2021 at 6:17 AM Heiko Thiery [off-list ref] 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.
Hi,
Am Mo., 20. Sept. 2021 um 12:52 Uhr schrieb Fabio Estevam [off-list ref]:
Hi Heiko,
On Mon, Sep 20, 2021 at 6:17 AM Heiko Thiery [off-list ref] wrote:
quoted
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.
Hi,
Am Mo., 20. Sept. 2021 um 12:52 Uhr schrieb Fabio Estevam [off-list ref]:
quoted
Hi Heiko,
On Mon, Sep 20, 2021 at 6:17 AM Heiko Thiery [off-list ref] wrote:
quoted
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.
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?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Frieder,
On Mon, Sep 20, 2021 at 11:26 AM Frieder Schrempf
[off-list ref] 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 Frieder,
On Mon, Sep 20, 2021 at 11:26 AM Frieder Schrempf
[off-list ref] wrote:
quoted
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Frieder,
On Mon, Sep 20, 2021 at 11:26 AM Frieder Schrempf
[off-list ref] wrote:
quoted
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?