Message-ID:
 <DB9PR04MB842929186683C1DF13DCBD92924A2@DB9PR04MB8429.eurprd04.prod.outlook.com>
Date: Mon, 28 Oct 2024 04:27:59 +0000
From: Sherry Sun <sherry.sun@....com>
To: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>, Marco Felsch
	<m.felsch@...gutronix.de>
CC: 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: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>
> Sent: Wednesday, October 23, 2024 10:38 PM
> To: Sherry Sun <sherry.sun@....com>; Marco Felsch
> <m.felsch@...gutronix.de>
> Cc: 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 23/10/2024 16:16, Sherry Sun wrote:
> > This email is not from Hexagon's Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> >
> >
> >>>> -----Original Message-----
> >>>> From: Marco Felsch <m.felsch@...gutronix.de>
> >>>> Sent: Tuesday, October 22, 2024 4: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:
> >>>>>> -----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).
> >>>> NACK. Each active external chip needs power, this is supplied via
> >>>> an
> >>>> supply- rail and this is what vcc/vdd/va/vdio/v***-supply are used for.
> >>>>
> >>>> The PDn is a digital input signal which tells the chip to go into
> >>>> power- down/reset mode or not.
> >>>>
> >>>>> 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;
> >>>>> };
> >>>> This is what I meant previously, you do use a regualtor device for
> >>>> switching the PDn signal. This is not correct, albeit a lot of
> >>>> people are doing this because they don't want to adapt the driver. The
> 'gpio'
> >>>> within this regualtor should enable/disable this particular
> >>>> physical
> >> regualtor.
> >>> Sorry I see it differently. I checked the datasheet of NXP wifi
> >>> chip(taking IW612 as an example), the PDn pin is not the BT reset
> >>> pin, we usually take it as the PMIC_EN/WL_REG_ON pin to control the
> >>> whole
> >> chip power supply.
> >>> I think the reset-gpio added here should control the IND_RST_BT pin
> >>> (Independent software reset for Bluetooth), similar for the
> >>> IND_RST_WL pin(Independent software reset for Wi-Fi).
> >>>
> >>> Best Regards
> >>> Sherry
> >> PDn is not the BT reset :
> >>
> >> - PDn is a chip reset and is shared b/w BT and WIFI : hence, it needs
> >> to be managed as a reset control
> >>
> > Ok, so can you please also send out the corresponding wifi driver
> > changes for the reset control for reference? Otherwise I suppose the
> > wifi part will be powered off unexpectedly if unload the BT driver with your
> patch.
> >
> > Best Regards
> > Sherry
>
> We use the NXP downstream driver mwifiex which doesn't have support for
> regulator or PDn.
>
> However, regulator is already supported by the MMC core (vmmc-supply).
>
> For PDn, we use mmc pwrseq simple driver that has been patched to add
> support for reset-control.
>
> Please check :
> https://lore.ke/
> rnel.org%2Fall%2F20241017131957.1171323-1-catalin.popescu%40leica-
> geosystems.com%2F&data=05%7C02%7Csherry.sun%40nxp.com%7C89459f5
> c23724f1bf16308dcf3704ce0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C0%7C638652910876819305%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7
> C%7C&sdata=6Kz7Fh1e%2F8iRZtTmb%2BHEWDeaTsG0D9tBa4TQjotZJwY%3D
> &reserved=0
>

Ok, thanks, the mmc change looks good for me, so there is no problem with the NXP SDIO wifi.
But how do you plan to handle the NXP PCIe wifi? We also need to make sure the BT patch won't break the PCIe wifi function.

Best Regards
Sherry

Powered by blists - more mailing lists