Re: [PATCH v2 2/3] arm64: dts: exynos: ExynosAutov920: Add regulators for the USB

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/02/2026 12:37, pritam.sutar@samsung.com wrote:
> Hi Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Thursday, January 22, 2026 7:04 PM
>> To: Pritam Manohar Sutar <pritam.sutar@samsung.com>; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org; alim.akhtar@samsung.com
>> Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org;
>> rosa.pila@samsung.com; dev.tailor@samsung.com;
>> faraz.ata@samsung.com; muhammed.ali@samsung.com;
>> selvarasu.g@samsung.com
>> Subject: Re: [PATCH v2 2/3] arm64: dts: exynos: ExynosAutov920: Add
>> regulators for the USB
>>
>> On 22/01/2026 14:07, Pritam Manohar Sutar wrote:
>>> Add dummy regulator for USB and PHY supplies.
>>>
>>> Add vbus regulator that is enabled by GPIO pin.
>>>
>>> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@samsung.com>
>>> ---
>>>  .../boot/dts/exynos/exynosautov920-sadk.dts   | 139
>> ++++++++++++++++++
>>>  1 file changed, 139 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dts
>>> b/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dts
>>> index a397f068ed53..f90f7704597c 100644
>>> --- a/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dts
>>> +++ b/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dts
>>> @@ -52,6 +52,76 @@ memory@80000000 {
>>>  		      <0x8 0x80000000 0x1 0xfba00000>,
>>>  		      <0xa 0x00000000 0x2 0x00000000>;
>>>  	};
>>> +
>>> +	/* TODO: Remove this once PMIC is implemented  */
>>> +	dummy_regulator: regulator-0 {
>>> +		 compatible = "regulator-fixed";
>>> +		 regulator-name = "dummy_regulator";
>>> +	};
>>> +
>>> +	usbdrd31_dwc3_vbus: usbdrd31_dwc3-vbus {
>>
>> Please use name for all fixed regulators which matches current format
>> recommendation: 'regulator-[0-9]v[0-9]'
>>
>> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
>> Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
>>
>> None of the regulators are called like you wrote. Really NONE.
>>
> 
> Thank you for the references. Will bring changes for regulator's name 
> and labels as 
> 			
> -       usbdrd31_dwc3_vbus: usbdrd31_dwc3-vbus {
> +       reg_usbdrd31_dwc3_vbus: regulator-1 {

Did you read the binding? That's not what I asked.


> 
>> Also, carefully follow DTS coding style.
>>
> 
> Sorry for that. Checking the code and will update in next 
> patchset version (v3).
> 
>>> +	       compatible = "regulator-fixed";
>>> +	       regulator-name = "usbdrd31_dwc3-vbus";
>>> +	       regulator-min-microvolt = <5000000>;
>>> +	       regulator-max-microvolt = <5000000>;
>>> +	       enable-active-high;
>>> +	       regulator-always-on;
>>> +	       gpio = <&gpp2 0 GPIO_ACTIVE_HIGH>;
>>> +	};
>>> +
>>> +	/* Use gpio for enabling vbus regulator */
>>> +	usb_phy0: usb-phy0 {
>>
>> usb-phy-0
>>
>>> +		compatible = "usb-nop-xceiv";
>>> +		vbus-supply = <&usbdrd31_dwc3_vbus>;
>>
>> That's pretty incomplete. Why do you need such node - just to toggle
>> regulator?
>>
> 
> There is no external controller to detect or switch the USB modes 
> automatically rather it is switched using userspace control 
> (sysfs attributes) for this SoC.  GPIO pins are required to enable 
> vbus regulators. Existing phy-generic and fixed regulator are used to 
> avoid making any USB driver changes for the same purpose and it is 
> referenced from below -
> 				
> https://lore.kernel.org/r/20230825215445.28309-3-semen.protsenko@linaro.org
> a12af1c20751340ab6e449a5ef78b37d31123aca: ARM64: dts: marvell: armada-3720-db: Add phy for USB3
> 		
>>> +	};
>>
>> ...
>>
>>> +&usbdrd20_phy2 {
>>> +	dvdd-supply = <&dummy_regulator>;
>>> +	vdd18-supply = <&dummy_regulator>;
>>> +	vdd33-supply = <&dummy_regulator>;
>>> +};
>>> +
>>> +&usbdrd20_dwc3_2 {
>>> +	maximum-speed = "high-speed";
>>> +	usb-phy = <&usb_phy3>;
>>> +};
>>> +
>>> +&usbdrd20_2 {
>>> +	vdd10-supply = <&dummy_regulator>;
>>> +	vdd33-supply = <&dummy_regulator>;
>>> +};
>>
>> That's a bit too much of dummies. This is heavily incomplete. You need to
>> bring back the PMIC first.
>>
> 
> Presently, relying on USB LDOs being enabled by the bootloader in this
> automotive SoC. However, we understand the concern and it is added in case 
> if anyone wants to use implemented PMIC in future. For now, would like 
> to proceed with the dummy regulators to enable the required USB features. 

And I don't see the point of these dummies. Solves nothing.

> Dummy regulators are clearly marked so that they will be replaced once 
> the PMIC driver is fully up.
> 


Best regards,
Krzysztof