| Message ID | 20241004113557.2851060-1-catalin.popescu@leica-geosystems.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: net: bluetooth: nxp: add support for supply and reset | expand |
> -----Original Message----- > From: Catalin Popescu <catalin.popescu@leica-geosystems.com> > Sent: Friday, October 4, 2024 7:36 PM > To: Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de > Cc: linux-bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; m.felsch@pengutronix.de; bsp- > development.geo@leica-geosystems.com; Catalin Popescu > <catalin.popescu@leica-geosystems.com> > Subject: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for supply > and reset > > Add support for chip power supply and chip reset/powerdown. > > Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com> > --- > .../bindings/net/bluetooth/nxp,88w8987-bt.yaml | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- > bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- > bt.yaml > index 37a65badb448..8520b3812bd2 100644 > --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- > bt.yaml > +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- > bt.yaml > @@ -34,6 +34,14 @@ properties: > firmware-name: > maxItems: 1 > > + 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. Best Regards Sherry > required: > - compatible > > @@ -41,10 +49,13 @@ additionalProperties: false > > examples: > - | > + #include <dt-bindings/gpio/gpio.h> > serial { > bluetooth { > compatible = "nxp,88w8987-bt"; > fw-init-baudrate = <3000000>; > firmware-name = "uartuart8987_bt_v0.bin"; > + vcc-supply = <&nxp_iw612_supply>; > + reset-gpios = <&gpioctrl 2 GPIO_ACTIVE_LOW>; > }; > }; > -- > 2.34.1 >
On 06/10/2024 10:49, Sherry Sun wrote: > > >> -----Original Message----- >> From: Catalin Popescu <catalin.popescu@leica-geosystems.com> >> Sent: Friday, October 4, 2024 7:36 PM >> To: Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >> conor+dt@kernel.org; p.zabel@pengutronix.de >> Cc: linux-bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >> kernel@vger.kernel.org; m.felsch@pengutronix.de; bsp- >> development.geo@leica-geosystems.com; Catalin Popescu >> <catalin.popescu@leica-geosystems.com> >> Subject: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for supply >> and reset >> >> Add support for chip power supply and chip reset/powerdown. >> >> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com> >> --- >> .../bindings/net/bluetooth/nxp,88w8987-bt.yaml | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >> bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >> bt.yaml >> index 37a65badb448..8520b3812bd2 100644 >> --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >> bt.yaml >> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >> bt.yaml >> @@ -34,6 +34,14 @@ properties: >> firmware-name: >> maxItems: 1 >> >> + 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. Please wrap your replies. It seems you need power sequencing just like Bartosz did for Qualcomm WCN. Best regards, Krzysztof
On 06/10/2024 13:33, Krzysztof Kozlowski wrote: > [Some people who received this message don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email. > > > On 06/10/2024 10:49, Sherry Sun wrote: >> >>> -----Original Message----- >>> From: Catalin Popescu <catalin.popescu@leica-geosystems.com> >>> Sent: Friday, October 4, 2024 7:36 PM >>> To: Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >>> conor+dt@kernel.org; p.zabel@pengutronix.de >>> Cc: linux-bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >>> kernel@vger.kernel.org; m.felsch@pengutronix.de; bsp- >>> development.geo@leica-geosystems.com; Catalin Popescu >>> <catalin.popescu@leica-geosystems.com> >>> Subject: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for supply >>> and reset >>> >>> Add support for chip power supply and chip reset/powerdown. >>> >>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com> >>> --- >>> .../bindings/net/bluetooth/nxp,88w8987-bt.yaml | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >>> bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >>> bt.yaml >>> index 37a65badb448..8520b3812bd2 100644 >>> --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >>> bt.yaml >>> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987- >>> bt.yaml >>> @@ -34,6 +34,14 @@ properties: >>> firmware-name: >>> maxItems: 1 >>> >>> + 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 ? > > Best regards, > Krzysztof >
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. Best regards, Krzysztof
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. Regards, Marco > Best regards, > Krzysztof > >
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. Best regards, Krzysztof
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 Right now the DT ABI for the BT part is incomplete since it assume a running WLAN part or some hog-gpios to pull the device out-of-reset which is addressed by this patchset. Making use of the new power-sequencing fw is a Linux detail and I don't see why the DT can't be extended later on. We always extend the DT if something is missing or if we found a better way to handle devices. Regards, Marco
On 21/10/2024 12:25, Marco Felsch 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 > > Right now the DT ABI for the BT part is incomplete since it assume a > running WLAN part or some hog-gpios to pull the device out-of-reset > which is addressed by this patchset. > > Making use of the new power-sequencing fw is a Linux detail and I don't > see why the DT can't be extended later on. We always extend the DT if > something is missing or if we found a better way to handle devices. Sure, although I am not really confident that you understand the implications - you will not be able to switch to proper power-sequencing with above bindings, because it will not be just possible without breaking the ABI or changing hardware description (which you say it is "fine", so complete/done). I am fine with it, just mind the implications. Best regards, Krzysztof
> -----Original Message----- > From: Marco Felsch <m.felsch@pengutronix.de> > Sent: Monday, October 21, 2024 6:26 PM > To: Krzysztof Kozlowski <krzk@kernel.org> > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Sherry Sun > <sherry.sun@nxp.com>; Amitkumar Karwar <amitkumar.karwar@nxp.com>; > Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > development.geo@leica-geosystems.com> > Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for > supply and reset > > 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. Best Regards Sherry
On 24-10-22, Krzysztof Kozlowski wrote: > On 21/10/2024 12:25, Marco Felsch wrote: > > On 24-10-21, Krzysztof Kozlowski wrote: > >> On 21/10/2024 08:41, Marco Felsch wrote: > >>> On 24-10-07, Krzysztof Kozlowski wrote: ... > >>>> 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 > > > > Right now the DT ABI for the BT part is incomplete since it assume a > > running WLAN part or some hog-gpios to pull the device out-of-reset > > which is addressed by this patchset. > > > > Making use of the new power-sequencing fw is a Linux detail and I don't > > see why the DT can't be extended later on. We always extend the DT if > > something is missing or if we found a better way to handle devices. > > Sure, although I am not really confident that you understand the > implications - you will not be able to switch to proper power-sequencing > with above bindings, because it will not be just possible without > breaking the ABI or changing hardware description (which you say it is > "fine", so complete/done). I am fine with it, just mind the implications. Sorry can you please share your concerns? I don't get the point yet why we do break the DT ABI if we are going from bt { reset-gpios = <&gpio 4 0>; vcc-supply = <&supply>; }; to bt { vcc-supply = <&pmu_supply>; }; or: bt { pmu = <&pmu>; }; Of course the driver need to support all 2/3 cases due to backward compatibility but from DT pov I don't see any breakage since we already need to define the power handling properties (gpio & supply) as optional. That beeing said I don't see the need for a PMU driver for this WLAN/BT combi chip which is way simpler than the Qualcomm one from Bartosz. Also there is physically no PMU device which powers the chip unlike the Qualcomm one. I'm not sure if you would accept virtual PMU devices. Regards, Marco > Best regards, > Krzysztof > >
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. Regards, Marco
On 22/10/2024 09:12, Marco Felsch wrote: > On 24-10-22, Krzysztof Kozlowski wrote: >> On 21/10/2024 12:25, Marco Felsch wrote: >>> On 24-10-21, Krzysztof Kozlowski wrote: >>>> On 21/10/2024 08:41, Marco Felsch wrote: >>>>> On 24-10-07, Krzysztof Kozlowski wrote: > > ... > >>>>>> 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 >>> >>> Right now the DT ABI for the BT part is incomplete since it assume a >>> running WLAN part or some hog-gpios to pull the device out-of-reset >>> which is addressed by this patchset. >>> >>> Making use of the new power-sequencing fw is a Linux detail and I don't >>> see why the DT can't be extended later on. We always extend the DT if >>> something is missing or if we found a better way to handle devices. >> >> Sure, although I am not really confident that you understand the >> implications - you will not be able to switch to proper power-sequencing >> with above bindings, because it will not be just possible without >> breaking the ABI or changing hardware description (which you say it is >> "fine", so complete/done). I am fine with it, just mind the implications. > > Sorry can you please share your concerns? I don't get the point yet why > we do break the DT ABI if we are going from Not necessarily breaking ABI, but changing the description. > > bt { > reset-gpios = <&gpio 4 0>; > vcc-supply = <&supply>; > }; > > to > > bt { > vcc-supply = <&pmu_supply>; ...because you just removed reset-gpios which is a property of this device. > }; > > or: > > bt { > pmu = <&pmu>; > }; > > Of course the driver need to support all 2/3 cases due to backward > compatibility but from DT pov I don't see any breakage since we already > need to define the power handling properties (gpio & supply) as > optional. Either existing binding is complete or not. Not half-done. > > That beeing said I don't see the need for a PMU driver for this WLAN/BT > combi chip which is way simpler than the Qualcomm one from Bartosz. Also > there is physically no PMU device which powers the chip unlike the > Qualcomm one. I'm not sure if you would accept virtual PMU devices. Virtual PMU, of course not. I would like to have complete hardware description, not something which matches your current driver model. Best regards, Krzysztof
On 22/10/2024 09:30, Krzysztof Kozlowski wrote: > On 22/10/2024 09:12, Marco Felsch wrote: >> On 24-10-22, Krzysztof Kozlowski wrote: >>> On 21/10/2024 12:25, Marco Felsch wrote: >>>> On 24-10-21, Krzysztof Kozlowski wrote: >>>>> On 21/10/2024 08:41, Marco Felsch wrote: >>>>>> On 24-10-07, Krzysztof Kozlowski wrote: >> >> ... >> >>>>>>> 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 >>>> >>>> Right now the DT ABI for the BT part is incomplete since it assume a >>>> running WLAN part or some hog-gpios to pull the device out-of-reset >>>> which is addressed by this patchset. >>>> >>>> Making use of the new power-sequencing fw is a Linux detail and I don't >>>> see why the DT can't be extended later on. We always extend the DT if >>>> something is missing or if we found a better way to handle devices. >>> >>> Sure, although I am not really confident that you understand the >>> implications - you will not be able to switch to proper power-sequencing >>> with above bindings, because it will not be just possible without >>> breaking the ABI or changing hardware description (which you say it is >>> "fine", so complete/done). I am fine with it, just mind the implications. >> >> Sorry can you please share your concerns? I don't get the point yet why >> we do break the DT ABI if we are going from > > Not necessarily breaking ABI, but changing the description. >> >> bt { >> reset-gpios = <&gpio 4 0>; >> vcc-supply = <&supply>; >> }; >> >> to >> >> bt { >> vcc-supply = <&pmu_supply>; > > ...because you just removed reset-gpios which is a property of this device. > >> }; >> >> or: >> >> bt { >> pmu = <&pmu>; Ah, and I forgot here: this also might not be correct, because if you have PMU, then the PMU consumes VCC, not the Bluetooth. Therefore both of above two solutions might be inaccurate description if you decide to go with PMU. >> }; >> >> Of course the driver need to support all 2/3 cases due to backward >> compatibility but from DT pov I don't see any breakage since we already >> need to define the power handling properties (gpio & supply) as >> optional. > > Either existing binding is complete or not. Not half-done. > >> >> That beeing said I don't see the need for a PMU driver for this WLAN/BT >> combi chip which is way simpler than the Qualcomm one from Bartosz. Also >> there is physically no PMU device which powers the chip unlike the >> Qualcomm one. I'm not sure if you would accept virtual PMU devices. > > Virtual PMU, of course not. I would like to have complete hardware > description, not something which matches your current driver model. > > Best regards, > Krzysztof > Best regards, Krzysztof
> -----Original Message----- > From: Marco Felsch <m.felsch@pengutronix.de> > Sent: Tuesday, October 22, 2024 3:23 PM > To: Sherry Sun <sherry.sun@nxp.com> > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar > Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > <krzk@kernel.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 = <®_usdhc3_vmmc>; ... } Best Regards Sherry
On 24-10-22, Krzysztof Kozlowski wrote: > On 22/10/2024 09:30, Krzysztof Kozlowski wrote: > > On 22/10/2024 09:12, Marco Felsch wrote: > >> On 24-10-22, Krzysztof Kozlowski wrote: > >>> On 21/10/2024 12:25, Marco Felsch wrote: > >>>> On 24-10-21, Krzysztof Kozlowski wrote: > >>>>> On 21/10/2024 08:41, Marco Felsch wrote: > >>>>>> On 24-10-07, Krzysztof Kozlowski wrote: > >> > >> ... > >> > >>>>>>> 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 > >>>> > >>>> Right now the DT ABI for the BT part is incomplete since it assume a > >>>> running WLAN part or some hog-gpios to pull the device out-of-reset > >>>> which is addressed by this patchset. > >>>> > >>>> Making use of the new power-sequencing fw is a Linux detail and I don't > >>>> see why the DT can't be extended later on. We always extend the DT if > >>>> something is missing or if we found a better way to handle devices. > >>> > >>> Sure, although I am not really confident that you understand the > >>> implications - you will not be able to switch to proper power-sequencing > >>> with above bindings, because it will not be just possible without > >>> breaking the ABI or changing hardware description (which you say it is > >>> "fine", so complete/done). I am fine with it, just mind the implications. > >> > >> Sorry can you please share your concerns? I don't get the point yet why > >> we do break the DT ABI if we are going from > > > > Not necessarily breaking ABI, but changing the description. > >> > >> bt { > >> reset-gpios = <&gpio 4 0>; > >> vcc-supply = <&supply>; > >> }; > >> > >> to > >> > >> bt { > >> vcc-supply = <&pmu_supply>; > > > > ...because you just removed reset-gpios which is a property of this device. An optional property. That beeing said, dropping the *-gpios was the solution for the Qualcomm DTS as well: bd37ce2eeb84 ("arm64: dts: qcom: qrb5165-rb5: add the Wifi node") > >> }; > >> > >> or: > >> > >> bt { > >> pmu = <&pmu>; > > Ah, and I forgot here: this also might not be correct, because if you > have PMU, then the PMU consumes VCC, not the Bluetooth. Therefore both > of above two solutions might be inaccurate description if you decide to > go with PMU. > > >> }; > >> > >> Of course the driver need to support all 2/3 cases due to backward > >> compatibility but from DT pov I don't see any breakage since we already > >> need to define the power handling properties (gpio & supply) as > >> optional. > > > > Either existing binding is complete or not. Not half-done. As I remember DT ABI must be backward compatible. I understand this as followed: In our current use-case the dt-bindings don't describe any required hw resource so we need to mark them as optional to be backward compatible. Regarding your above comment: "complete or not. Not half-done". Do you see the current dt-bindings for this particular device as complete or not? In other words can we mark the reset-gpios and vcc-supply properties as required albeit this would break the DT ABI since all current users don't specify it? > >> That beeing said I don't see the need for a PMU driver for this WLAN/BT > >> combi chip which is way simpler than the Qualcomm one from Bartosz. Also > >> there is physically no PMU device which powers the chip unlike the > >> Qualcomm one. I'm not sure if you would accept virtual PMU devices. > > > > Virtual PMU, of course not. I would like to have complete hardware > > description, not something which matches your current driver model. Okay so PMU is no option and we don't have to consider this idea any longer. So reset-gpios and vcc-supply it is :) and I don't expect this to change. Regards, Marco
On 22/10/2024 10:09, Sherry Sun wrote: > [收到此邮件的某些人通常不会收到来自 sherry.sun@nxp.com 的电子邮件。请访问 https://aka.ms/LearnAboutSenderIdentification，以了解为什么这一点很重要。] > > 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@pengutronix.de> >> Sent: Tuesday, October 22, 2024 3:23 PM >> To: Sherry Sun <sherry.sun@nxp.com> >> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar >> Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- >> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- >> development.geo@leica-geosystems.com>; Krzysztof Kozlowski >> <krzk@kernel.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 = <®_usdhc3_vmmc>; > ... > } > > Best Regards > Sherry Hi Sherry, I'm sorry but I have to disagree. I checked again block diagrams for IW611, IW612, IW416 which are available on NXP website and they clearly differentiate between power supply(s) and power-down. Power-down is actually a reset and should be treated as such in the DT, not as a supply regulator. BR, Catalin
On 24-10-22, Sherry Sun wrote: > > > > -----Original Message----- > > From: Marco Felsch <m.felsch@pengutronix.de> > > Sent: Tuesday, October 22, 2024 3:23 PM > > To: Sherry Sun <sherry.sun@nxp.com> > > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar > > Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > > <krzk@kernel.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. Regards, Marco > &usdhc3 { > ... > vmmc-supply = <®_usdhc3_vmmc>; > ... > } > > Best Regards > Sherry >
On 22/10/2024 10:13, Marco Felsch wrote: > On 24-10-22, Krzysztof Kozlowski wrote: >> On 22/10/2024 09:30, Krzysztof Kozlowski wrote: >>> On 22/10/2024 09:12, Marco Felsch wrote: >>>> On 24-10-22, Krzysztof Kozlowski wrote: >>>>> On 21/10/2024 12:25, Marco Felsch wrote: >>>>>> On 24-10-21, Krzysztof Kozlowski wrote: >>>>>>> On 21/10/2024 08:41, Marco Felsch wrote: >>>>>>>> On 24-10-07, Krzysztof Kozlowski wrote: >>>> >>>> ... >>>> >>>>>>>>> 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 >>>>>> >>>>>> Right now the DT ABI for the BT part is incomplete since it assume a >>>>>> running WLAN part or some hog-gpios to pull the device out-of-reset >>>>>> which is addressed by this patchset. >>>>>> >>>>>> Making use of the new power-sequencing fw is a Linux detail and I don't >>>>>> see why the DT can't be extended later on. We always extend the DT if >>>>>> something is missing or if we found a better way to handle devices. >>>>> >>>>> Sure, although I am not really confident that you understand the >>>>> implications - you will not be able to switch to proper power-sequencing >>>>> with above bindings, because it will not be just possible without >>>>> breaking the ABI or changing hardware description (which you say it is >>>>> "fine", so complete/done). I am fine with it, just mind the implications. >>>> >>>> Sorry can you please share your concerns? I don't get the point yet why >>>> we do break the DT ABI if we are going from >>> >>> Not necessarily breaking ABI, but changing the description. >>>> >>>> bt { >>>> reset-gpios = <&gpio 4 0>; >>>> vcc-supply = <&supply>; >>>> }; >>>> >>>> to >>>> >>>> bt { >>>> vcc-supply = <&pmu_supply>; >>> >>> ...because you just removed reset-gpios which is a property of this device. > > An optional property. That beeing said, dropping the *-gpios was the > solution for the Qualcomm DTS as well: > > bd37ce2eeb84 ("arm64: dts: qcom: qrb5165-rb5: add the Wifi node") True, the difference is I think that we came with proper PMU model only recently while above device is supported for few years. This is not the case here: you can choose now hardware description which will be both accurate and solve power sequencing issues. > >>>> }; >>>> >>>> or: >>>> >>>> bt { >>>> pmu = <&pmu>; >> >> Ah, and I forgot here: this also might not be correct, because if you >> have PMU, then the PMU consumes VCC, not the Bluetooth. Therefore both >> of above two solutions might be inaccurate description if you decide to >> go with PMU. >> >>>> }; >>>> >>>> Of course the driver need to support all 2/3 cases due to backward >>>> compatibility but from DT pov I don't see any breakage since we already >>>> need to define the power handling properties (gpio & supply) as >>>> optional. >>> >>> Either existing binding is complete or not. Not half-done. > > As I remember DT ABI must be backward compatible. I understand this as > followed: In our current use-case the dt-bindings don't describe any > required hw resource so we need to mark them as optional to be backward > compatible. > > Regarding your above comment: "complete or not. Not half-done". Do you > see the current dt-bindings for this particular device as complete or > not? In other words can we mark the reset-gpios and vcc-supply > properties as required albeit this would break the DT ABI since all > current users don't specify it? It is not about required property. Does this device has reset lines or not? If yes, then please do not come in 2 years removing it from DTS. Because this breaks all of DTS users. > >>>> That beeing said I don't see the need for a PMU driver for this WLAN/BT >>>> combi chip which is way simpler than the Qualcomm one from Bartosz. Also >>>> there is physically no PMU device which powers the chip unlike the >>>> Qualcomm one. I'm not sure if you would accept virtual PMU devices. >>> >>> Virtual PMU, of course not. I would like to have complete hardware >>> description, not something which matches your current driver model. > > Okay so PMU is no option and we don't have to consider this idea any > longer. So reset-gpios and vcc-supply it is :) and I don't expect this > to change. Ack. Best regards, Krzysztof
> -----Original Message----- > From: Marco Felsch <m.felsch@pengutronix.de> > Sent: Tuesday, October 22, 2024 4:23 PM > To: Sherry Sun <sherry.sun@nxp.com> > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar > Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > <krzk@kernel.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@pengutronix.de> > > > Sent: Tuesday, October 22, 2024 3:23 PM > > > To: Sherry Sun <sherry.sun@nxp.com> > > > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; > > > Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > > > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > > > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > > > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > > > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > > > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > > > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > > > <krzk@kernel.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
On 22/10/2024 16:24, Sherry Sun wrote: > [收到此邮件的某些人通常不会收到来自 sherry.sun@nxp.com 的电子邮件。请访问 https://aka.ms/LearnAboutSenderIdentification，以了解为什么这一点很重要。] > > 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@pengutronix.de> >> Sent: Tuesday, October 22, 2024 4:23 PM >> To: Sherry Sun <sherry.sun@nxp.com> >> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar >> Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- >> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- >> development.geo@leica-geosystems.com>; Krzysztof Kozlowski >> <krzk@kernel.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@pengutronix.de> >>>> Sent: Tuesday, October 22, 2024 3:23 PM >>>> To: Sherry Sun <sherry.sun@nxp.com> >>>> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; >>>> Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >>>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >>>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >>>> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- >>>> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >>>> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- >>>> development.geo@leica-geosystems.com>; Krzysztof Kozlowski >>>> <krzk@kernel.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 - BT reset is specific to BT and can be handled as a simple gpio as it's not being shared with other driver (e.g WIFI) I've only added support for power-supply and PDn. BT specific reset has been ignored so far as it's optional software reset and it can be left open if not needed in the design.
> > > >> -----Original Message----- > >> From: Marco Felsch <m.felsch@pengutronix.de> > >> Sent: Tuesday, October 22, 2024 4:23 PM > >> To: Sherry Sun <sherry.sun@nxp.com> > >> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; > Amitkumar > >> Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > >> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > >> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > >> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > >> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > >> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > >> development.geo@leica-geosystems.com>; Krzysztof Kozlowski > >> <krzk@kernel.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@pengutronix.de> > >>>> Sent: Tuesday, October 22, 2024 3:23 PM > >>>> To: Sherry Sun <sherry.sun@nxp.com> > >>>> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; > >>>> Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > >>>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > >>>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > >>>> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > >>>> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > >>>> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > >>>> development.geo@leica-geosystems.com>; Krzysztof Kozlowski > >>>> <krzk@kernel.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 > - BT reset is specific to BT and can be handled as a simple gpio as it's not > being shared with other driver (e.g WIFI) > > I've only added support for power-supply and PDn. > > BT specific reset has been ignored so far as it's optional software reset and it > can be left open if not needed in the design. >
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@pengutronix.de> >>>> Sent: Tuesday, October 22, 2024 4:23 PM >>>> To: Sherry Sun <sherry.sun@nxp.com> >>>> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; >> Amitkumar >>>> Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >>>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >>>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >>>> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- >>>> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >>>> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- >>>> development.geo@leica-geosystems.com>; Krzysztof Kozlowski >>>> <krzk@kernel.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@pengutronix.de> >>>>>> Sent: Tuesday, October 22, 2024 3:23 PM >>>>>> To: Sherry Sun <sherry.sun@nxp.com> >>>>>> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; >>>>>> Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale >>>>>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; >>>>>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; >>>>>> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- >>>>>> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- >>>>>> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- >>>>>> development.geo@leica-geosystems.com>; Krzysztof Kozlowski >>>>>> <krzk@kernel.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.kernel.org/all/20241017131957.1171323-1-catalin.popescu@leica-geosystems.com/ BR, > >> - BT reset is specific to BT and can be handled as a simple gpio as it's not >> being shared with other driver (e.g WIFI) >> >> I've only added support for power-supply and PDn. >> >> BT specific reset has been ignored so far as it's optional software reset and it >> can be left open if not needed in the design. >>
> -----Original Message----- > From: POPESCU Catalin <catalin.popescu@leica-geosystems.com> > Sent: Wednesday, October 23, 2024 10:38 PM > To: Sherry Sun <sherry.sun@nxp.com>; Marco Felsch > <m.felsch@pengutronix.de> > Cc: Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > <krzk@kernel.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@pengutronix.de> > >>>> Sent: Tuesday, October 22, 2024 4:23 PM > >>>> To: Sherry Sun <sherry.sun@nxp.com> > >>>> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; > >> Amitkumar > >>>> Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > >>>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > >>>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > >>>> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > >>>> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > >>>> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > >>>> development.geo@leica-geosystems.com>; Krzysztof Kozlowski > >>>> <krzk@kernel.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@pengutronix.de> > >>>>>> Sent: Tuesday, October 22, 2024 3:23 PM > >>>>>> To: Sherry Sun <sherry.sun@nxp.com> > >>>>>> Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; > >>>>>> Amitkumar Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay > Kale > >>>>>> <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > >>>>>> luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > >>>>>> conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > >>>>>> bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > >>>>>> kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > >>>>>> development.geo@leica-geosystems.com>; Krzysztof Kozlowski > >>>>>> <krzk@kernel.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
Hi, On 24-10-28, Sherry Sun wrote: > > > From: POPESCU Catalin <catalin.popescu@leica-geosystems.com> > > > > 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. > > 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. Can you please elaborate how this could break the PCIe use-case? Regards, Marco
> -----Original Message----- > From: Marco Felsch <m.felsch@pengutronix.de> > Sent: Monday, October 28, 2024 5:00 PM > To: Sherry Sun <sherry.sun@nxp.com> > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar > Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > <krzk@kernel.org> > Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for > supply and reset > > Hi, > > On 24-10-28, Sherry Sun wrote: > > > > > From: POPESCU Catalin <catalin.popescu@leica-geosystems.com> > > > > > > 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. > > > > 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. > > Can you please elaborate how this could break the PCIe use-case? Similar to the SDIO wifi, if no corresponding reset control for the PDn pin in PCIe wifi driver, the wifi part will be unexpectedly powered off when removing the BT driver. Best Regards Sherry
On 24-10-28, Sherry Sun wrote: > > > From: Marco Felsch <m.felsch@pengutronix.de> > > > > Hi, > > > > On 24-10-28, Sherry Sun wrote: > > > > > > > From: POPESCU Catalin <catalin.popescu@leica-geosystems.com> > > > > > > > > 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. > > > > > > 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. > > > > Can you please elaborate how this could break the PCIe use-case? > > Similar to the SDIO wifi, if no corresponding reset control for the > PDn pin in PCIe wifi driver, the wifi part will be unexpectedly > powered off when removing the BT driver. Nope it's not that easy for PCIe case since the phy + link layer handling is much more complex compared to the MMC case. For the PCIe case the intial handling is very strict according to the PCIe spec and we can't handle the BT device independently. _BUT_ this patch doesn't cause any regression for the PCIe use-case since the support added by Catalin is optional which means that the user don't have to use these options. To sum up: WLAN (PCIe) used + BT (UART) used -> no independent handling possible. BT depends on WLAN. WLAN (PCIe) not used + BT (UART) used -> This patchset allow us to handle BT. Without the patchset this is not possible. WLAN (SDIO) + BT (UART) -> This patchset and the mmc-power-seq patchset allow us to handle WLAN and BT independently regardless if BT or WLAN is used or not. Regards, Marco
> -----Original Message----- > From: Marco Felsch <m.felsch@pengutronix.de> > Sent: Monday, October 28, 2024 7:52 PM > To: Sherry Sun <sherry.sun@nxp.com> > Cc: POPESCU Catalin <catalin.popescu@leica-geosystems.com>; Amitkumar > Karwar <amitkumar.karwar@nxp.com>; Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com>; marcel@holtmann.org; > luiz.dentz@gmail.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; p.zabel@pengutronix.de; linux- > bluetooth@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; GEO-CHHER-bsp-development <bsp- > development.geo@leica-geosystems.com>; Krzysztof Kozlowski > <krzk@kernel.org> > Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for > supply and reset > > On 24-10-28, Sherry Sun wrote: > > > > > From: Marco Felsch <m.felsch@pengutronix.de> > > > > > > Hi, > > > > > > On 24-10-28, Sherry Sun wrote: > > > > > > > > > From: POPESCU Catalin <catalin.popescu@leica-geosystems.com> > > > > > > > > > > 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. > > > > > > > > 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. > > > > > > Can you please elaborate how this could break the PCIe use-case? > > > > Similar to the SDIO wifi, if no corresponding reset control for the > > PDn pin in PCIe wifi driver, the wifi part will be unexpectedly > > powered off when removing the BT driver. > > Nope it's not that easy for PCIe case since the phy + link layer handling is > much more complex compared to the MMC case. For the PCIe case the intial > handling is very strict according to the PCIe spec and we can't handle the BT > device independently. > > _BUT_ this patch doesn't cause any regression for the PCIe use-case since the > support added by Catalin is optional which means that the user don't have to > use these options. > > To sum up: > > WLAN (PCIe) used + BT (UART) used -> no independent handling > possible. BT depends on WLAN. > > WLAN (PCIe) not used + BT (UART) used -> This patchset allow us to > handle BT. Without the patchset > this is not possible. > > WLAN (SDIO) + BT (UART) -> This patchset and the mmc-power-seq patchset > allow us to handle WLAN and BT independently > regardless if BT or WLAN is used or not. > If we add the reset-gpios property in the BT dts node when using the SDIO wifi chip, my concern is for some host platforms, taking i.MX95-19x19-EVK as an example, it supports both SDIO and PCIe interface wifi chip through the M.2 connector, when customers want to plug in the PCIe wifi chip, they have to remove the reset-gpios in the BT dts node to avoid the PCIe WLAN been affected by BT, right? And it looks strange that we can only add the reset-gpios BT property to the hosts that only support SDIO WLAN, we hope there is a solution for the PCIe WLAN too. Best Regards Sherry
On 24-10-28, Sherry Sun wrote: > > > From: Marco Felsch <m.felsch@pengutronix.de> > > > > On 24-10-28, Sherry Sun wrote: > > > > > > > From: Marco Felsch <m.felsch@pengutronix.de> > > > > > > > > Hi, > > > > > > > > On 24-10-28, Sherry Sun wrote: > > > > > > > > > > > From: POPESCU Catalin <catalin.popescu@leica-geosystems.com> > > > > > > > > > > > > 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. > > > > > > > > > > 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. > > > > > > > > Can you please elaborate how this could break the PCIe use-case? > > > > > > Similar to the SDIO wifi, if no corresponding reset control for the > > > PDn pin in PCIe wifi driver, the wifi part will be unexpectedly > > > powered off when removing the BT driver. > > > > Nope it's not that easy for PCIe case since the phy + link layer handling is > > much more complex compared to the MMC case. For the PCIe case the intial > > handling is very strict according to the PCIe spec and we can't handle the BT > > device independently. > > > > _BUT_ this patch doesn't cause any regression for the PCIe use-case since the > > support added by Catalin is optional which means that the user don't have to > > use these options. > > > > To sum up: > > > > WLAN (PCIe) used + BT (UART) used -> no independent handling > > possible. BT depends on WLAN. > > > > WLAN (PCIe) not used + BT (UART) used -> This patchset allow us to > > handle BT. Without the patchset > > this is not possible. > > > > WLAN (SDIO) + BT (UART) -> This patchset and the mmc-power-seq patchset > > allow us to handle WLAN and BT independently > > regardless if BT or WLAN is used or not. > > If we add the reset-gpios property in the BT dts node when using the > SDIO wifi chip, my concern is for some host platforms, taking > i.MX95-19x19-EVK as an example, it supports both SDIO and PCIe > interface wifi chip through the M.2 connector, when customers want to > plug in the PCIe wifi chip, they have to remove the reset-gpios in the > BT dts node to avoid the PCIe WLAN been affected by BT, right? I don't know the i.MX95-19x19-EVK platform since it is not upstream. If you want to support both: > > WLAN (PCIe) used + BT (UART) used -> no independent handling > > possible. BT depends on WLAN. and > > WLAN (SDIO) + BT (UART) -> This patchset and the mmc-power-seq patchset > > allow us to handle WLAN and BT independently > > regardless if BT or WLAN is used or not. you need to stick with the dependent handling which is no problem once this patchset get applied if your system support hot-plug. If hot-plug is not possible you could consider unsing overlays. However, this patchset does _NOT_ cause any regression neither for the MMC nor the PCIe use-case, and you don't have to touch your DTS files. It would be an improvement for platforms (not speaking of NXP EVK platforms) which utilize the MMC+UART interfaces only. > And it looks strange that we can only add the reset-gpios BT property > to the hosts that only support SDIO WLAN, we hope there is a solution > for the PCIe WLAN too. "We hope there is a solution" <-- This is not how upstream work. Also as said: The WLAN PCIe interface must/should be compatible with the PCIe Spec. There is no way that we can handle both devices independent since the PCIe spec specifies the power-up-sequence very strict. If for example, we do handle it independent and the BT part brings the device out-of-reset while the PCIe bus is not yet ready, the device's WLAN PCIe subsystem may get confused. There are two solution NXP could provide: - The PCIe WLAN/BT devices exposes all devices WLAN + BT via PCIe, this would eliminate the UART part. - All new WLAN/BT devices do have a separate hw reset line for each radio the device supports. Regards, Marco
diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml index 37a65badb448..8520b3812bd2 100644 --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml @@ -34,6 +34,14 @@ properties: firmware-name: maxItems: 1 + vcc-supply: + description: + phandle of the regulator that provides the supply voltage. + + reset-gpios: + description: + Chip powerdown/reset signal (PDn). + required: - compatible @@ -41,10 +49,13 @@ additionalProperties: false examples: - | + #include <dt-bindings/gpio/gpio.h> serial { bluetooth { compatible = "nxp,88w8987-bt"; fw-init-baudrate = <3000000>; firmware-name = "uartuart8987_bt_v0.bin"; + vcc-supply = <&nxp_iw612_supply>; + reset-gpios = <&gpioctrl 2 GPIO_ACTIVE_LOW>; }; };
Add support for chip power supply and chip reset/powerdown. Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com> --- .../bindings/net/bluetooth/nxp,88w8987-bt.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+)