* [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer @ 2021-12-08 15:15 Christoph Niedermaier 2021-12-09 0:23 ` Marek Vasut 0 siblings, 1 reply; 8+ messages in thread From: Christoph Niedermaier @ 2021-12-08 15:15 UTC (permalink / raw) To: linux-arm-kernel Cc: Christoph Niedermaier, Shawn Guo, Fabio Estevam, Marek Vasut, NXP Linux Team, kernel Add USB overcurrent pin muxing on SoM layer, but disable it by default. If a board has connected this pin like the picoITX, this property should be removed in the board file. Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Fabio Estevam <festevam@gmail.com> Cc: Marek Vasut <marex@denx.de> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: kernel@dh-electronics.com To: linux-arm-kernel@lists.infradead.org --- arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ 2 files changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi index 4cd4cb9543c8..a67682bfe7bd 100644 --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi @@ -48,6 +48,10 @@ "", "", "", "", "", "", "", ""; }; +&usbh1 { /* USB overcurrent pin is connected */ + /delete-property/ disable-over-current; +}; + &iomuxc { pinctrl-0 = < /* diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi index 5d10c40313cb..e4fdce016c34 100644 --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi @@ -385,6 +385,7 @@ }; &usbh1 { + disable-over-current; dr_mode = "host"; pinctrl-0 = <&pinctrl_usbh1>; pinctrl-names = "default"; @@ -728,6 +729,7 @@ pinctrl_usbh1: usbh1-grp { fsl,pins = < MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >; }; -- 2.11.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-08 15:15 [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer Christoph Niedermaier @ 2021-12-09 0:23 ` Marek Vasut 2021-12-09 9:54 ` Christoph Niedermaier 0 siblings, 1 reply; 8+ messages in thread From: Marek Vasut @ 2021-12-09 0:23 UTC (permalink / raw) To: Christoph Niedermaier, linux-arm-kernel Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel On 12/8/21 16:15, Christoph Niedermaier wrote: > Add USB overcurrent pin muxing on SoM layer, but disable it > by default. If a board has connected this pin like the > picoITX, this property should be removed in the board file. > > Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Marek Vasut <marex@denx.de> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: kernel@dh-electronics.com > To: linux-arm-kernel@lists.infradead.org > --- > arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ > arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ > 2 files changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi > index 4cd4cb9543c8..a67682bfe7bd 100644 > --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi > +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi > @@ -48,6 +48,10 @@ > "", "", "", "", "", "", "", ""; > }; > > +&usbh1 { /* USB overcurrent pin is connected */ > + /delete-property/ disable-over-current; > +}; > + > &iomuxc { > pinctrl-0 = < > /* > diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi > index 5d10c40313cb..e4fdce016c34 100644 > --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi > +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi > @@ -385,6 +385,7 @@ > }; > > &usbh1 { > + disable-over-current; > dr_mode = "host"; > pinctrl-0 = <&pinctrl_usbh1>; > pinctrl-names = "default"; > @@ -728,6 +729,7 @@ > pinctrl_usbh1: usbh1-grp { > fsl,pins = < > MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 > + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 > >; > }; Shouldn't this be the other way around -- boards with unused USB overcurrent detection should disable it ? In this case, PDK2 and DRC02 should add the 'disable-over-current' DT property and pinmux, while picoitx should add the USB OC pinmux . _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-09 0:23 ` Marek Vasut @ 2021-12-09 9:54 ` Christoph Niedermaier 2021-12-09 22:26 ` Marek Vasut 0 siblings, 1 reply; 8+ messages in thread From: Christoph Niedermaier @ 2021-12-09 9:54 UTC (permalink / raw) To: Marek MV. Vasut, linux-arm-kernel@lists.infradead.org Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel From: Marek Vasut Sent: Thursday, December 9, 2021 1:23 AM > > On 12/8/21 16:15, Christoph Niedermaier wrote: >> Add USB overcurrent pin muxing on SoM layer, but disable it >> by default. If a board has connected this pin like the >> picoITX, this property should be removed in the board file. >> >> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >> Cc: Shawn Guo <shawnguo@kernel.org> >> Cc: Fabio Estevam <festevam@gmail.com> >> Cc: Marek Vasut <marex@denx.de> >> Cc: NXP Linux Team <linux-imx@nxp.com> >> Cc: kernel@dh-electronics.com >> To: linux-arm-kernel@lists.infradead.org >> --- >> arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ >> arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >> index 4cd4cb9543c8..a67682bfe7bd 100644 >> --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >> @@ -48,6 +48,10 @@ >> "", "", "", "", "", "", "", ""; >> }; >> >> +&usbh1 { /* USB overcurrent pin is connected */ >> + /delete-property/ disable-over-current; >> +}; >> + >> &iomuxc { >> pinctrl-0 = < >> /* >> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >> index 5d10c40313cb..e4fdce016c34 100644 >> --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >> @@ -385,6 +385,7 @@ >> }; >> >> &usbh1 { >> + disable-over-current; >> dr_mode = "host"; >> pinctrl-0 = <&pinctrl_usbh1>; >> pinctrl-names = "default"; >> @@ -728,6 +729,7 @@ >> pinctrl_usbh1: usbh1-grp { >> fsl,pins = < >> MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 >> + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >> >; >> }; > > Shouldn't this be the other way around -- boards with unused USB > overcurrent detection should disable it ? In this case, PDK2 and DRC02 > should add the 'disable-over-current' DT property and pinmux, while > picoitx should add the USB OC pinmux . This pin is defined by the DHCOM standard, therefore no other function can be used on this pin. That is why it should be configured in the SoM file. The first internal version was the other way around, but then we had a discussion about accidentally enabling the overcurrent detection, because it is enabled by default. Since most customers do not use overcurrent detection, we have decided to disable it by default. So if a customer uses that pin, he has to actively remove the DT property as for example shown in the PicoITX board file. In this way, the source of issues should be reduced. Regards Christoph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-09 9:54 ` Christoph Niedermaier @ 2021-12-09 22:26 ` Marek Vasut 2021-12-10 7:58 ` Christoph Niedermaier 0 siblings, 1 reply; 8+ messages in thread From: Marek Vasut @ 2021-12-09 22:26 UTC (permalink / raw) To: Christoph Niedermaier, linux-arm-kernel@lists.infradead.org Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel On 12/9/21 10:54, Christoph Niedermaier wrote: > From: Marek Vasut > Sent: Thursday, December 9, 2021 1:23 AM >> >> On 12/8/21 16:15, Christoph Niedermaier wrote: >>> Add USB overcurrent pin muxing on SoM layer, but disable it >>> by default. If a board has connected this pin like the >>> picoITX, this property should be removed in the board file. >>> >>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >>> Cc: Shawn Guo <shawnguo@kernel.org> >>> Cc: Fabio Estevam <festevam@gmail.com> >>> Cc: Marek Vasut <marex@denx.de> >>> Cc: NXP Linux Team <linux-imx@nxp.com> >>> Cc: kernel@dh-electronics.com >>> To: linux-arm-kernel@lists.infradead.org >>> --- >>> arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ >>> arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ >>> 2 files changed, 6 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>> index 4cd4cb9543c8..a67682bfe7bd 100644 >>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>> @@ -48,6 +48,10 @@ >>> "", "", "", "", "", "", "", ""; >>> }; >>> >>> +&usbh1 { /* USB overcurrent pin is connected */ >>> + /delete-property/ disable-over-current; >>> +}; >>> + >>> &iomuxc { >>> pinctrl-0 = < >>> /* >>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>> index 5d10c40313cb..e4fdce016c34 100644 >>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>> @@ -385,6 +385,7 @@ >>> }; >>> >>> &usbh1 { >>> + disable-over-current; >>> dr_mode = "host"; >>> pinctrl-0 = <&pinctrl_usbh1>; >>> pinctrl-names = "default"; >>> @@ -728,6 +729,7 @@ >>> pinctrl_usbh1: usbh1-grp { >>> fsl,pins = < >>> MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 >>> + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >>> >; >>> }; >> >> Shouldn't this be the other way around -- boards with unused USB >> overcurrent detection should disable it ? In this case, PDK2 and DRC02 >> should add the 'disable-over-current' DT property and pinmux, while >> picoitx should add the USB OC pinmux . > > This pin is defined by the DHCOM standard, therefore no other function > can be used on this pin. That is why it should be configured in the SoM > file. Then the pinmux in the SoM dtsi is OK. > The first internal version was the other way around, but then we had a > discussion about accidentally enabling the overcurrent detection, because > it is enabled by default. Since most customers do not use overcurrent > detection, we have decided to disable it by default. So if a customer > uses that pin, he has to actively remove the DT property as for example > shown in the PicoITX board file. In this way, the source of issues should > be reduced. It seems to me that if the SoM has a dedicated pin for USB OC, then the SoM dtsi should keep the default configuration of USB OC (i.e. enabled). If a board does not use the USB OC (e.g. because there is a USB hub on it), then the board should add the 'disable-over-current' property, because this is clearly a board property, not a SoM property. Besides, on systems without a USB hub, you likely want to make sure the OC detection is not accidentally forgotten disabled, as that might lead to damage to the port. So I would say, keep the pinmux settings in the SoM dtsi, and add disable-over-current property on board level dts. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-09 22:26 ` Marek Vasut @ 2021-12-10 7:58 ` Christoph Niedermaier 2021-12-12 23:24 ` Marek Vasut 0 siblings, 1 reply; 8+ messages in thread From: Christoph Niedermaier @ 2021-12-10 7:58 UTC (permalink / raw) To: Marek MV. Vasut, linux-arm-kernel@lists.infradead.org Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel From: Marek Vasut [mailto:marex@denx.de] Sent: Thursday, December 9, 2021 11:26 PM > On 12/9/21 10:54, Christoph Niedermaier wrote: >> From: Marek Vasut >> Sent: Thursday, December 9, 2021 1:23 AM >>> >>> On 12/8/21 16:15, Christoph Niedermaier wrote: >>>> Add USB overcurrent pin muxing on SoM layer, but disable it >>>> by default. If a board has connected this pin like the >>>> picoITX, this property should be removed in the board file. >>>> >>>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >>>> Cc: Shawn Guo <shawnguo@kernel.org> >>>> Cc: Fabio Estevam <festevam@gmail.com> >>>> Cc: Marek Vasut <marex@denx.de> >>>> Cc: NXP Linux Team <linux-imx@nxp.com> >>>> Cc: kernel@dh-electronics.com >>>> To: linux-arm-kernel@lists.infradead.org >>>> --- >>>> arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ >>>> arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ >>>> 2 files changed, 6 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>> index 4cd4cb9543c8..a67682bfe7bd 100644 >>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>> @@ -48,6 +48,10 @@ >>>> "", "", "", "", "", "", "", ""; >>>> }; >>>> >>>> +&usbh1 { /* USB overcurrent pin is connected */ >>>> + /delete-property/ disable-over-current; >>>> +}; >>>> + >>>> &iomuxc { >>>> pinctrl-0 = < >>>> /* >>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>> index 5d10c40313cb..e4fdce016c34 100644 >>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>> @@ -385,6 +385,7 @@ >>>> }; >>>> >>>> &usbh1 { >>>> + disable-over-current; >>>> dr_mode = "host"; >>>> pinctrl-0 = <&pinctrl_usbh1>; >>>> pinctrl-names = "default"; >>>> @@ -728,6 +729,7 @@ >>>> pinctrl_usbh1: usbh1-grp { >>>> fsl,pins = < >>>> MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 >>>> + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >>>> >; >>>> }; >>> >>> Shouldn't this be the other way around -- boards with unused USB >>> overcurrent detection should disable it ? In this case, PDK2 and DRC02 >>> should add the 'disable-over-current' DT property and pinmux, while >>> picoitx should add the USB OC pinmux . >> >> This pin is defined by the DHCOM standard, therefore no other function >> can be used on this pin. That is why it should be configured in the SoM >> file. > > Then the pinmux in the SoM dtsi is OK. > >> The first internal version was the other way around, but then we had a >> discussion about accidentally enabling the overcurrent detection, because >> it is enabled by default. Since most customers do not use overcurrent >> detection, we have decided to disable it by default. So if a customer >> uses that pin, he has to actively remove the DT property as for example >> shown in the PicoITX board file. In this way, the source of issues should >> be reduced. > > It seems to me that if the SoM has a dedicated pin for USB OC, then the > SoM dtsi should keep the default configuration of USB OC (i.e. enabled). > If a board does not use the USB OC (e.g. because there is a USB hub on > it), then the board should add the 'disable-over-current' property, > because this is clearly a board property, not a SoM property. > > Besides, on systems without a USB hub, you likely want to make sure the > OC detection is not accidentally forgotten disabled, as that might lead > to damage to the port. > > So I would say, keep the pinmux settings in the SoM dtsi, and add > disable-over-current property on board level dts. I am with you, it is a board property. But I don't want to enable it by default, because here I rate the accidental damage of the port higher. So if you need it you can enable it on board layer. Because of the negative logic have to do it this way. What is the argument against it? Regards Christoph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-10 7:58 ` Christoph Niedermaier @ 2021-12-12 23:24 ` Marek Vasut 2021-12-14 8:59 ` Christoph Niedermaier 0 siblings, 1 reply; 8+ messages in thread From: Marek Vasut @ 2021-12-12 23:24 UTC (permalink / raw) To: Christoph Niedermaier, linux-arm-kernel@lists.infradead.org Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel On 12/10/21 08:58, Christoph Niedermaier wrote: > From: Marek Vasut [mailto:marex@denx.de] > Sent: Thursday, December 9, 2021 11:26 PM >> On 12/9/21 10:54, Christoph Niedermaier wrote: >>> From: Marek Vasut >>> Sent: Thursday, December 9, 2021 1:23 AM >>>> >>>> On 12/8/21 16:15, Christoph Niedermaier wrote: >>>>> Add USB overcurrent pin muxing on SoM layer, but disable it >>>>> by default. If a board has connected this pin like the >>>>> picoITX, this property should be removed in the board file. >>>>> >>>>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >>>>> Cc: Shawn Guo <shawnguo@kernel.org> >>>>> Cc: Fabio Estevam <festevam@gmail.com> >>>>> Cc: Marek Vasut <marex@denx.de> >>>>> Cc: NXP Linux Team <linux-imx@nxp.com> >>>>> Cc: kernel@dh-electronics.com >>>>> To: linux-arm-kernel@lists.infradead.org >>>>> --- >>>>> arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ >>>>> arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ >>>>> 2 files changed, 6 insertions(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>> index 4cd4cb9543c8..a67682bfe7bd 100644 >>>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>> @@ -48,6 +48,10 @@ >>>>> "", "", "", "", "", "", "", ""; >>>>> }; >>>>> >>>>> +&usbh1 { /* USB overcurrent pin is connected */ >>>>> + /delete-property/ disable-over-current; >>>>> +}; >>>>> + >>>>> &iomuxc { >>>>> pinctrl-0 = < >>>>> /* >>>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>> index 5d10c40313cb..e4fdce016c34 100644 >>>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>> @@ -385,6 +385,7 @@ >>>>> }; >>>>> >>>>> &usbh1 { >>>>> + disable-over-current; >>>>> dr_mode = "host"; >>>>> pinctrl-0 = <&pinctrl_usbh1>; >>>>> pinctrl-names = "default"; >>>>> @@ -728,6 +729,7 @@ >>>>> pinctrl_usbh1: usbh1-grp { >>>>> fsl,pins = < >>>>> MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 >>>>> + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >>>>> >; >>>>> }; >>>> >>>> Shouldn't this be the other way around -- boards with unused USB >>>> overcurrent detection should disable it ? In this case, PDK2 and DRC02 >>>> should add the 'disable-over-current' DT property and pinmux, while >>>> picoitx should add the USB OC pinmux . >>> >>> This pin is defined by the DHCOM standard, therefore no other function >>> can be used on this pin. That is why it should be configured in the SoM >>> file. >> >> Then the pinmux in the SoM dtsi is OK. >> >>> The first internal version was the other way around, but then we had a >>> discussion about accidentally enabling the overcurrent detection, because >>> it is enabled by default. Since most customers do not use overcurrent >>> detection, we have decided to disable it by default. So if a customer >>> uses that pin, he has to actively remove the DT property as for example >>> shown in the PicoITX board file. In this way, the source of issues should >>> be reduced. >> >> It seems to me that if the SoM has a dedicated pin for USB OC, then the >> SoM dtsi should keep the default configuration of USB OC (i.e. enabled). >> If a board does not use the USB OC (e.g. because there is a USB hub on >> it), then the board should add the 'disable-over-current' property, >> because this is clearly a board property, not a SoM property. >> >> Besides, on systems without a USB hub, you likely want to make sure the >> OC detection is not accidentally forgotten disabled, as that might lead >> to damage to the port. >> >> So I would say, keep the pinmux settings in the SoM dtsi, and add >> disable-over-current property on board level dts. > > I am with you, it is a board property. But I don't want to enable it by > default, because here I rate the accidental damage of the port higher. > So if you need it you can enable it on board layer. Because of the negative > logic have to do it this way. What is the argument against it? Per the DHCOM specification, there is a dedicated USB OC input pin on the SoM. This would imply the SoM deliberately exposes a functionality and therefore that functionality should be enabled by default, or left in default setting (which is enabled) -- that means putting only the pinmux settings into the SoM DT, the OC input is enabled by default by the CI HDRC driver, so no need for extra SoM level properties. Next, if a board does not use the OC input of the SoM, that is a board property, so the board should add 'disable-over-current' property in the board DT. The recommended configuration of boards is to connect the OC input, so in case there is an OC condition on a port, the CI HDRC driver would be notified and can turn off Vbus too e.g. via regulator to avoid stressing those Vbus power distribution switches on the board. There is also an additional benefit of not adding the 'disable-over-current' property into SoM DT when it comes to DTOs, which are used on some DHSOM. A DTO can add a new DT property, but it is not capable of deleting an existing property in the base DT. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-12 23:24 ` Marek Vasut @ 2021-12-14 8:59 ` Christoph Niedermaier 2021-12-14 9:43 ` Marek Vasut 0 siblings, 1 reply; 8+ messages in thread From: Christoph Niedermaier @ 2021-12-14 8:59 UTC (permalink / raw) To: Marek MV. Vasut, linux-arm-kernel@lists.infradead.org Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel From: Marek Vasut Sent: Monday, December 13, 2021 12:25 AM > On 12/10/21 08:58, Christoph Niedermaier wrote: >> From: Marek Vasut [mailto:marex@denx.de] >> Sent: Thursday, December 9, 2021 11:26 PM >>> On 12/9/21 10:54, Christoph Niedermaier wrote: >>>> From: Marek Vasut >>>> Sent: Thursday, December 9, 2021 1:23 AM >>>>> >>>>> On 12/8/21 16:15, Christoph Niedermaier wrote: >>>>>> Add USB overcurrent pin muxing on SoM layer, but disable it >>>>>> by default. If a board has connected this pin like the >>>>>> picoITX, this property should be removed in the board file. >>>>>> >>>>>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >>>>>> Cc: Shawn Guo <shawnguo@kernel.org> >>>>>> Cc: Fabio Estevam <festevam@gmail.com> >>>>>> Cc: Marek Vasut <marex@denx.de> >>>>>> Cc: NXP Linux Team <linux-imx@nxp.com> >>>>>> Cc: kernel@dh-electronics.com >>>>>> To: linux-arm-kernel@lists.infradead.org >>>>>> --- >>>>>> arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ >>>>>> arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ >>>>>> 2 files changed, 6 insertions(+) >>>>>> >>>>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>>> index 4cd4cb9543c8..a67682bfe7bd 100644 >>>>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>>> @@ -48,6 +48,10 @@ >>>>>> "", "", "", "", "", "", "", ""; >>>>>> }; >>>>>> >>>>>> +&usbh1 { /* USB overcurrent pin is connected */ >>>>>> + /delete-property/ disable-over-current; >>>>>> +}; >>>>>> + >>>>>> &iomuxc { >>>>>> pinctrl-0 = < >>>>>> /* >>>>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>>> index 5d10c40313cb..e4fdce016c34 100644 >>>>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>>> @@ -385,6 +385,7 @@ >>>>>> }; >>>>>> >>>>>> &usbh1 { >>>>>> + disable-over-current; >>>>>> dr_mode = "host"; >>>>>> pinctrl-0 = <&pinctrl_usbh1>; >>>>>> pinctrl-names = "default"; >>>>>> @@ -728,6 +729,7 @@ >>>>>> pinctrl_usbh1: usbh1-grp { >>>>>> fsl,pins = < >>>>>> MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 >>>>>> + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >>>>>> >; >>>>>> }; >>>>> >>>>> Shouldn't this be the other way around -- boards with unused USB >>>>> overcurrent detection should disable it ? In this case, PDK2 and DRC02 >>>>> should add the 'disable-over-current' DT property and pinmux, while >>>>> picoitx should add the USB OC pinmux . >>>> >>>> This pin is defined by the DHCOM standard, therefore no other function >>>> can be used on this pin. That is why it should be configured in the SoM >>>> file. >>> >>> Then the pinmux in the SoM dtsi is OK. >>> >>>> The first internal version was the other way around, but then we had a >>>> discussion about accidentally enabling the overcurrent detection, because >>>> it is enabled by default. Since most customers do not use overcurrent >>>> detection, we have decided to disable it by default. So if a customer >>>> uses that pin, he has to actively remove the DT property as for example >>>> shown in the PicoITX board file. In this way, the source of issues should >>>> be reduced. >>> >>> It seems to me that if the SoM has a dedicated pin for USB OC, then the >>> SoM dtsi should keep the default configuration of USB OC (i.e. enabled). >>> If a board does not use the USB OC (e.g. because there is a USB hub on >>> it), then the board should add the 'disable-over-current' property, >>> because this is clearly a board property, not a SoM property. >>> >>> Besides, on systems without a USB hub, you likely want to make sure the >>> OC detection is not accidentally forgotten disabled, as that might lead >>> to damage to the port. >>> >>> So I would say, keep the pinmux settings in the SoM dtsi, and add >>> disable-over-current property on board level dts. >> >> I am with you, it is a board property. But I don't want to enable it by >> default, because here I rate the accidental damage of the port higher. >> So if you need it you can enable it on board layer. Because of the negative >> logic have to do it this way. What is the argument against it? > > Per the DHCOM specification, there is a dedicated USB OC input pin on > the SoM. This would imply the SoM deliberately exposes a functionality > and therefore that functionality should be enabled by default, or left > in default setting (which is enabled) -- that means putting only the > pinmux settings into the SoM DT, the OC input is enabled by default by > the CI HDRC driver, so no need for extra SoM level properties. > > Next, if a board does not use the OC input of the SoM, that is a board > property, so the board should add 'disable-over-current' property in the > board DT. The recommended configuration of boards is to connect the OC > input, so in case there is an OC condition on a port, the CI HDRC driver > would be notified and can turn off Vbus too e.g. via regulator to avoid > stressing those Vbus power distribution switches on the board. > > There is also an additional benefit of not adding the > 'disable-over-current' property into SoM DT when it comes to DTOs, which > are used on some DHSOM. A DTO can add a new DT property, but it is not > capable of deleting an existing property in the base DT. I will send a version 2. Regards Christoph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer 2021-12-14 8:59 ` Christoph Niedermaier @ 2021-12-14 9:43 ` Marek Vasut 0 siblings, 0 replies; 8+ messages in thread From: Marek Vasut @ 2021-12-14 9:43 UTC (permalink / raw) To: Christoph Niedermaier, linux-arm-kernel@lists.infradead.org Cc: Shawn Guo, Fabio Estevam, NXP Linux Team, kernel On 12/14/21 09:59, Christoph Niedermaier wrote: > From: Marek Vasut > Sent: Monday, December 13, 2021 12:25 AM >> On 12/10/21 08:58, Christoph Niedermaier wrote: >>> From: Marek Vasut [mailto:marex@denx.de] >>> Sent: Thursday, December 9, 2021 11:26 PM >>>> On 12/9/21 10:54, Christoph Niedermaier wrote: >>>>> From: Marek Vasut >>>>> Sent: Thursday, December 9, 2021 1:23 AM >>>>>> >>>>>> On 12/8/21 16:15, Christoph Niedermaier wrote: >>>>>>> Add USB overcurrent pin muxing on SoM layer, but disable it >>>>>>> by default. If a board has connected this pin like the >>>>>>> picoITX, this property should be removed in the board file. >>>>>>> >>>>>>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com> >>>>>>> Cc: Shawn Guo <shawnguo@kernel.org> >>>>>>> Cc: Fabio Estevam <festevam@gmail.com> >>>>>>> Cc: Marek Vasut <marex@denx.de> >>>>>>> Cc: NXP Linux Team <linux-imx@nxp.com> >>>>>>> Cc: kernel@dh-electronics.com >>>>>>> To: linux-arm-kernel@lists.infradead.org >>>>>>> --- >>>>>>> arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi | 4 ++++ >>>>>>> arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi | 2 ++ >>>>>>> 2 files changed, 6 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>>>> index 4cd4cb9543c8..a67682bfe7bd 100644 >>>>>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-picoitx.dtsi >>>>>>> @@ -48,6 +48,10 @@ >>>>>>> "", "", "", "", "", "", "", ""; >>>>>>> }; >>>>>>> >>>>>>> +&usbh1 { /* USB overcurrent pin is connected */ >>>>>>> + /delete-property/ disable-over-current; >>>>>>> +}; >>>>>>> + >>>>>>> &iomuxc { >>>>>>> pinctrl-0 = < >>>>>>> /* >>>>>>> diff --git a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>>>> index 5d10c40313cb..e4fdce016c34 100644 >>>>>>> --- a/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>>>> +++ b/arch/arm/boot/dts/imx6qdl-dhcom-som.dtsi >>>>>>> @@ -385,6 +385,7 @@ >>>>>>> }; >>>>>>> >>>>>>> &usbh1 { >>>>>>> + disable-over-current; >>>>>>> dr_mode = "host"; >>>>>>> pinctrl-0 = <&pinctrl_usbh1>; >>>>>>> pinctrl-names = "default"; >>>>>>> @@ -728,6 +729,7 @@ >>>>>>> pinctrl_usbh1: usbh1-grp { >>>>>>> fsl,pins = < >>>>>>> MX6QDL_PAD_EIM_D31__GPIO3_IO31 0x120b0 >>>>>>> + MX6QDL_PAD_EIM_D30__USB_H1_OC 0x1b0b1 >>>>>>> >; >>>>>>> }; >>>>>> >>>>>> Shouldn't this be the other way around -- boards with unused USB >>>>>> overcurrent detection should disable it ? In this case, PDK2 and DRC02 >>>>>> should add the 'disable-over-current' DT property and pinmux, while >>>>>> picoitx should add the USB OC pinmux . >>>>> >>>>> This pin is defined by the DHCOM standard, therefore no other function >>>>> can be used on this pin. That is why it should be configured in the SoM >>>>> file. >>>> >>>> Then the pinmux in the SoM dtsi is OK. >>>> >>>>> The first internal version was the other way around, but then we had a >>>>> discussion about accidentally enabling the overcurrent detection, because >>>>> it is enabled by default. Since most customers do not use overcurrent >>>>> detection, we have decided to disable it by default. So if a customer >>>>> uses that pin, he has to actively remove the DT property as for example >>>>> shown in the PicoITX board file. In this way, the source of issues should >>>>> be reduced. >>>> >>>> It seems to me that if the SoM has a dedicated pin for USB OC, then the >>>> SoM dtsi should keep the default configuration of USB OC (i.e. enabled). >>>> If a board does not use the USB OC (e.g. because there is a USB hub on >>>> it), then the board should add the 'disable-over-current' property, >>>> because this is clearly a board property, not a SoM property. >>>> >>>> Besides, on systems without a USB hub, you likely want to make sure the >>>> OC detection is not accidentally forgotten disabled, as that might lead >>>> to damage to the port. >>>> >>>> So I would say, keep the pinmux settings in the SoM dtsi, and add >>>> disable-over-current property on board level dts. >>> >>> I am with you, it is a board property. But I don't want to enable it by >>> default, because here I rate the accidental damage of the port higher. >>> So if you need it you can enable it on board layer. Because of the negative >>> logic have to do it this way. What is the argument against it? >> >> Per the DHCOM specification, there is a dedicated USB OC input pin on >> the SoM. This would imply the SoM deliberately exposes a functionality >> and therefore that functionality should be enabled by default, or left >> in default setting (which is enabled) -- that means putting only the >> pinmux settings into the SoM DT, the OC input is enabled by default by >> the CI HDRC driver, so no need for extra SoM level properties. >> >> Next, if a board does not use the OC input of the SoM, that is a board >> property, so the board should add 'disable-over-current' property in the >> board DT. The recommended configuration of boards is to connect the OC >> input, so in case there is an OC condition on a port, the CI HDRC driver >> would be notified and can turn off Vbus too e.g. via regulator to avoid >> stressing those Vbus power distribution switches on the board. >> >> There is also an additional benefit of not adding the >> 'disable-over-current' property into SoM DT when it comes to DTOs, which >> are used on some DHSOM. A DTO can add a new DT property, but it is not >> capable of deleting an existing property in the base DT. > > I will send a version 2. Thanks _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-14 9:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-08 15:15 [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer Christoph Niedermaier 2021-12-09 0:23 ` Marek Vasut 2021-12-09 9:54 ` Christoph Niedermaier 2021-12-09 22:26 ` Marek Vasut 2021-12-10 7:58 ` Christoph Niedermaier 2021-12-12 23:24 ` Marek Vasut 2021-12-14 8:59 ` Christoph Niedermaier 2021-12-14 9:43 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).