Message-ID:
 <DB9PR04MB8429657FCB48ACAD74FDD471924C2@DB9PR04MB8429.eurprd04.prod.outlook.com>
Date: Tue, 22 Oct 2024 08:09:33 +0000
From: Sherry Sun <sherry.sun@....com>
To: Marco Felsch <m.felsch@...gutronix.de>
CC: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>, Amitkumar Karwar
	<amitkumar.karwar@....com>, Neeraj Sanjay Kale <neeraj.sanjaykale@....com>,
	"marcel@...tmann.org" <marcel@...tmann.org>, "luiz.dentz@...il.com"
	<luiz.dentz@...il.com>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
	"linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	GEO-CHHER-bsp-development <bsp-development.geo@...ca-geosystems.com>,
	Krzysztof Kozlowski <krzk@...nel.org>
Subject: RE: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for
 supply and reset



> -----Original Message-----
> From: Marco Felsch <m.felsch@...gutronix.de>
> Sent: Tuesday, October 22, 2024 3:23 PM
> To: Sherry Sun <sherry.sun@....com>
> Cc: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>; Amitkumar
> Karwar <amitkumar.karwar@....com>; Neeraj Sanjay Kale
> <neeraj.sanjaykale@....com>; marcel@...tmann.org;
> luiz.dentz@...il.com; robh@...nel.org; krzk+dt@...nel.org;
> conor+dt@...nel.org; p.zabel@...gutronix.de; linux-
> bluetooth@...r.kernel.org; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; GEO-CHHER-bsp-development <bsp-
> development.geo@...ca-geosystems.com>; Krzysztof Kozlowski
> <krzk@...nel.org>
> Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for
> supply and reset
> 
> On 24-10-22, Sherry Sun wrote:
> > > On 24-10-21, Krzysztof Kozlowski wrote:
> > > > On 21/10/2024 08:41, Marco Felsch wrote:
> > > > > On 24-10-07, Krzysztof Kozlowski wrote:
> > > > >> On 07/10/2024 14:58, POPESCU Catalin wrote:
> > > > >>>>>>
> > > > >>>>>> +  vcc-supply:
> > > > >>>>>> +    description:
> > > > >>>>>> +      phandle of the regulator that provides the supply voltage.
> > > > >>>>>> +
> > > > >>>>>> +  reset-gpios:
> > > > >>>>>> +    description:
> > > > >>>>>> +      Chip powerdown/reset signal (PDn).
> > > > >>>>>> +
> > > > >>>>> Hi Catalin,
> > > > >>>>>
> > > > >>>>> For NXP WIFI/BT chip, WIFI and BT share the one PDn pin,
> > > > >>>>> which
> > > means that both wifi and BT controller will be powered on and off at
> > > the same time.
> > > > >>>>> Taking the M.2 NXP WIFI/BT module as an example,
> > > pin56(W_DISABLE1) is connected to the WIFI/BT chip PDn pin, we has
> > > already controlled this pin in the corresponding PCIe/SDIO controller dts
> nodes.
> > > > >>>>> It is not clear to me what exactly pins for vcc-supply and
> > > > >>>>> reset-gpios
> > > you describing here. Can you help understand the corresponding pins
> > > on M.2 interface as an example? Thanks.
> > > > >>>
> > > > >>> Hi Sherry,
> > > > >>>
> > > > >>> Regulators and reset controls being refcounted, we can then
> > > > >>> implement powerup sequence in both bluetooth/wlan drivers and
> > > > >>> have the drivers operate independently. This way bluetooth
> > > > >>> driver would has no dependance on the wlan driver for :
> > > > >>>
> > > > >>> - its power supply
> > > > >>>
> > > > >>> - its reset pin (PDn)
> > > > >>>
> > > > >>> - its firmware (being downloaded as part of the combo
> > > > >>> firmware)
> > > > >>>
> > > > >>> For the wlan driver we use mmc power sequence to drive the
> > > > >>> chip reset pin and there's another patchset that adds support
> > > > >>> for reset control into the mmc pwrseq simple driver.
> > > > >>>
> > > > >>>> Please wrap your replies.
> > > > >>>>
> > > > >>>> It seems you need power sequencing just like Bartosz did for
> > > Qualcomm WCN.
> > > > >>>
> > > > >>> Hi Krzysztof,
> > > > >>>
> > > > >>> I'm not familiar with power sequencing, but looks like way
> > > > >>> more complicated than reset controls. So, why power sequencing
> > > > >>> is recommended here ? Is it b/c a supply is involved ?
> > > > >>
> > > > >> Based on earlier message:
> > > > >>
> > > > >> "For NXP WIFI/BT chip, WIFI and BT share the one PDn pin, which
> > > > >> means that both wifi and BT controller will be powered on and
> > > > >> off at the same time."
> > > > >>
> > > > >> but maybe that's not needed. No clue, I don't know the hardware.
> > > > >> But be carefully what you write in the bindings, because then
> > > > >> it will be
> > > ABI.
> > > > >
> > > > > We noticed the new power-sequencing infrastructure which is part
> > > > > of
> > > > > 6.11 too but I don't think that this patch is wrong. The DT ABI
> > > > > won't break if we switch to the power-sequencing later on since
> > > > > the
> > > "reset-gpios"
> > > > > are not marked as required. So it is up to the driver to handle
> > > > > it either via a separate power-sequence driver or via "power-supply"
> > > > > and "reset-gpios" directly.
> > > >
> > > > That's not the point. We expect correct hardware description. If
> > > > you say now it has "reset-gpios" but later say "actually no,
> > > > because it has PMU", I respond: no. Describe the hardware, not current
> Linux.
> > >
> > > I know that DT abstracts the HW. That said I don't see the problem
> > > with this patch. The HW is abstracted just fine:
> > >
> > > shared PDn          -> reset-gpios
> > > shared power-supply -> vcc-supply
> >
> > Actually we should use vcc-supply to control the PDn pin, this is the
> > power supply for NXP wifi/BT.
> 
> Please don't since this is regular pin on the wlan/bt device not the regulator.
> People often do that for GPIOs if the driver is missing the support to pull the
> reset/pdn/enable gpio but the enable-gpio on the regulator is to enable the
> regulator and _not_ the bt/wlan device.
> 
> Therefore the implementation Catalin provided is the correct one.
> 

For NXP wifi/BT, the PDn is the only power control pin, no specific regulator, per my understanding,
it is a common way to configure this pin as the vcc-supply for the wifi interface(SDIO or PCIe).

reg_usdhc3_vmmc: regulator-usdhc3 {
         compatible = "regulator-fixed";
         regulator-name = "WLAN_EN";
         regulator-min-microvolt = <3300000>;
         regulator-max-microvolt = <3300000>;
         gpio = <&pcal6524 20 GPIO_ACTIVE_HIGH>;
         enable-active-high;
};

&usdhc3 {
...
      vmmc-supply = <&reg_usdhc3_vmmc>;
...
}

Best Regards
Sherry

Powered by blists - more mailing lists