The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: [PATCH v1] arm64: dts: qcom: Add fixed regulators for WLAN supplies
       [not found] <20260501051918.1990713-1-qian.zhang@oss.qualcomm.com>
@ 2026-05-07 13:42 ` Shawn Guo
  2026-05-07 13:58 ` Bjorn Andersson
  1 sibling, 0 replies; 3+ messages in thread
From: Shawn Guo @ 2026-05-07 13:42 UTC (permalink / raw)
  To: Qian Zhang
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm,
	devicetree, linux-kernel

On Fri, May 01, 2026 at 01:19:18PM +0800, Qian Zhang wrote:
> Add GPIO-controlled fixed regulators for the WLAN power rails on the
> Arduino VENTUNO Q board:
> 
>   - wlan_reg_3v3: 3.3V supply controlled by GPIO54 (wlan_rf_kill)
>   - wlan_reg_1v8: 1.8V supply controlled by GPIO56 (wlan_en)
> 
> Both regulators are enabled at boot to support WLAN initialization.
> 
> Signed-off-by: Qian Zhang <qian.zhang@oss.qualcomm.com>

Nit: you might want to have subject prefix like below to tell the scope
of the change?

  arm64: dts: qcom: arduino-monza: 

> ---
> Changes in v1:

Patch version numbering generally starts from 1 rather than 0. You have
a particular reason for numbering from 0?

Shawn

>   - Clarified GPIO roles: GPIO54 for wlan_rf_kill, GPIO56 for wlan_en
>   - Improved commit message readability with bullet list format
> 
>  .../boot/dts/qcom/monaco-arduino-monza.dts    | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> index ca14f0ea4dae..1a40ac5bb4bb 100644
> --- a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> +++ b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> @@ -153,6 +153,27 @@ vreg_nvme: regulator-3p3-m2 {
>  		enable-active-high;
>  		startup-delay-us = <20000>;
>  	};
> +
> +	wlan_reg_3v3: wlan-3v3-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "wlan_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&tlmm 54 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-boot-on;
> +	};
> +
> +	wlan_reg_1v8: wlan-1v8-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "wlan_1v8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		gpio = <&tlmm 56 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-boot-on;
> +	};
> +
>  };
>  
>  &ethernet0 {
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] arm64: dts: qcom: Add fixed regulators for WLAN supplies
       [not found] <20260501051918.1990713-1-qian.zhang@oss.qualcomm.com>
  2026-05-07 13:42 ` [PATCH v1] arm64: dts: qcom: Add fixed regulators for WLAN supplies Shawn Guo
@ 2026-05-07 13:58 ` Bjorn Andersson
  2026-05-09  3:15   ` Qian Zhang
  1 sibling, 1 reply; 3+ messages in thread
From: Bjorn Andersson @ 2026-05-07 13:58 UTC (permalink / raw)
  To: Qian Zhang
  Cc: konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel

On Fri, May 01, 2026 at 01:19:18PM +0800, Qian Zhang wrote:

There shouldn't be a "v1" tag in your subject, and the subject prefix
should be:

arm64: dts: qcom: monaco-arduino-monza: ...

> Add GPIO-controlled fixed regulators for the WLAN power rails on the
> Arduino VENTUNO Q board:
> 
>   - wlan_reg_3v3: 3.3V supply controlled by GPIO54 (wlan_rf_kill)
>   - wlan_reg_1v8: 1.8V supply controlled by GPIO56 (wlan_en)

Those really sounds like gpios, rather than regulators. Why are they
represented as regulators. Please start your commit message by
describing the "problem" that you're solving.

> 
> Both regulators are enabled at boot to support WLAN initialization.
> 
> Signed-off-by: Qian Zhang <qian.zhang@oss.qualcomm.com>
> ---
> Changes in v1:

What do you mean? Changes since version 0?

>   - Clarified GPIO roles: GPIO54 for wlan_rf_kill, GPIO56 for wlan_en
>   - Improved commit message readability with bullet list format

It's impossible for me to know if the regulator vs gpio was already
discussed, because you didn't include a link to the previous version.

Use b4 instead of doing your own thing, it will handle the versioning
for you and it will provide links to previous versions.

> 
>  .../boot/dts/qcom/monaco-arduino-monza.dts    | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> index ca14f0ea4dae..1a40ac5bb4bb 100644
> --- a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> +++ b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> @@ -153,6 +153,27 @@ vreg_nvme: regulator-3p3-m2 {
>  		enable-active-high;
>  		startup-delay-us = <20000>;
>  	};
> +
> +	wlan_reg_3v3: wlan-3v3-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "wlan_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&tlmm 54 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-boot-on;
> +	};
> +
> +	wlan_reg_1v8: wlan-1v8-regulator {

Shouldn't these two regulators be referenced from a WiFi or PMU node?

Regards,
Bjorn

> +		compatible = "regulator-fixed";
> +		regulator-name = "wlan_1v8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		gpio = <&tlmm 56 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-boot-on;
> +	};
> +
>  };
>  
>  &ethernet0 {
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] arm64: dts: qcom: Add fixed regulators for WLAN supplies
  2026-05-07 13:58 ` Bjorn Andersson
