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

[<pritam.sutar@samsung.com>]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Sunday, February 15, 2026 2:27 PM
> To: 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 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.gi
> >> t/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.
> 

Yes. 
Sorry for misinterpreting above comment. Is it expected as below?

This is based on our understanding by referring binding and other vendor dts.

--- a/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dts
+++ b/arch/arm64/boot/dts/exynos/exynosautov920-sadk.dts
@@ -59,7 +59,7 @@ dummy_regulator: regulator-0 {
                 regulator-name = "dummy_regulator";
        };

-       usbdrd31_dwc3_vbus: usbdrd31_dwc3-vbus {
+       reg_usb_vbus0: regulator-5v0-vbus0 {
               compatible = "regulator-fixed";
               regulator-name = "usbdrd31_dwc3-vbus";
               regulator-min-microvolt = <5000000>;
@@ -75,7 +75,7 @@ usb_phy0: usb-phy0 {
                vbus-supply = <&usbdrd31_dwc3_vbus>;
        };

-       usbdrd20_dwc3_0_vbus: usbdrd20_dwc3_0-vbus {
+       reg_usb_vbus1: regulator-5v0-vbus1 {
               compatible = "regulator-fixed";
               regulator-name = "usbdrd20_dwc3_0-vbus";
               regulator-min-microvolt = <5000000>;
@@ -91,7 +91,7 @@ usb_phy1: usb-phy1 {
                vbus-supply = <&usbdrd20_dwc3_0_vbus>;
        };

-       usbdrd20_dwc3_1_vbus: usbdrd20_dwc3_1-vbus {
+       reg_usb_vbus2: regulator-5v0-vbus2 {
               compatible = "regulator-fixed";
               regulator-name = "usbdrd20_dwc3_1-vbus";
               regulator-min-microvolt = <5000000>;
@@ -107,7 +107,7 @@ usb_phy2: usb-phy2 {
                vbus-supply = <&usbdrd20_dwc3_1_vbus>;
        };

-       usbdrd20_dwc3_2_vbus: usbdrd20_dwc3_2-vbus {
+       reg_usb_vbus3: regulator-5v0-vbus3 {
               compatible = "regulator-fixed";
               regulator-name = "usbdrd20_dwc3_2-vbus";
               regulator-min-microvolt = <5000000>;

> 
> >
> >> 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@linar
> > o.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.
> 

Are you expecting details as mentioned in above section in commit message? 
However, we have mentioned these details in cover letter.

if not, please provide more details on this comment.

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

Thank you,

Regards,
Pritam