Message-ID: <9b09774e-d0ed-4c97-b6a0-e976580b5bb5@leica-geosystems.com>
Date: Tue, 22 Oct 2024 15:49:20 +0000
From: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>
To: Sherry Sun <sherry.sun@....com>, Marco Felsch <m.felsch@...gutronix.de>
CC: Amitkumar Karwar <amitkumar.karwar@....com>, Neeraj Sanjay Kale
	<neeraj.sanjaykale@....com>, "marcel@...tmann.org" <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 22/10/2024 16:24, Sherry Sun wrote:
> [收到此邮件的某些人通常不会收到来自 sherry.sun@....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@...gutronix.de>
>> Sent: Tuesday, October 22, 2024 4:23 PM
>> To: Sherry Sun <sherry.sun@....com>
>> Cc: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>; Amitkumar
>> Karwar <amitkumar.karwar@....com>; Neeraj Sanjay Kale
>> <neeraj.sanjaykale@....com>; marcel@...tmann.org;
>> luiz.dentz@...il.com; robh@...nel.org; krzk+dt@...nel.org;
>> conor+dt@...nel.org; p.zabel@...gutronix.de; linux-
>> bluetooth@...r.kernel.org; devicetree@...r.kernel.org; linux-
>> kernel@...r.kernel.org; GEO-CHHER-bsp-development <bsp-
>> development.geo@...ca-geosystems.com>; Krzysztof Kozlowski
>> <krzk@...nel.org>
>> Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for
>> supply and reset
>>
>> On 24-10-22, Sherry Sun wrote:
>>>
>>>> -----Original Message-----
>>>> From: Marco Felsch <m.felsch@...gutronix.de>
>>>> Sent: Tuesday, October 22, 2024 3:23 PM
>>>> To: Sherry Sun <sherry.sun@....com>
>>>> Cc: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>;
>>>> Amitkumar Karwar <amitkumar.karwar@....com>; Neeraj Sanjay Kale
>>>> <neeraj.sanjaykale@....com>; marcel@...tmann.org;
>>>> luiz.dentz@...il.com; robh@...nel.org; krzk+dt@...nel.org;
>>>> conor+dt@...nel.org; p.zabel@...gutronix.de; linux-
>>>> bluetooth@...r.kernel.org; devicetree@...r.kernel.org; linux-
>>>> kernel@...r.kernel.org; GEO-CHHER-bsp-development <bsp-
>>>> development.geo@...ca-geosystems.com>; Krzysztof Kozlowski
>>>> <krzk@...nel.org>
>>>> Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add
>>>> support for supply and reset
>>>>
>>>> On 24-10-22, Sherry Sun wrote:
>>>>>> On 24-10-21, Krzysztof Kozlowski wrote:
>>>>>>> On 21/10/2024 08:41, Marco Felsch wrote:
>>>>>>>> On 24-10-07, Krzysztof Kozlowski wrote:
>>>>>>>>> On 07/10/2024 14:58, POPESCU Catalin wrote:
>>>>>>>>>>>>> +  vcc-supply:
>>>>>>>>>>>>> +    description:
>>>>>>>>>>>>> +      phandle of the regulator that provides the supply
>> voltage.
>>>>>>>>>>>>> +
>>>>>>>>>>>>> +  reset-gpios:
>>>>>>>>>>>>> +    description:
>>>>>>>>>>>>> +      Chip powerdown/reset signal (PDn).
>>>>>>>>>>>>> +
>>>>>>>>>>>> Hi Catalin,
>>>>>>>>>>>>
>>>>>>>>>>>> For NXP WIFI/BT chip, WIFI and BT share the one PDn pin,
>>>>>>>>>>>> which
>>>>>> means that both wifi and BT controller will be powered on and
>>>>>> off at the same time.
>>>>>>>>>>>> Taking the M.2 NXP WIFI/BT module as an example,
>>>>>> pin56(W_DISABLE1) is connected to the WIFI/BT chip PDn pin, we
>>>>>> has already controlled this pin in the corresponding PCIe/SDIO
>>>>>> controller dts
>>>> nodes.
>>>>>>>>>>>> It is not clear to me what exactly pins for vcc-supply
>>>>>>>>>>>> and reset-gpios
>>>>>> you describing here. Can you help understand the corresponding
>>>>>> pins on M.2 interface as an example? Thanks.
>>>>>>>>>> Hi Sherry,
>>>>>>>>>>
>>>>>>>>>> Regulators and reset controls being refcounted, we can
>>>>>>>>>> then implement powerup sequence in both bluetooth/wlan
>>>>>>>>>> drivers and have the drivers operate independently. This
>>>>>>>>>> way bluetooth driver would has no dependance on the wlan
>> driver for :
>>>>>>>>>> - its power supply
>>>>>>>>>>
>>>>>>>>>> - its reset pin (PDn)
>>>>>>>>>>
>>>>>>>>>> - its firmware (being downloaded as part of the combo
>>>>>>>>>> firmware)
>>>>>>>>>>
>>>>>>>>>> For the wlan driver we use mmc power sequence to drive the
>>>>>>>>>> chip reset pin and there's another patchset that adds
>>>>>>>>>> support for reset control into the mmc pwrseq simple driver.
>>>>>>>>>>
>>>>>>>>>>> Please wrap your replies.
>>>>>>>>>>>
>>>>>>>>>>> It seems you need power sequencing just like Bartosz did
>>>>>>>>>>> for
>>>>>> Qualcomm WCN.
>>>>>>>>>> Hi Krzysztof,
>>>>>>>>>>
>>>>>>>>>> I'm not familiar with power sequencing, but looks like way
>>>>>>>>>> more complicated than reset controls. So, why power
>>>>>>>>>> sequencing is recommended here ? Is it b/c a supply is
>> involved ?
>>>>>>>>> Based on earlier message:
>>>>>>>>>
>>>>>>>>> "For NXP WIFI/BT chip, WIFI and BT share the one PDn pin,
>>>>>>>>> which means that both wifi and BT controller will be
>>>>>>>>> powered on and off at the same time."
>>>>>>>>>
>>>>>>>>> but maybe that's not needed. No clue, I don't know the hardware.
>>>>>>>>> But be carefully what you write in the bindings, because
>>>>>>>>> then it will be
>>>>>> ABI.
>>>>>>>> We noticed the new power-sequencing infrastructure which is
>>>>>>>> part of
>>>>>>>> 6.11 too but I don't think that this patch is wrong. The DT
>>>>>>>> ABI won't break if we switch to the power-sequencing later
>>>>>>>> on since the
>>>>>> "reset-gpios"
>>>>>>>> are not marked as required. So it is up to the driver to
>>>>>>>> handle it either via a separate power-sequence driver or via
>> "power-supply"
>>>>>>>> and "reset-gpios" directly.
>>>>>>> That's not the point. We expect correct hardware description.
>>>>>>> If you say now it has "reset-gpios" but later say "actually
>>>>>>> no, because it has PMU", I respond: no. Describe the hardware,
>>>>>>> not current
>>>> Linux.
>>>>>> I know that DT abstracts the HW. That said I don't see the
>>>>>> problem with this patch. The HW is abstracted just fine:
>>>>>>
>>>>>> shared PDn          -> reset-gpios
>>>>>> shared power-supply -> vcc-supply
>>>>> Actually we should use vcc-supply to control the PDn pin, this is
>>>>> the power supply for NXP wifi/BT.
>>>> Please don't since this is regular pin on the wlan/bt device not the
>> regulator.
>>>> People often do that for GPIOs if the driver is missing the support
>>>> to pull the reset/pdn/enable gpio but the enable-gpio on the
>>>> regulator is to enable the regulator and _not_ the bt/wlan device.
>>>>
>>>> Therefore the implementation Catalin provided is the correct one.
>>>>
>>> For NXP wifi/BT, the PDn is the only power control pin, no specific
>>> regulator, per my understanding, it is a common way to configure this
>>> pin as the vcc-supply for the wifi interface(SDIO or PCIe).
>> NACK. Each active external chip needs power, this is supplied via an supply-
>> rail and this is what vcc/vdd/va/vdio/v***-supply are used for.
>>
>> The PDn is a digital input signal which tells the chip to go into power-
>> down/reset mode or not.
>>
>>> reg_usdhc3_vmmc: regulator-usdhc3 {
>>>           compatible = "regulator-fixed";
>>>           regulator-name = "WLAN_EN";
>>>           regulator-min-microvolt = <3300000>;
>>>           regulator-max-microvolt = <3300000>;
>>>           gpio = <&pcal6524 20 GPIO_ACTIVE_HIGH>;
>>>           enable-active-high;
>>> };
>> This is what I meant previously, you do use a regualtor device for switching
>> the PDn signal. This is not correct, albeit a lot of people are doing this
>> because they don't want to adapt the driver. The 'gpio'
>> within this regualtor should enable/disable this particular physical regualtor.
>>
> Sorry I see it differently. I checked the datasheet of NXP wifi chip(taking IW612
> as an example), the PDn pin is not the BT reset pin, we usually take it as the
> PMIC_EN/WL_REG_ON pin to control the whole chip power supply.
>
> I think the reset-gpio added here should control the IND_RST_BT pin
> (Independent software reset for Bluetooth), similar for the
> IND_RST_WL pin(Independent software reset for Wi-Fi).
>
> Best Regards
> Sherry

PDn is not the BT reset :

- PDn is a chip reset and is shared b/w BT and WIFI : hence, it needs to 
be managed as a reset control

- 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.

Powered by blists - more mailing lists