Re: [PATCH] ARM: dts: imx6qdl-dhcom: Add USB overcurrent pin on SoM layer

[Marek Vasut <marex@denx.de>]

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