Devicetree
 help / color / mirror / Atom feed
From: Ajit Singh <blfizzyy@gmail.com>
To: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Cc: Bjorn Andersson <andersson@kernel.org>,
	 Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	 Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	 Conor Dooley <conor+dt@kernel.org>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 3/3] arm64: dts: qcom: Add Vicharak Axon Mini
Date: Sat, 20 Jun 2026 10:27:00 +0530	[thread overview]
Message-ID: <ajYcnHPZjkFwGjly@page.local> (raw)
In-Reply-To: <c16a2837-4b04-466a-bf4a-3717436c25be@oss.qualcomm.com>

On Fri, Jun 19, 2026 at 05:30:40PM +0530, Konrad Dybcio wrote:
> On 6/18/26 8:29 AM, Ajit Singh wrote:
> > On Tue, Jun 16, 2026 at 02:10:44PM +0530, Konrad Dybcio wrote:
> >> On 6/12/26 6:16 AM, Ajit Singh wrote:
> >>> On Wed, Jun 10, 2026 at 02:58:19PM +0530, Konrad Dybcio wrote:
> >>>> On 6/7/26 1:36 PM, Ajit Singh wrote:
> >>>>> Add DTS for the Vicharak Axon Mini board based on the Qualcomm
> >>>>> QCS6490 SoC.
> >>>>>
> >>>>> This adds debug UART, eMMC, UFS, SDIO WLAN, USB 2.0 host, PCIe
> >>>>> support along with regulators.
> >>>>>
> >>>>> The UFS ICE block is kept disabled because enabling it currently causes
> >>>>> an SError during qcom_ice_create() on this board. UFS works without ICE.
> >>>>>
> >>>>> Signed-off-by: Ajit Singh <blfizzyy@gmail.com>
> >>>>> ---
> >>>>
> >>>> [...]
> >>>>
> >>>>> +		vreg_l12c_1p8: ldo12 {
> >>>>> +			regulator-name = "vreg_l12c_1p8";
> >>>>> +			regulator-min-microvolt = <1800000>;
> >>>>> +			regulator-max-microvolt = <2000000>;
> >>>>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> >>>>> +
> >>>>> +			/*
> >>>>> +			 * VREG_L12C_1P8 supplies the Ampak WLAN/BT module
> >>>>> +			 * VDDIO and the external 32.768 kHz oscillator.
> >>>>> +			 */
> >>>>
> >>>> Sorry for the long review timelines on the previous patch, many of us
> >>>> were out for conferences..
> >>>>
> >>>> Is the oscillator used for that WLAN module? Would you ideally like to
> >>>> be able to turn it on/off?
> >>>
> >>> yes, oscillator is used for WLAN modules. Oscillator is powered from the same
> >>> VREG_L12C rail as WLAN VDDIO, so there is no separate regulator control to put
> >>> in pwrseq. So I think this will work fine?
> >>
> >> Probably? My point is that you marked it as always-on, so it will *never*
> >> turn off right now. For e.g. Qualcomm wifi, there's some timing spec that
> >> needs to be met wrt delays between toggling various regulators and GPIOs
> >> going to the module, hence I suggested you may need some pwrseq inbetween
> >> to achieve reliable powering on/off
> >>
> > Right, I checked the module timing requirements.
> > 
> > The module requires VBAT to be present before or at the same time as VDDIO, and
> > WL_REG_ON to be asserted only after VBAT/VDDIO are valid, around 2 sleep-clock
> > cycles later.
> > 
> > On this board, VBAT is the shared VCC_3V3 rail and is enabled by hardware, so
> > it is already present before VDDIO. VREG_L12C supplies WLAN/BT VDDIO and is
> > kept on. The WL_REG_ON timing is handled by the existing mmc-pwrseq-simple
> > reset GPIO/delay before SDIO enumeration.
> > 
> > So I think the current sequencing matches the module timing requirement.
> 
> Since the sequencing is handled, can we drop the always-on property,
> perhaps by describing the SDIO WLAN?
> 
> e.g. in arch/arm/boot/dts/st/ste-ux500-samsung-codina-tmo.dts there is:
> 
> /* WLAN SDIO channel */
> mmc@80118000 {
> 	arm,primecell-periphid = <0x10480180>;
> 	max-frequency = <50000000>;
> 	bus-width = <4>;
> 	non-removable;
> 	cap-sd-highspeed;
> 	pinctrl-names = "default", "sleep";
> 	pinctrl-0 = <&mc1_a_2_default>;
> 	pinctrl-1 = <&mc1_a_2_sleep>;
> 	/*
> 		* GPIO-controlled voltage enablement: this drives
> 		* the WL_REG_ON line high when we use this device.
> 		* Represented as regulator to fill OCR mask.
> 		*/
> 	vmmc-supply = <&wl_reg>;
> 
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	status = "okay";
> 
> 	wifi@1 {
> 		compatible = "brcm,bcm4334-fmac", "brcm,bcm4329-fmac";
> 		reg = <1>;
> 		/* GPIO216 WL_HOST_WAKE */
> 		interrupt-parent = <&gpio6>;
> 		interrupts = <24 IRQ_TYPE_EDGE_FALLING>;
> 		interrupt-names = "host-wake";
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&wlan_default_mode>;
> 	};
> };
> 
> most notably though, it seems that the brcmfmac driver doesn't even use
> the regulator framework, probably because all of the SDIO WLANs that
> Linux supports were wired in a more "embedded" way, where the V(Q)MMC
> supplies were enough.. unless it's the case here too?

It might not be possible to drop regulator-always-on for VREG_L12C here. The
SDHC2 host already has its own vmmc/vqmmc rails:

vqmmc-supply = <&vreg_l2c_1p62>;
vmmc-supply = <&vreg_l6c_2p96>;

VREG_L12C is a separate module-side VDDIO rail for the AP6272S WLAN/BT module.
There is no separate GPIO-controlled enable for this rail;

as you said, since brcmfmac does not appear to consume an extra VDDIO regulator
from the SDIO child node, so unless there is a preferred way to model this
module-side VDDIO rail, I think VREG_L12C still needs to stay always-on.

> 
> Konrad

Ajit

  reply	other threads:[~2026-06-20  4:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-07 11:36 [PATCH v4 0/3] arm64: dts: qcom: Add Vicharak Axon Mini Ajit Singh
2026-06-07 11:36 ` [PATCH v4 1/3] dt-bindings: vendor-prefixes: Add prefix for Vicharak Ajit Singh
2026-06-07 11:36 ` [PATCH v4 2/3] dt-bindings: arm: qcom: Add Vicharak Axon Mini Ajit Singh
2026-06-07 11:36 ` [PATCH v4 3/3] arm64: dts: " Ajit Singh
2026-06-07 18:56   ` Dmitry Baryshkov
2026-06-10 12:58   ` Konrad Dybcio
2026-06-12  4:16     ` Ajit Singh
2026-06-16 12:10       ` Konrad Dybcio
2026-06-18  6:29         ` Ajit Singh
2026-06-19 15:30           ` Konrad Dybcio
2026-06-20  4:57             ` Ajit Singh [this message]
2026-06-10 13:01   ` Konrad Dybcio
2026-06-11  9:35     ` Ajit Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajYcnHPZjkFwGjly@page.local \
    --to=blfizzyy@gmail.com \
    --cc=andersson@kernel.org \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox