linux-kernel - Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for supply and reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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

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