@ 2026-05-09  3:15   ` Qian Zhang
  0 siblings, 0 replies; 3+ messages in thread
From: Qian Zhang @ 2026-05-09  3:15 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
	linux-kernel

On Thu, May 7, 2026 at 9:58 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Fri, May 01, 2026 at 01:19:18PM +0800, Qian Zhang wrote:
>
> There shouldn't be a "v1" tag in your subject, and the subject prefix
> should be:
>
> arm64: dts: qcom: monaco-arduino-monza: ...
>
Thank you for the review. Apologies for the formatting issues.
The previous version (v1) can be found at:
   https://lore.kernel.org/all/20260425031712.3800662-1-qian.zhang@oss.qualcomm.com/

> > Add GPIO-controlled fixed regulators for the WLAN power rails on the
> > Arduino VENTUNO Q board:
> >
> >   - wlan_reg_3v3: 3.3V supply controlled by GPIO54 (wlan_rf_kill)
> >   - wlan_reg_1v8: 1.8V supply controlled by GPIO56 (wlan_en)
>
> Those really sounds like gpios, rather than regulators. Why are they
> represented as regulators. Please start your commit message by
> describing the "problem" that you're solving.
>
> >
> > Both regulators are enabled at boot to support WLAN initialization.
> >
> > Signed-off-by: Qian Zhang <qian.zhang@oss.qualcomm.com>
> > ---
> > Changes in v1:
>
> What do you mean? Changes since version 0?
>
> >   - Clarified GPIO roles: GPIO54 for wlan_rf_kill, GPIO56 for wlan_en
> >   - Improved commit message readability with bullet list format
>
> It's impossible for me to know if the regulator vs gpio was already
> discussed, because you didn't include a link to the previous version.
>
> Use b4 instead of doing your own thing, it will handle the versioning
> for you and it will provide links to previous versions.
>
I will use b4 for future submissions to handle versioning and
automatically include links to previous versions.

Regarding the subject: I will fix it to:
    arm64: dts: qcom: monaco-arduino-monza: ...
and remove the "v1" tag from the subject line.

> >
> >  .../boot/dts/qcom/monaco-arduino-monza.dts    | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> > index ca14f0ea4dae..1a40ac5bb4bb 100644
> > --- a/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> > +++ b/arch/arm64/boot/dts/qcom/monaco-arduino-monza.dts
> > @@ -153,6 +153,27 @@ vreg_nvme: regulator-3p3-m2 {
> >               enable-active-high;
> >               startup-delay-us = <20000>;
> >       };
> > +
> > +     wlan_reg_3v3: wlan-3v3-regulator {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "wlan_3v3";
> > +             regulator-min-microvolt = <3300000>;
> > +             regulator-max-microvolt = <3300000>;
> > +             gpio = <&tlmm 54 GPIO_ACTIVE_HIGH>;
> > +             enable-active-high;
> > +             regulator-boot-on;
> > +     };
> > +
> > +     wlan_reg_1v8: wlan-1v8-regulator {
>
> Shouldn't these two regulators be referenced from a WiFi or PMU node?
>
Regarding regulator vs gpio: you are correct. These two GPIOs serve
distinct purposes and regulator-fixed is not the right abstraction:

- GPIO54 (rfkill): controls the RF kill line. I will replace this
  with a rfkill-gpio node so the rfkill subsystem can properly
  manage RF state.

- GPIO56 (wlan_en):  must be asserted before PCIe enumeration so
  the WLAN module is detectable by the PCIe root complex.
  I will model it as a regulator-fixed and reference it from the PCIe node
  via vddpe-1v8-supply.

I will rework the patch in the next version with a corrected commit
message that describes the problem first.

> Regards,
> Bjorn
>
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "wlan_1v8";
> > +             regulator-min-microvolt = <1800000>;
> > +             regulator-max-microvolt = <1800000>;
> > +             gpio = <&tlmm 56 GPIO_ACTIVE_HIGH>;
> > +             enable-active-high;
> > +             regulator-boot-on;
> > +     };
> > +
> >  };
> >
> >  &ethernet0 {
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-09  3:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260501051918.1990713-1-qian.zhang@oss.qualcomm.com>
2026-05-07 13:42 ` [PATCH v1] arm64: dts: qcom: Add fixed regulators for WLAN supplies Shawn Guo
2026-05-07 13:58 ` Bjorn Andersson
2026-05-09  3:15   ` Qian Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox