* [PATCH v5 0/5] Add AP6275P wireless support
@ 2024-07-30 3:30 Jacobe Zang
2024-07-30 3:30 ` [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d Jacobe Zang
` (4 more replies)
0 siblings, 5 replies; 25+ messages in thread
From: Jacobe Zang @ 2024-07-30 3:30 UTC (permalink / raw)
To: robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni,
conor+dt
Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi,
duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl,
nick, Jacobe Zang
These add AP6275P wireless support on Khadas Edge2. Enable 32k clock
for Wi-Fi module and extend the hardware IDs table in the brcmfmac
driver for it to attach.
Changes in v5:
- Add more commit message to the clock in bindings
- Use IS_ERR_OR_NULL as a judgment condition of clk
- Link to v4: https://lore.kernel.org/all/20240729070102.3770318-1-jacobe.zang@wesion.com/
Changes in v4:
- Change clock description in dt-bindings
- Move enable clk from pcie.c to of.c
- Add compatible for wifi node in DTS
- Add random seed flag for firmware download
- Link to v3: https://lore.kernel.org/all/20240630073605.2164346-1-jacobe.zang@wesion.com/
Changes in v3:
- Dropped redundant parts in dt-bindings.
- Change driver patch title prefix as 'wifi: brcmfmac:'.
- Change DTS Wi-Fi node clock-name as 'lpo'.
- Link to v2: https://lore.kernel.org/all/20240624081906.1399447-1-jacobe.zang@wesion.com/
Changes in v2:
- Add SoB tags for original developer.
- Add dt-bindings for pci14e4,449d and clocks.
- Replace dev_info to brcmf_dbg in pcie.c
- Link to v1: https://lore.kernel.org/all/20240620020015.4021696-1-jacobe.zang@wesion.com/
Jacobe Zang (5):
dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d
dt-bindings: net: wireless: brcm4329-fmac: add clock description for
AP6275P
arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2
wifi: brcmfmac: Add optional lpo clock enable support
wifi: brcmfmac: add flag for random seed during firmware download
.../net/wireless/brcm,bcm4329-fmac.yaml | 9 ++++
.../dts/rockchip/rk3588s-khadas-edge2.dts | 16 ++++++
.../wireless/broadcom/brcm80211/brcmfmac/of.c | 8 +++
.../broadcom/brcm80211/brcmfmac/pcie.c | 52 ++++++++++++++++---
.../broadcom/brcm80211/include/brcm_hw_ids.h | 2 +
5 files changed, 79 insertions(+), 8 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d 2024-07-30 3:30 [PATCH v5 0/5] Add AP6275P wireless support Jacobe Zang @ 2024-07-30 3:30 ` Jacobe Zang 2024-07-30 6:03 ` Krzysztof Kozlowski 2024-07-30 3:30 ` [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P Jacobe Zang ` (3 subsequent siblings) 4 siblings, 1 reply; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 3:30 UTC (permalink / raw) To: robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Jacobe Zang, Arend van Spriel It's the device id used by AP6275P which is the Wi-Fi module used by Rockchip's RK3588 evaluation board and also used in some other RK3588 boards. Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> --- .../devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml index e564f20d8f415..2c2093c77ec9a 100644 --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml @@ -53,6 +53,7 @@ properties: - pci14e4,4488 # BCM4377 - pci14e4,4425 # BCM4378 - pci14e4,4433 # BCM4387 + - pci14e4,449d # BCM43752 reg: description: SDIO function number for the device (for most cases -- 2.34.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d 2024-07-30 3:30 ` [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d Jacobe Zang @ 2024-07-30 6:03 ` Krzysztof Kozlowski 0 siblings, 0 replies; 25+ messages in thread From: Krzysztof Kozlowski @ 2024-07-30 6:03 UTC (permalink / raw) To: Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Arend van Spriel On 30/07/2024 05:30, Jacobe Zang wrote: > It's the device id used by AP6275P which is the Wi-Fi module > used by Rockchip's RK3588 evaluation board and also used in > some other RK3588 boards. > > Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 3:30 [PATCH v5 0/5] Add AP6275P wireless support Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d Jacobe Zang @ 2024-07-30 3:30 ` Jacobe Zang 2024-07-30 6:03 ` Krzysztof Kozlowski 2024-07-30 6:37 ` Arend Van Spriel 2024-07-30 3:30 ` [PATCH v5 3/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2 Jacobe Zang ` (2 subsequent siblings) 4 siblings, 2 replies; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 3:30 UTC (permalink / raw) To: robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Jacobe Zang, Arend van Spriel Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow external low power clock input. In DTS the clock as an optional choice in the absence of an internal clock. Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> --- .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml index 2c2093c77ec9a..a3607d55ef367 100644 --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml @@ -122,6 +122,14 @@ properties: NVRAM. This would normally be filled in by the bootloader from platform configuration data. + clocks: + items: + - description: External Low Power Clock input (32.768KHz) + + clock-names: + items: + - const: lpo + required: - compatible - reg -- 2.34.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 3:30 ` [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P Jacobe Zang @ 2024-07-30 6:03 ` Krzysztof Kozlowski 2024-07-30 6:37 ` Arend Van Spriel 1 sibling, 0 replies; 25+ messages in thread From: Krzysztof Kozlowski @ 2024-07-30 6:03 UTC (permalink / raw) To: Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Arend van Spriel On 30/07/2024 05:30, Jacobe Zang wrote: > Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow > external low power clock input. In DTS the clock as an optional choice in > the absence of an internal clock. > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 3:30 ` [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P Jacobe Zang 2024-07-30 6:03 ` Krzysztof Kozlowski @ 2024-07-30 6:37 ` Arend Van Spriel 2024-07-30 9:01 ` Krzysztof Kozlowski 1 sibling, 1 reply; 25+ messages in thread From: Arend Van Spriel @ 2024-07-30 6:37 UTC (permalink / raw) To: Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick + Linus W On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: > Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow > external low power clock input. In DTS the clock as an optional choice in > the absence of an internal clock. > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > --- > .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > index 2c2093c77ec9a..a3607d55ef367 100644 > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > @@ -122,6 +122,14 @@ properties: > NVRAM. This would normally be filled in by the bootloader from platform > configuration data. > > + clocks: > + items: > + - description: External Low Power Clock input (32.768KHz) > + > + clock-names: > + items: > + - const: lpo > + We still have an issue that this clock input is also present in the bindings specification broadcom-bluetooth.yaml (not in bluetooth subfolder). This clock is actually a chip resource. What happens if both are defined and both wifi and bt drivers try to enable this clock? Can this be expressed in yaml or can we only put a textual warning in the property descriptions? Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 6:37 ` Arend Van Spriel @ 2024-07-30 9:01 ` Krzysztof Kozlowski 2024-07-30 9:52 ` Arend Van Spriel 0 siblings, 1 reply; 25+ messages in thread From: Krzysztof Kozlowski @ 2024-07-30 9:01 UTC (permalink / raw) To: Arend Van Spriel, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick On 30/07/2024 08:37, Arend Van Spriel wrote: > + Linus W > > On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: > >> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >> external low power clock input. In DTS the clock as an optional choice in >> the absence of an internal clock. >> >> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >> --- >> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >> index 2c2093c77ec9a..a3607d55ef367 100644 >> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >> @@ -122,6 +122,14 @@ properties: >> NVRAM. This would normally be filled in by the bootloader from platform >> configuration data. >> >> + clocks: >> + items: >> + - description: External Low Power Clock input (32.768KHz) >> + >> + clock-names: >> + items: >> + - const: lpo >> + > > We still have an issue that this clock input is also present in the > bindings specification broadcom-bluetooth.yaml (not in bluetooth > subfolder). This clock is actually a chip resource. What happens if both > are defined and both wifi and bt drivers try to enable this clock? Can this > be expressed in yaml or can we only put a textual warning in the property > descriptions? Just like all clocks, what would happen? It will be enabled. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 9:01 ` Krzysztof Kozlowski @ 2024-07-30 9:52 ` Arend Van Spriel 2024-07-30 10:00 ` Jacobe Zang 2024-07-30 10:18 ` Krzysztof Kozlowski 0 siblings, 2 replies; 25+ messages in thread From: Arend Van Spriel @ 2024-07-30 9:52 UTC (permalink / raw) To: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 30/07/2024 08:37, Arend Van Spriel wrote: >> + Linus W >> >> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >> >>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>> external low power clock input. In DTS the clock as an optional choice in >>> the absence of an internal clock. >>> >>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>> --- >>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>> index 2c2093c77ec9a..a3607d55ef367 100644 >>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>> @@ -122,6 +122,14 @@ properties: >>> NVRAM. This would normally be filled in by the bootloader from platform >>> configuration data. >>> >>> + clocks: >>> + items: >>> + - description: External Low Power Clock input (32.768KHz) >>> + >>> + clock-names: >>> + items: >>> + - const: lpo >>> + >> >> We still have an issue that this clock input is also present in the >> bindings specification broadcom-bluetooth.yaml (not in bluetooth >> subfolder). This clock is actually a chip resource. What happens if both >> are defined and both wifi and bt drivers try to enable this clock? Can this >> be expressed in yaml or can we only put a textual warning in the property >> descriptions? > > Just like all clocks, what would happen? It will be enabled. Oh, wow! Cool stuff. But seriously is it not a problem to have two entities controlling one and the same clock? Is this use-case taken into account by the clock framework? Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 9:52 ` Arend Van Spriel @ 2024-07-30 10:00 ` Jacobe Zang 2024-07-30 10:08 ` Arend Van Spriel 2024-07-30 10:18 ` Krzysztof Kozlowski 1 sibling, 1 reply; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 10:00 UTC (permalink / raw) To: Arend Van Spriel, Krzysztof Kozlowski, robh@kernel.org, krzk+dt@kernel.org, heiko@sntech.de, kvalo@kernel.org, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, conor+dt@kernel.org, Linus Walleij Cc: efectn@protonmail.com, dsimic@manjaro.org, jagan@edgeble.ai, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, arend@broadcom.com, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, megi@xff.cz, duoming@zju.edu.cn, bhelgaas@google.com, minipli@grsecurity.net, brcm80211@lists.linux.dev, brcm80211-dev-list.pdl@broadcom.com, Nick Xie >> On 30/07/2024 08:37, Arend Van Spriel wrote: >>> + Linus W >>> >>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>> >>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>> external low power clock input. In DTS the clock as an optional choice in >>>> the absence of an internal clock. >>>> >>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>> --- >>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> @@ -122,6 +122,14 @@ properties: >>>> NVRAM. This would normally be filled in by the bootloader from platform >>>> configuration data. >>>> >>>> + clocks: >>>> + items: >>>> + - description: External Low Power Clock input (32.768KHz) >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: lpo >>>> + >>> >>> We still have an issue that this clock input is also present in the >>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>> subfolder). This clock is actually a chip resource. What happens if both >>> are defined and both wifi and bt drivers try to enable this clock? Can this >>> be expressed in yaml or can we only put a textual warning in the property >>> descriptions? >> >> Just like all clocks, what would happen? It will be enabled. > > Oh, wow! Cool stuff. But seriously is it not a problem to have two entities > controlling one and the same clock? Is this use-case taken into account by > the clock framework? I have enabled the same clock both in bluetooth and wifi just now, they worked well. Maybe this make sense? --- Best Regards Jacobe ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 10:00 ` Jacobe Zang @ 2024-07-30 10:08 ` Arend Van Spriel 2024-07-30 10:17 ` Jacobe Zang 0 siblings, 1 reply; 25+ messages in thread From: Arend Van Spriel @ 2024-07-30 10:08 UTC (permalink / raw) To: Jacobe Zang, Krzysztof Kozlowski, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, Nick Xie On July 30, 2024 12:00:25 PM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>> On 30/07/2024 08:37, Arend Van Spriel wrote: >>>> + Linus W >>>> >>>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>>> >>>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>>> external low power clock input. In DTS the clock as an optional choice in >>>>> the absence of an internal clock. >>>>> >>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>>> --- >>>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> @@ -122,6 +122,14 @@ properties: >>>>> NVRAM. This would normally be filled in by the bootloader from platform >>>>> configuration data. >>>>> >>>>> + clocks: >>>>> + items: >>>>> + - description: External Low Power Clock input (32.768KHz) >>>>> + >>>>> + clock-names: >>>>> + items: >>>>> + - const: lpo >>>>> + >>>> >>>> We still have an issue that this clock input is also present in the >>>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>>> subfolder). This clock is actually a chip resource. What happens if both >>>> are defined and both wifi and bt drivers try to enable this clock? Can this >>>> be expressed in yaml or can we only put a textual warning in the property >>>> descriptions? >>> >>> Just like all clocks, what would happen? It will be enabled. >> >> Oh, wow! Cool stuff. But seriously is it not a problem to have two entities >> controlling one and the same clock? Is this use-case taken into account by >> the clock framework? > > I have enabled the same clock both in bluetooth and wifi just now, they worked > well. Maybe this make sense? What happens if you unload one of the drivers? Also would like to know if you are using an nvram file. If so can you share it's content. Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 10:08 ` Arend Van Spriel @ 2024-07-30 10:17 ` Jacobe Zang 0 siblings, 0 replies; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 10:17 UTC (permalink / raw) To: Arend Van Spriel, Krzysztof Kozlowski, robh@kernel.org, krzk+dt@kernel.org, heiko@sntech.de, kvalo@kernel.org, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, conor+dt@kernel.org, Linus Walleij Cc: efectn@protonmail.com, dsimic@manjaro.org, jagan@edgeble.ai, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, arend@broadcom.com, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, megi@xff.cz, duoming@zju.edu.cn, bhelgaas@google.com, minipli@grsecurity.net, brcm80211@lists.linux.dev, brcm80211-dev-list.pdl@broadcom.com, Nick Xie >>>> On 30/07/2024 08:37, Arend Van Spriel wrote: >>>>> + Linus W >>>>> >>>>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>>>> >>>>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>>>> external low power clock input. In DTS the clock as an optional choice in >>>>>> the absence of an internal clock. >>>>>> >>>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>>>> --- >>>>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>>>> 1 file changed, 8 insertions(+) >>>>>> >>>>>> diff --git >>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>> @@ -122,6 +122,14 @@ properties: >>>>>> NVRAM. This would normally be filled in by the bootloader from platform >>>>>> configuration data. >>>>>> >>>>>> + clocks: >>>>>> + items: >>>>>> + - description: External Low Power Clock input (32.768KHz) >>>>>> + >>>>>> + clock-names: >>>>>> + items: >>>>>> + - const: lpo >>>>>> + >>>>> >>>>> We still have an issue that this clock input is also present in the >>>>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>>>> subfolder). This clock is actually a chip resource. What happens if both >>>>> are defined and both wifi and bt drivers try to enable this clock? Can this >>>>> be expressed in yaml or can we only put a textual warning in the property >>>>> descriptions? >>>> >>>> Just like all clocks, what would happen? It will be enabled. >>> >>> Oh, wow! Cool stuff. But seriously is it not a problem to have two entities >>> controlling one and the same clock? Is this use-case taken into account by >>> the clock framework? >> >> I have enabled the same clock both in bluetooth and wifi just now, they worked >> well. Maybe this make sense? > > What happens if you unload one of the drivers? Also would like to know if > you are using an nvram file. If so can you share it's content. After rmmod the wifi relevant driver, bluetooth still works well. The content of nvram text shows below: # AP6275P_NVRAM_V1.1_20200702 # AP6271P_V00 board, iPA version. # nvram copied and edited from AP6271P_EVB_V01 EVB board // # SSID generated using Alberto's boardssid.py script: # ********************SUMMARY******************** # Board Name: AP6271P_V00 #SSID: 0x086d #macmid: 0x02df # Successfully made SSID entry in sromdefs.tcl. # Successfully made macmid entry in sromdefs.tcl. # Successfully made SSID entry in tblssid.py. # ************************************************* # $ Copyright Broadcom $ # # # <<Broadcom-WL-IPTag/Proprietary:>> NVRAMRev=$Rev: 874188 $ sromrev=11 boardrev=0x1213 boardtype=0x08ed boardflags=0x00400201 boardflags2=0xc0800000 boardflags3=0x40002180 #boardnum=57410 macaddr=00:90:4c:12:d0:01 jtag_irw=38 #Regulatory specific ccode=0 regrev=0 # Board specific vendid=0x14e4 devid=0x449d manfid=0x2d0 antswitch=0 pdgain5g=0 pdgain2g=0 aa2g=3 aa5g=3 agbg0=2 agbg1=2 aga0=2 aga1=2 extpagain2g=2 extpagain5g=2 rxgains2gelnagaina0=0 rxgains2gtrisoa0=0 rxgains2gtrelnabypa0=0 rxgains5gelnagaina0=0 rxgains5gtrisoa0=0 rxgains5gtrelnabypa0=0 rxgains5gmelnagaina0=0 rxgains5gmtrisoa0=0 rxgains5gmtrelnabypa0=0 rxgains5ghelnagaina0=0 rxgains5ghtrisoa0=0 rxgains5ghtrelnabypa0=0 rxgains2gelnagaina1=0 rxgains2gtrisoa1=0 rxgains2gtrelnabypa1=0 rxgains5gelnagaina1=0 rxgains5gtrisoa1=0 rxgains5gtrelnabypa1=0 rxgains5gmelnagaina1=0 rxgains5gmtrisoa1=0 rxgains5gmtrelnabypa1=0 rxgains5ghelnagaina1=0 rxgains5ghtrisoa1=0 rxgains5ghtrelnabypa1=0 #RSSI related # 2G rssicorrnorm_c0=4,4 rssicorrnorm_c1=4,4 # 5G rssicorrnorm5g_c0=5,5,5,5,5,5,5,5,5,5,5,5 rssicorrnorm5g_c1=4,4,4,4,4,4,4,4,4,4,4,4 #Two range TSSI tworangetssi2g=0 tworangetssi5g=0 lowpowerrange2g=0 lowpowerrange5g=0 low_adc_rate_en=1 nocrc=1 otpimagesize=502 xtalfreq=37400 txchain=3 rxchain=3 cckdigfilttype=2 #bit mask for slice capability bit 0:2G bit 1:5G bandcap=3 #TXBF Related rpcal2g=0 rpcal5gb0=0 rpcal5gb1=0 rpcal5gb2=0 rpcal5gb3=0 #FDSS Related fdss_level_2g=4,4 fdss_interp_en=1 fdss_level_5g=3,3 fdss_level_11ax_2g=3,3 fdss_level_11ax_2g_ch1=3,3 fdss_level_11ax_2g_ch11=3,3 fdss_level_11ax_5g=3,3 #Tempsense Related tempthresh=255 tempoffset=40 rawtempsense=0x1ff phycal_tempdelta=15 temps_period=15 temps_hysteresis=15 #------------- TSSI Related ------------- tssipos2g=1 tssipos5g=1 AvVmid_c0=2,127,4,92,4,91,4,91,4,94 AvVmid_c1=2,127,4,93,4,93,4,95,3,110 # CCK in case of multi mode 2 pa2gccka0=-137,7810,-928 pa2gccka1=-139,7853,-929 # OFDM in case of multi_mode 2 pa2ga0=-103,7727,-855 pa2ga1=-126,7258,-826 pa5ga0=-176,5703,-703,-180,5747,-708,-165,5994,-732,-146,6299,-757 pa5ga1=-132,6132,-760,-107,6472,-769,-142,6184,-752,-108,7237,-858 # Max power and offsets maxp2ga0=86 maxp2ga1=86 maxp5ga0=74,74,74,74 maxp5ga1=68,68,68,70 subband5gver=0x4 paparambwver=3 cckpwroffset0=0 cckpwroffset1=0 pdoffset40ma0=0x4433 pdoffset80ma0=0x3232 pdoffset40ma1=0x2333 pdoffset80ma1=0x1222 cckbw202gpo=0x2222 cckbw20ul2gpo=0 mcsbw202gpo=0x98533221 mcsbw402gpo=0x44000000 dot11agofdmhrbw202gpo=0x4444 ofdmlrbw202gpo=0x0033 mcsbw205glpo=0x49533322 mcsbw405glpo=0xE9443342 mcsbw805glpo=0xEC665542 mcsbw1605glpo=0 mcsbw205gmpo=0x49200000 mcsbw405gmpo=0xE9443342 mcsbw805gmpo=0xEC665542 mcsbw1605gmpo=0 mcsbw205ghpo=0x49312220 #mcsbw405ghpo=0x84-1-1-2-2-2-5 mcsbw405ghpo=0xC8221110 #mcsbw405ghpo=0x88555555 mcsbw805ghpo=0xCC443320 powoffs2gtna0=0,0,0,0,0,0,0,0,0,0,0,0,0,0 powoffs2gtna1=0,0,0,0,0,0,0,0,0,0,0,0,0,0 mcs1024qam2gpo=0xDDDD mcs1024qam5glpo=0xFFFFCC mcs1024qam5gmpo=0xFFFFCC mcs1024qam5ghpo=0xFFFFCC mcs1024qam5gx1po=0xFFFFCC mcs1024qam5gx2po=0xFFFFCC mcs8poexp=0 mcs9poexp=0 mcs10poexp=0 # 5G power offset per channel for band edge channel powoffs5g20mtna0=0,0,0,0,0,0,0 powoffs5g20mtna1=0,0,0,0,0,0,0 powoffs5g40mtna0=0,0,0,0,0 powoffs5g40mtna1=0,0,0,0,0 powoffs5g80mtna0=0,0,0,0,0 powoffs5g80mtna1=0,0,0,0,0 mcs11poexp=0 #LTE Coex Related ltecxmux=0 ltecxpadnum=0x0504 ltecxfnsel=0x44 ltecxgcigpio=0x04 #OOB params #device_wake_opt=1 host_wake_opt=0 # SWCTRL Related swctrlmap_2g=0x10101010,0x06030401,0x04011010,0x000000,0x3FF swctrlmapext_2g=0x00000000,0x00000000,0x00000000,0x000000,0x000 swctrlmap_5g=0x80408040,0x21240120,0x01208040,0x000000,0x3FF swctrlmapext_5g=0x00000000,0x00000000,0x00000000,0x000000,0x000 clb2gslice0core0=0x01b clb2gslice1core0=0x000 clb5gslice0core0=0x064 clb5gslice1core0=0x000 clb2gslice0core1=0x056 clb2gslice1core1=0x000 clb5gslice0core1=0x0a1 clb5gslice1core1=0x000 btc_prisel_ant_mask=0x2 clb_swctrl_smask_ant0=0x27f clb_swctrl_smask_ant1=0x2f7 muxenab=1 #BT Coex 1:TDM btc_mode=1 # --- PAPD Cal related params ---- txwbpapden=0 # 0:NBPAPD 1:WBPAPD # NB PAPD Cal params nb_eps_offset=470,470 nb_bbmult=66,66 nb_papdcalidx=6,6 nb_txattn=2,2 nb_rxattn=1,1 nb_eps_stopidx=63 epsilonoff_5g20_c0=0,0,0,0 epsilonoff_5g40_c0=0,0,0,0 epsilonoff_5g80_c0=0,0,0,0 epsilonoff_5g20_c1=0,0,0,0 epsilonoff_5g40_c1=0,0,0,0 epsilonoff_5g80_c1=0,0,-1,-1 epsilonoff_2g20_c0=0 epsilonoff_2g20_c1=0 # energy detect threshold ed_thresh2g=-67 ed_thresh5g=-67 # energy detect threshold for EU eu_edthresh2g=-67 eu_edthresh5g=-67 #rpcal coef for imptxbf rpcal5gb0=238 rpcal5gb1=228 rpcal5gb2=222 rpcal5gb3=229 rpcal2g=15 ocl=1 bt_coex_chdep_div=1 # OLPC Related disable_olpc=0 olpc_thresh5g=32 olpc_anchor5g=40 olpc_thresh2g=32 olpc_anchor2g=40 #PAPR related paprdis=0 paprrmcsgamma2g=500,550,550,-1,-1,-1,-1,-1,-1,-1,-1,-1 paprrmcsgain2g=128,128,128,0,0,0,0,0,0,0,0,0 paprrmcsgamma2g_ch13=500,550,550,-1,-1,-1,-1,-1,-1,-1,-1,-1 paprrmcsgain2g_ch13=128,128,128,0,0,0,0,0,0,0,0,0 paprrmcsgamma2g_ch1=500,550,550,-1,-1,-1,-1,-1,-1,-1,-1,-1 paprrmcsgain2g_ch1=128,128,128,0,0,0,0,0,0,0,0,0 paprrmcsgamma5g20=500,500,500,-1,-1,-1,-1,-1,-1,-1,-1,-1 paprrmcsgain5g20=128,128,128,0,0,0,0,0,0,0,0,0 paprrmcsgamma5g40=600,600,600,-1,-1,-1,-1,-1,-1,-1,-1,-1 paprrmcsgain5g40=128,128,128,0,0,0,0,0,0,0,0,0 paprrmcsgamma5g80=-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1 paprrmcsgain5g80=0,0,0,0,0,0,0,0,0,0,0,0 # Enable papd for cck when target pwr ge 16dBm cckpapd_pwrthresh=64 ## ULOFDMA Board limit PPRs for 2G20 ## ruppr2g20bpska0=0x0 ruppr2g20bpska1=0x0 ruppr2g20qpska0=0x0 ruppr2g20qpska1=0x0 ruppr2g20qam16a0=0x0 ruppr2g20qam16a1=0x0 ruppr2g20qam64a0=0x1 ruppr2g20qam64a1=0x1 ruppr2g20qam256a0=0x21084 ruppr2g20qam256a1=0x21084 ruppr2g20qam1024a0=0x0 ruppr2g20qam1024a1=0x0 ## ULOFDMA Board limit PPRs for 5G20 ## ruppr5g20bpska0=0x20000 ruppr5g20bpska1=0x20000 ruppr5g20qpska0=0x18000 ruppr5g20qpska1=0x18000 ruppr5g20qam16a0=0x28000 ruppr5g20qam16a1=0x28000 ruppr5g20qam64a0=0x29086 ruppr5g20qam64a1=0x29086 ruppr5g20qam256a0=0x62908 ruppr5g20qam256a1=0x62908 ruppr5g20qam1024a0=0x70000 ruppr5g20qam1024a1=0x70000 ## ULOFDMA Board limit PPRs for 5G40 ## ruppr5g40bpska0=0x638000 ruppr5g40bpska1=0x638000 ruppr5g40qpska0=0x840020 ruppr5g40qpska1=0x840020 ruppr5g40qam16a0=0x638001 ruppr5g40qam16a1=0x638001 ruppr5g40qam64a0=0x739085 ruppr5g40qam64a1=0x739085 ruppr5g40qam256a0=0x106a108 ruppr5g40qam256a1=0x106a108 ruppr5g40qam1024a0=0x1078000 ruppr5g40qam1024a1=0x1078000 ## ULOFDMA Board limit PPRs for 5G80 ## ruppr5g80bpska0=0x0 ruppr5g80bpska1=0x0 ruppr5g80qpska0=0x0 ruppr5g80qpska1=0x0 ruppr5g80qam16a0=0x0 ruppr5g80qam16a1=0x0 ruppr5g80qam64a0=0x187218e7 ruppr5g80qam64a1=0x187218e7 ruppr5g80qam256a0=0x3904254a ruppr5g80qam256a1=0x3904254a ruppr5g80qam1024a0=0x49068000 ruppr5g80qam1024a1=0x49068000 --- Best Regards Jacobe ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 9:52 ` Arend Van Spriel 2024-07-30 10:00 ` Jacobe Zang @ 2024-07-30 10:18 ` Krzysztof Kozlowski 2024-07-30 11:16 ` Arend Van Spriel 1 sibling, 1 reply; 25+ messages in thread From: Krzysztof Kozlowski @ 2024-07-30 10:18 UTC (permalink / raw) To: Arend Van Spriel, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick On 30/07/2024 11:52, Arend Van Spriel wrote: > On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >> On 30/07/2024 08:37, Arend Van Spriel wrote: >>> + Linus W >>> >>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>> >>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>> external low power clock input. In DTS the clock as an optional choice in >>>> the absence of an internal clock. >>>> >>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>> --- >>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>> @@ -122,6 +122,14 @@ properties: >>>> NVRAM. This would normally be filled in by the bootloader from platform >>>> configuration data. >>>> >>>> + clocks: >>>> + items: >>>> + - description: External Low Power Clock input (32.768KHz) >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: lpo >>>> + >>> >>> We still have an issue that this clock input is also present in the >>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>> subfolder). This clock is actually a chip resource. What happens if both >>> are defined and both wifi and bt drivers try to enable this clock? Can this >>> be expressed in yaml or can we only put a textual warning in the property >>> descriptions? >> >> Just like all clocks, what would happen? It will be enabled. > > Oh, wow! Cool stuff. But seriously is it not a problem to have two entities > controlling one and the same clock? Is this use-case taken into account by > the clock framework? Yes, it is handled correctly. That's a basic use-case, handled by CCF since some years (~12?). Anyway, whatever OS is doing (or not doing) with the clocks is independent of the bindings here. The question is about hardware - does this node, which represents PCI interface of the chip, has/uses the clocks? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 10:18 ` Krzysztof Kozlowski @ 2024-07-30 11:16 ` Arend Van Spriel 2024-07-30 17:38 ` Sebastian Reichel 0 siblings, 1 reply; 25+ messages in thread From: Arend Van Spriel @ 2024-07-30 11:16 UTC (permalink / raw) To: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 30/07/2024 11:52, Arend Van Spriel wrote: >> On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >>> On 30/07/2024 08:37, Arend Van Spriel wrote: >>>> + Linus W >>>> >>>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>>> >>>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>>> external low power clock input. In DTS the clock as an optional choice in >>>>> the absence of an internal clock. >>>>> >>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>>> --- >>>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>> @@ -122,6 +122,14 @@ properties: >>>>> NVRAM. This would normally be filled in by the bootloader from platform >>>>> configuration data. >>>>> >>>>> + clocks: >>>>> + items: >>>>> + - description: External Low Power Clock input (32.768KHz) >>>>> + >>>>> + clock-names: >>>>> + items: >>>>> + - const: lpo >>>>> + >>>> >>>> We still have an issue that this clock input is also present in the >>>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>>> subfolder). This clock is actually a chip resource. What happens if both >>>> are defined and both wifi and bt drivers try to enable this clock? Can this >>>> be expressed in yaml or can we only put a textual warning in the property >>>> descriptions? >>> >>> Just like all clocks, what would happen? It will be enabled. >> >> Oh, wow! Cool stuff. But seriously is it not a problem to have two entities >> controlling one and the same clock? Is this use-case taken into account by >> the clock framework? > > Yes, it is handled correctly. That's a basic use-case, handled by CCF > since some years (~12?). Anyway, whatever OS is doing (or not doing) > with the clocks is independent of the bindings here. The question is Agree. Probably the bindings would not be the place to document this if it would be an issue. > about hardware - does this node, which represents PCI interface of the > chip, has/uses the clocks. The schematics I found for the wifi module and the khadas edge platform show these are indeed wired to the chip. Regards, Arend > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 11:16 ` Arend Van Spriel @ 2024-07-30 17:38 ` Sebastian Reichel 2024-07-31 12:57 ` Arend van Spriel 0 siblings, 1 reply; 25+ messages in thread From: Sebastian Reichel @ 2024-07-30 17:38 UTC (permalink / raw) To: Arend Van Spriel Cc: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij, efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick [-- Attachment #1: Type: text/plain, Size: 4097 bytes --] Hi, On Tue, Jul 30, 2024 at 01:16:57PM GMT, Arend Van Spriel wrote: > On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > On 30/07/2024 11:52, Arend Van Spriel wrote: > > > On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > On 30/07/2024 08:37, Arend Van Spriel wrote: > > > > > + Linus W > > > > > > > > > > On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: > > > > > > > > > > > Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow > > > > > > external low power clock input. In DTS the clock as an optional choice in > > > > > > the absence of an internal clock. > > > > > > > > > > > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > > > > > > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > > > > > > --- > > > > > > .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > index 2c2093c77ec9a..a3607d55ef367 100644 > > > > > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > @@ -122,6 +122,14 @@ properties: > > > > > > NVRAM. This would normally be filled in by the bootloader from platform > > > > > > configuration data. > > > > > > > > > > > > + clocks: > > > > > > + items: > > > > > > + - description: External Low Power Clock input (32.768KHz) > > > > > > + > > > > > > + clock-names: > > > > > > + items: > > > > > > + - const: lpo > > > > > > + > > > > > > > > > > We still have an issue that this clock input is also present in the > > > > > bindings specification broadcom-bluetooth.yaml (not in bluetooth > > > > > subfolder). This clock is actually a chip resource. What happens if both > > > > > are defined and both wifi and bt drivers try to enable this clock? Can this > > > > > be expressed in yaml or can we only put a textual warning in the property > > > > > descriptions? > > > > > > > > Just like all clocks, what would happen? It will be enabled. > > > > > > Oh, wow! Cool stuff. But seriously is it not a problem to have two entities > > > controlling one and the same clock? Is this use-case taken into account by > > > the clock framework? > > > > Yes, it is handled correctly. That's a basic use-case, handled by CCF > > since some years (~12?). Anyway, whatever OS is doing (or not doing) > > with the clocks is independent of the bindings here. The question is > > Agree. Probably the bindings would not be the place to document this if it > would be an issue. > > > about hardware - does this node, which represents PCI interface of the > > chip, has/uses the clocks. > > The schematics I found for the wifi module and the khadas edge platform show > these are indeed wired to the chip. I have a Rockchip RK3588 Evaluation Board on my desk, which uses the same WLAN AP6275P module. I think I already commented on a prior version of this series: The LPO clock is needed to make the PCIe device visible on the bus. That means this series only works if the clock has already been running. Otherwise the PCIe driver will never be probed. To become visible the devices requires: 1. The LPO clock to be enabled 2. Power to be applied 3. The WL_EN gpio to be configured correctly If one of the above is not met, the device will not even appear in 'lspci'. I believe the binding needs to take into consideration, that pwrseq is needed for the PCIe side. Fortuantely the heavy lifting of creating the proper infrastructure for this has already been done by Bartosz Golaszewski for Qualcomm WLAN chips. What is missing is a pwrseq driver for the Broadcom chip (or this specific module?). Greetings, -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-30 17:38 ` Sebastian Reichel @ 2024-07-31 12:57 ` Arend van Spriel 2024-07-31 13:54 ` Sebastian Reichel 0 siblings, 1 reply; 25+ messages in thread From: Arend van Spriel @ 2024-07-31 12:57 UTC (permalink / raw) To: Sebastian Reichel Cc: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij, efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Andy Green On 7/30/2024 7:38 PM, Sebastian Reichel wrote: > Hi, > > On Tue, Jul 30, 2024 at 01:16:57PM GMT, Arend Van Spriel wrote: >> On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >>> On 30/07/2024 11:52, Arend Van Spriel wrote: >>>> On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>> >>>>> On 30/07/2024 08:37, Arend Van Spriel wrote: >>>>>> + Linus W >>>>>> >>>>>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>>>>> >>>>>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>>>>> external low power clock input. In DTS the clock as an optional choice in >>>>>>> the absence of an internal clock. >>>>>>> >>>>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>>>>> --- >>>>>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>>>>> 1 file changed, 8 insertions(+) >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>>>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> @@ -122,6 +122,14 @@ properties: >>>>>>> NVRAM. This would normally be filled in by the bootloader from platform >>>>>>> configuration data. >>>>>>> >>>>>>> + clocks: >>>>>>> + items: >>>>>>> + - description: External Low Power Clock input (32.768KHz) >>>>>>> + >>>>>>> + clock-names: >>>>>>> + items: >>>>>>> + - const: lpo >>>>>>> + >>>>>> >>>>>> We still have an issue that this clock input is also present in the >>>>>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>>>>> subfolder). This clock is actually a chip resource. What happens if both >>>>>> are defined and both wifi and bt drivers try to enable this clock? Can this >>>>>> be expressed in yaml or can we only put a textual warning in the property >>>>>> descriptions? >>>>> >>>>> Just like all clocks, what would happen? It will be enabled. >>>> >>>> Oh, wow! Cool stuff. But seriously is it not a problem to have two entities >>>> controlling one and the same clock? Is this use-case taken into account by >>>> the clock framework? >>> >>> Yes, it is handled correctly. That's a basic use-case, handled by CCF >>> since some years (~12?). Anyway, whatever OS is doing (or not doing) >>> with the clocks is independent of the bindings here. The question is >> >> Agree. Probably the bindings would not be the place to document this if it >> would be an issue. >> >>> about hardware - does this node, which represents PCI interface of the >>> chip, has/uses the clocks. >> >> The schematics I found for the wifi module and the khadas edge platform show >> these are indeed wired to the chip. > > I have a Rockchip RK3588 Evaluation Board on my desk, which uses the > same WLAN AP6275P module. I think I already commented on a prior > version of this series: The LPO clock is needed to make the PCIe > device visible on the bus. That means this series only works if the > clock has already been running. Otherwise the PCIe driver will never > be probed. To become visible the devices requires: > > 1. The LPO clock to be enabled > 2. Power to be applied > 3. The WL_EN gpio to be configured correctly > > If one of the above is not met, the device will not even appear in > 'lspci'. I believe the binding needs to take into consideration, that > pwrseq is needed for the PCIe side. Fortuantely the heavy lifting of > creating the proper infrastructure for this has already been done by > Bartosz Golaszewski for Qualcomm WLAN chips. What is missing is a > pwrseq driver for the Broadcom chip (or this specific module?). That does not really make sense. There is no relation between the LPO clock and the PCIe clocks so 1) being a requirement for probing the device looks odd. It also does not match past experience when I assisted Andy Green in getting this module up and running almost two years ago. """ On 11/9/22 18:26, Arend Van Spriel wrote: > On November 8, 2022 11:48:22 AM Andy Green <andy@warmcat.com> wrote: >> Hi - >> >> I'm trying to bring up AP6275 support on 6.1-rc4... I have tried a forward-ported sdk broadcom driver from the 5.10 based soc sdk, and the mainline brcm fullmac driver. > > Do you have a reference to the SDK? For what SoC? Hi Arend - It's the OOT broadcom driver that came with the latest (Sept 2022) vendor SDK for RK3588, from Rockchip. Their evb has an AP6275 onboard. PCIe generally is working on this (eg, for NVMe in the PCIe 4-lane slot) and for network, and the PCIe part seems OK when I hack in a gpio regulator to hold up the module enable gpio. """ So regarding 2) and 3) I agree with you. Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-31 12:57 ` Arend van Spriel @ 2024-07-31 13:54 ` Sebastian Reichel 2024-07-31 15:12 ` Arend Van Spriel 0 siblings, 1 reply; 25+ messages in thread From: Sebastian Reichel @ 2024-07-31 13:54 UTC (permalink / raw) To: Arend van Spriel Cc: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij, efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Andy Green [-- Attachment #1: Type: text/plain, Size: 6071 bytes --] Hi, On Wed, Jul 31, 2024 at 02:57:37PM GMT, Arend van Spriel wrote: > On 7/30/2024 7:38 PM, Sebastian Reichel wrote: > > Hi, > > > > On Tue, Jul 30, 2024 at 01:16:57PM GMT, Arend Van Spriel wrote: > > > On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > On 30/07/2024 11:52, Arend Van Spriel wrote: > > > > > On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > > > > > On 30/07/2024 08:37, Arend Van Spriel wrote: > > > > > > > + Linus W > > > > > > > > > > > > > > On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: > > > > > > > > > > > > > > > Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow > > > > > > > > external low power clock input. In DTS the clock as an optional choice in > > > > > > > > the absence of an internal clock. > > > > > > > > > > > > > > > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > > > > > > > > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > > > > > > > > --- > > > > > > > > .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ > > > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > > > > > diff --git > > > > > > > > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > index 2c2093c77ec9a..a3607d55ef367 100644 > > > > > > > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > @@ -122,6 +122,14 @@ properties: > > > > > > > > NVRAM. This would normally be filled in by the bootloader from platform > > > > > > > > configuration data. > > > > > > > > > > > > > > > > + clocks: > > > > > > > > + items: > > > > > > > > + - description: External Low Power Clock input (32.768KHz) > > > > > > > > + > > > > > > > > + clock-names: > > > > > > > > + items: > > > > > > > > + - const: lpo > > > > > > > > + > > > > > > > > > > > > > > We still have an issue that this clock input is also present in the > > > > > > > bindings specification broadcom-bluetooth.yaml (not in bluetooth > > > > > > > subfolder). This clock is actually a chip resource. What happens if both > > > > > > > are defined and both wifi and bt drivers try to enable this clock? Can this > > > > > > > be expressed in yaml or can we only put a textual warning in the property > > > > > > > descriptions? > > > > > > > > > > > > Just like all clocks, what would happen? It will be enabled. > > > > > > > > > > Oh, wow! Cool stuff. But seriously is it not a problem to have two entities > > > > > controlling one and the same clock? Is this use-case taken into account by > > > > > the clock framework? > > > > > > > > Yes, it is handled correctly. That's a basic use-case, handled by CCF > > > > since some years (~12?). Anyway, whatever OS is doing (or not doing) > > > > with the clocks is independent of the bindings here. The question is > > > > > > Agree. Probably the bindings would not be the place to document this if it > > > would be an issue. > > > > > > > about hardware - does this node, which represents PCI interface of the > > > > chip, has/uses the clocks. > > > > > > The schematics I found for the wifi module and the khadas edge platform show > > > these are indeed wired to the chip. > > > > I have a Rockchip RK3588 Evaluation Board on my desk, which uses the > > same WLAN AP6275P module. I think I already commented on a prior > > version of this series: The LPO clock is needed to make the PCIe > > device visible on the bus. That means this series only works if the > > clock has already been running. Otherwise the PCIe driver will never > > be probed. To become visible the devices requires: > > > > 1. The LPO clock to be enabled > > 2. Power to be applied > > 3. The WL_EN gpio to be configured correctly > > > > If one of the above is not met, the device will not even appear in > > 'lspci'. I believe the binding needs to take into consideration, that > > pwrseq is needed for the PCIe side. Fortuantely the heavy lifting of > > creating the proper infrastructure for this has already been done by > > Bartosz Golaszewski for Qualcomm WLAN chips. What is missing is a > > pwrseq driver for the Broadcom chip (or this specific module?). > > That does not really make sense. There is no relation between the LPO clock > and the PCIe clocks so 1) being a requirement for probing the device looks > odd. It also does not match past experience when I assisted Andy Green in > getting this module up and running almost two years ago. Well, first of all I can easily reproduce this on my RK3588 EVB1. I intentionally ignore any bluetooth bits to avoid cross-effects from bluetooth enabling any clocks / regulators / GPIOs and make sure the RTC output clock is disabled at boot time (i.e. boot once without any reference to the RTC clock and without 'clk_ignore_unused' kernel argument). When booting up like this the WLAN device is not visible in 'lspci' despite the WL_REG_ON GPIO being hogged. If I additionally hack the RTC output clock to be enabled the WLAN device becomes visible in 'lspci'. The datasheet fully explains this: https://www.lcsc.com/datasheet/lcsc_datasheet_2203281730_AMPAK-Tech-AP6275P_C2984107.pdf PDF Page 23/24 (20/21 in the footer) has the Host Interface Timing Diagram. WL_REG_ON should only be enabled after 2 cycles from LPO. That means with LPO being disabled WL_REG_ON cannot be enabled. I'm pretty sure WL_REG_ON means WLAN_REGULATOR_ON, so the logic is not powered. On page 27 (24 in the footer) there is also a PCIe Power-On Timing diagram, which shows that WL_REG_ON must be enabled before the PCIe refclk is enabled. So there is a specific power up sequence, which must be followed. Greetings, -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-31 13:54 ` Sebastian Reichel @ 2024-07-31 15:12 ` Arend Van Spriel 2024-07-31 17:50 ` Sebastian Reichel 0 siblings, 1 reply; 25+ messages in thread From: Arend Van Spriel @ 2024-07-31 15:12 UTC (permalink / raw) To: Sebastian Reichel Cc: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij, efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Andy Green On July 31, 2024 3:54:52 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > Hi, > > On Wed, Jul 31, 2024 at 02:57:37PM GMT, Arend van Spriel wrote: >> On 7/30/2024 7:38 PM, Sebastian Reichel wrote: >>> Hi, >>> >>> On Tue, Jul 30, 2024 at 01:16:57PM GMT, Arend Van Spriel wrote: >>>> On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>> >>>>> On 30/07/2024 11:52, Arend Van Spriel wrote: >>>>>> On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>>>> >>>>>>> On 30/07/2024 08:37, Arend Van Spriel wrote: >>>>>>> > + Linus W >>>>>>> > >>>>>>> > On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>>>>>> > >>>>>>> > > Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>>>>> > > external low power clock input. In DTS the clock as an optional choice in >>>>>>> > > the absence of an internal clock. >>>>>>> > > >>>>>>> > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>>>> > > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>>>>> > > --- >>>>>>> > > .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>>>>> > > 1 file changed, 8 insertions(+) >>>>>>> > > >>>>>>> > > diff --git >>>>>>> > > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> > > b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> > > index 2c2093c77ec9a..a3607d55ef367 100644 >>>>>>> > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>> > > @@ -122,6 +122,14 @@ properties: >>>>>>> > > NVRAM. This would normally be filled in by the bootloader from platform >>>>>>> > > configuration data. >>>>>>> > > >>>>>>> > > + clocks: >>>>>>> > > + items: >>>>>>> > > + - description: External Low Power Clock input (32.768KHz) >>>>>>> > > + >>>>>>> > > + clock-names: >>>>>>> > > + items: >>>>>>> > > + - const: lpo >>>>>>> > > + >>>>>>> > >>>>>>> > We still have an issue that this clock input is also present in the >>>>>>> > bindings specification broadcom-bluetooth.yaml (not in bluetooth >>>>>>> > subfolder). This clock is actually a chip resource. What happens if both >>>>>>> > are defined and both wifi and bt drivers try to enable this clock? Can this >>>>>>> > be expressed in yaml or can we only put a textual warning in the property >>>>>>> > descriptions? >>>>>>> >>>>>>> Just like all clocks, what would happen? It will be enabled. >>>>>> >>>>>> Oh, wow! Cool stuff. But seriously is it not a problem to have two entities >>>>>> controlling one and the same clock? Is this use-case taken into account by >>>>>> the clock framework? >>>>> >>>>> Yes, it is handled correctly. That's a basic use-case, handled by CCF >>>>> since some years (~12?). Anyway, whatever OS is doing (or not doing) >>>>> with the clocks is independent of the bindings here. The question is >>>> >>>> Agree. Probably the bindings would not be the place to document this if it >>>> would be an issue. >>>> >>>>> about hardware - does this node, which represents PCI interface of the >>>>> chip, has/uses the clocks. >>>> >>>> The schematics I found for the wifi module and the khadas edge platform show >>>> these are indeed wired to the chip. >>> >>> I have a Rockchip RK3588 Evaluation Board on my desk, which uses the >>> same WLAN AP6275P module. I think I already commented on a prior >>> version of this series: The LPO clock is needed to make the PCIe >>> device visible on the bus. That means this series only works if the >>> clock has already been running. Otherwise the PCIe driver will never >>> be probed. To become visible the devices requires: >>> >>> 1. The LPO clock to be enabled >>> 2. Power to be applied >>> 3. The WL_EN gpio to be configured correctly >>> >>> If one of the above is not met, the device will not even appear in >>> 'lspci'. I believe the binding needs to take into consideration, that >>> pwrseq is needed for the PCIe side. Fortuantely the heavy lifting of >>> creating the proper infrastructure for this has already been done by >>> Bartosz Golaszewski for Qualcomm WLAN chips. What is missing is a >>> pwrseq driver for the Broadcom chip (or this specific module?). >> >> That does not really make sense. There is no relation between the LPO clock >> and the PCIe clocks so 1) being a requirement for probing the device looks >> odd. It also does not match past experience when I assisted Andy Green in >> getting this module up and running almost two years ago. > > Well, first of all I can easily reproduce this on my RK3588 EVB1. I > intentionally ignore any bluetooth bits to avoid cross-effects from > bluetooth enabling any clocks / regulators / GPIOs and make sure the > RTC output clock is disabled at boot time (i.e. boot once without > any reference to the RTC clock and without 'clk_ignore_unused' > kernel argument). When booting up like this the WLAN device is not > visible in 'lspci' despite the WL_REG_ON GPIO being hogged. If I > additionally hack the RTC output clock to be enabled the WLAN device > becomes visible in 'lspci'. > > The datasheet fully explains this: > > https://www.lcsc.com/datasheet/lcsc_datasheet_2203281730_AMPAK-Tech-AP6275P_C2984107.pdf > > PDF Page 23/24 (20/21 in the footer) has the Host Interface Timing > Diagram. WL_REG_ON should only be enabled after 2 cycles from LPO. > That means with LPO being disabled WL_REG_ON cannot be enabled. I'm > pretty sure WL_REG_ON means WLAN_REGULATOR_ON, so the logic is not > powered. On page 27 (24 in the footer) there is also a PCIe Power-On > Timing diagram, which shows that WL_REG_ON must be enabled before > the PCIe refclk is enabled. > > So there is a specific power up sequence, which must be followed. The chip also has an (less accurate) internal LPO so the 32khz sleep clock in the diagram does not have to be an external clock. Maybe Ampak bootstrapped the chip to disable the internal clock. Dunno. What Andy needed back then to get firmware running was a change in the nvram file to force using the internal LPO, but the device was already visible on the PCIe bus. Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-31 15:12 ` Arend Van Spriel @ 2024-07-31 17:50 ` Sebastian Reichel 2024-07-31 18:27 ` Arend van Spriel 0 siblings, 1 reply; 25+ messages in thread From: Sebastian Reichel @ 2024-07-31 17:50 UTC (permalink / raw) To: Arend Van Spriel Cc: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij, efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Andy Green [-- Attachment #1: Type: text/plain, Size: 7249 bytes --] Hi, On Wed, Jul 31, 2024 at 05:12:43PM GMT, Arend Van Spriel wrote: > On July 31, 2024 3:54:52 PM Sebastian Reichel > <sebastian.reichel@collabora.com> wrote: > > On Wed, Jul 31, 2024 at 02:57:37PM GMT, Arend van Spriel wrote: > > > On 7/30/2024 7:38 PM, Sebastian Reichel wrote: > > > > On Tue, Jul 30, 2024 at 01:16:57PM GMT, Arend Van Spriel wrote: > > > > > On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > > > > > On 30/07/2024 11:52, Arend Van Spriel wrote: > > > > > > > On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > > > > > > > > > On 30/07/2024 08:37, Arend Van Spriel wrote: > > > > > > > > > + Linus W > > > > > > > > > > > > > > > > > > On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: > > > > > > > > > > > > > > > > > > > Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow > > > > > > > > > > external low power clock input. In DTS the clock as an optional choice in > > > > > > > > > > the absence of an internal clock. > > > > > > > > > > > > > > > > > > > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > > > > > > > > > > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > > > > > > > > > > --- > > > > > > > > > > .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ > > > > > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git > > > > > > > > > > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > > > b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > > > index 2c2093c77ec9a..a3607d55ef367 100644 > > > > > > > > > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml > > > > > > > > > > @@ -122,6 +122,14 @@ properties: > > > > > > > > > > NVRAM. This would normally be filled in by the bootloader from platform > > > > > > > > > > configuration data. > > > > > > > > > > > > > > > > > > > > + clocks: > > > > > > > > > > + items: > > > > > > > > > > + - description: External Low Power Clock input (32.768KHz) > > > > > > > > > > + > > > > > > > > > > + clock-names: > > > > > > > > > > + items: > > > > > > > > > > + - const: lpo > > > > > > > > > > + > > > > > > > > > > > > > > > > > > We still have an issue that this clock input is also present in the > > > > > > > > > bindings specification broadcom-bluetooth.yaml (not in bluetooth > > > > > > > > > subfolder). This clock is actually a chip resource. What happens if both > > > > > > > > > are defined and both wifi and bt drivers try to enable this clock? Can this > > > > > > > > > be expressed in yaml or can we only put a textual warning in the property > > > > > > > > > descriptions? > > > > > > > > > > > > > > > > Just like all clocks, what would happen? It will be enabled. > > > > > > > > > > > > > > Oh, wow! Cool stuff. But seriously is it not a problem to have two entities > > > > > > > controlling one and the same clock? Is this use-case taken into account by > > > > > > > the clock framework? > > > > > > > > > > > > Yes, it is handled correctly. That's a basic use-case, handled by CCF > > > > > > since some years (~12?). Anyway, whatever OS is doing (or not doing) > > > > > > with the clocks is independent of the bindings here. The question is > > > > > > > > > > Agree. Probably the bindings would not be the place to document this if it > > > > > would be an issue. > > > > > > > > > > > about hardware - does this node, which represents PCI interface of the > > > > > > chip, has/uses the clocks. > > > > > > > > > > The schematics I found for the wifi module and the khadas edge platform show > > > > > these are indeed wired to the chip. > > > > > > > > I have a Rockchip RK3588 Evaluation Board on my desk, which uses the > > > > same WLAN AP6275P module. I think I already commented on a prior > > > > version of this series: The LPO clock is needed to make the PCIe > > > > device visible on the bus. That means this series only works if the > > > > clock has already been running. Otherwise the PCIe driver will never > > > > be probed. To become visible the devices requires: > > > > > > > > 1. The LPO clock to be enabled > > > > 2. Power to be applied > > > > 3. The WL_EN gpio to be configured correctly > > > > > > > > If one of the above is not met, the device will not even appear in > > > > 'lspci'. I believe the binding needs to take into consideration, that > > > > pwrseq is needed for the PCIe side. Fortuantely the heavy lifting of > > > > creating the proper infrastructure for this has already been done by > > > > Bartosz Golaszewski for Qualcomm WLAN chips. What is missing is a > > > > pwrseq driver for the Broadcom chip (or this specific module?). > > > > > > That does not really make sense. There is no relation between the LPO clock > > > and the PCIe clocks so 1) being a requirement for probing the device looks > > > odd. It also does not match past experience when I assisted Andy Green in > > > getting this module up and running almost two years ago. > > > > Well, first of all I can easily reproduce this on my RK3588 EVB1. I > > intentionally ignore any bluetooth bits to avoid cross-effects from > > bluetooth enabling any clocks / regulators / GPIOs and make sure the > > RTC output clock is disabled at boot time (i.e. boot once without > > any reference to the RTC clock and without 'clk_ignore_unused' > > kernel argument). When booting up like this the WLAN device is not > > visible in 'lspci' despite the WL_REG_ON GPIO being hogged. If I > > additionally hack the RTC output clock to be enabled the WLAN device > > becomes visible in 'lspci'. > > > > The datasheet fully explains this: > > > > https://www.lcsc.com/datasheet/lcsc_datasheet_2203281730_AMPAK-Tech-AP6275P_C2984107.pdf > > > > PDF Page 23/24 (20/21 in the footer) has the Host Interface Timing > > Diagram. WL_REG_ON should only be enabled after 2 cycles from LPO. > > That means with LPO being disabled WL_REG_ON cannot be enabled. I'm > > pretty sure WL_REG_ON means WLAN_REGULATOR_ON, so the logic is not > > powered. On page 27 (24 in the footer) there is also a PCIe Power-On > > Timing diagram, which shows that WL_REG_ON must be enabled before > > the PCIe refclk is enabled. > > > > So there is a specific power up sequence, which must be followed. > > The chip also has an (less accurate) internal LPO so the 32khz sleep clock > in the diagram does not have to be an external clock. Maybe Ampak > bootstrapped the chip to disable the internal clock. Dunno. > > What Andy needed back then to get firmware running was a change in the nvram > file to force using the internal LPO, but the device was already visible on > the PCIe bus. mh, I just tested again and I can no longer reproduce the LPO requirement for PCIe detection. Maybe it was something else all along (I did most of my tests quite some time ago). Sorry for the noise. -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P 2024-07-31 17:50 ` Sebastian Reichel @ 2024-07-31 18:27 ` Arend van Spriel 0 siblings, 0 replies; 25+ messages in thread From: Arend van Spriel @ 2024-07-31 18:27 UTC (permalink / raw) To: Sebastian Reichel Cc: Krzysztof Kozlowski, Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt, Linus Walleij, efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Andy Green On 7/31/2024 7:50 PM, Sebastian Reichel wrote: > Hi, > > On Wed, Jul 31, 2024 at 05:12:43PM GMT, Arend Van Spriel wrote: >> On July 31, 2024 3:54:52 PM Sebastian Reichel >> <sebastian.reichel@collabora.com> wrote: >>> On Wed, Jul 31, 2024 at 02:57:37PM GMT, Arend van Spriel wrote: >>>> On 7/30/2024 7:38 PM, Sebastian Reichel wrote: >>>>> On Tue, Jul 30, 2024 at 01:16:57PM GMT, Arend Van Spriel wrote: >>>>>> On July 30, 2024 12:18:20 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>>>> >>>>>>> On 30/07/2024 11:52, Arend Van Spriel wrote: >>>>>>>> On July 30, 2024 11:01:43 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>>>>>> >>>>>>>>> On 30/07/2024 08:37, Arend Van Spriel wrote: >>>>>>>>>> + Linus W >>>>>>>>>> >>>>>>>>>> On July 30, 2024 5:31:15 AM Jacobe Zang <jacobe.zang@wesion.com> wrote: >>>>>>>>>> >>>>>>>>>>> Not only AP6275P Wi-Fi device but also all Broadcom wireless devices allow >>>>>>>>>>> external low power clock input. In DTS the clock as an optional choice in >>>>>>>>>>> the absence of an internal clock. >>>>>>>>>>> >>>>>>>>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>>>>>>>> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >>>>>>>>>>> --- >>>>>>>>>>> .../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 8 ++++++++ >>>>>>>>>>> 1 file changed, 8 insertions(+) >>>>>>>>>>> >>>>>>>>>>> diff --git >>>>>>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>>>>>> index 2c2093c77ec9a..a3607d55ef367 100644 >>>>>>>>>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>>>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml >>>>>>>>>>> @@ -122,6 +122,14 @@ properties: >>>>>>>>>>> NVRAM. This would normally be filled in by the bootloader from platform >>>>>>>>>>> configuration data. >>>>>>>>>>> >>>>>>>>>>> + clocks: >>>>>>>>>>> + items: >>>>>>>>>>> + - description: External Low Power Clock input (32.768KHz) >>>>>>>>>>> + >>>>>>>>>>> + clock-names: >>>>>>>>>>> + items: >>>>>>>>>>> + - const: lpo >>>>>>>>>>> + >>>>>>>>>> >>>>>>>>>> We still have an issue that this clock input is also present in the >>>>>>>>>> bindings specification broadcom-bluetooth.yaml (not in bluetooth >>>>>>>>>> subfolder). This clock is actually a chip resource. What happens if both >>>>>>>>>> are defined and both wifi and bt drivers try to enable this clock? Can this >>>>>>>>>> be expressed in yaml or can we only put a textual warning in the property >>>>>>>>>> descriptions? >>>>>>>>> >>>>>>>>> Just like all clocks, what would happen? It will be enabled. >>>>>>>> >>>>>>>> Oh, wow! Cool stuff. But seriously is it not a problem to have two entities >>>>>>>> controlling one and the same clock? Is this use-case taken into account by >>>>>>>> the clock framework? >>>>>>> >>>>>>> Yes, it is handled correctly. That's a basic use-case, handled by CCF >>>>>>> since some years (~12?). Anyway, whatever OS is doing (or not doing) >>>>>>> with the clocks is independent of the bindings here. The question is >>>>>> >>>>>> Agree. Probably the bindings would not be the place to document this if it >>>>>> would be an issue. >>>>>> >>>>>>> about hardware - does this node, which represents PCI interface of the >>>>>>> chip, has/uses the clocks. >>>>>> >>>>>> The schematics I found for the wifi module and the khadas edge platform show >>>>>> these are indeed wired to the chip. >>>>> >>>>> I have a Rockchip RK3588 Evaluation Board on my desk, which uses the >>>>> same WLAN AP6275P module. I think I already commented on a prior >>>>> version of this series: The LPO clock is needed to make the PCIe >>>>> device visible on the bus. That means this series only works if the >>>>> clock has already been running. Otherwise the PCIe driver will never >>>>> be probed. To become visible the devices requires: >>>>> >>>>> 1. The LPO clock to be enabled >>>>> 2. Power to be applied >>>>> 3. The WL_EN gpio to be configured correctly >>>>> >>>>> If one of the above is not met, the device will not even appear in >>>>> 'lspci'. I believe the binding needs to take into consideration, that >>>>> pwrseq is needed for the PCIe side. Fortuantely the heavy lifting of >>>>> creating the proper infrastructure for this has already been done by >>>>> Bartosz Golaszewski for Qualcomm WLAN chips. What is missing is a >>>>> pwrseq driver for the Broadcom chip (or this specific module?). >>>> >>>> That does not really make sense. There is no relation between the LPO clock >>>> and the PCIe clocks so 1) being a requirement for probing the device looks >>>> odd. It also does not match past experience when I assisted Andy Green in >>>> getting this module up and running almost two years ago. >>> >>> Well, first of all I can easily reproduce this on my RK3588 EVB1. I >>> intentionally ignore any bluetooth bits to avoid cross-effects from >>> bluetooth enabling any clocks / regulators / GPIOs and make sure the >>> RTC output clock is disabled at boot time (i.e. boot once without >>> any reference to the RTC clock and without 'clk_ignore_unused' >>> kernel argument). When booting up like this the WLAN device is not >>> visible in 'lspci' despite the WL_REG_ON GPIO being hogged. If I >>> additionally hack the RTC output clock to be enabled the WLAN device >>> becomes visible in 'lspci'. >>> >>> The datasheet fully explains this: >>> >>> https://www.lcsc.com/datasheet/lcsc_datasheet_2203281730_AMPAK-Tech-AP6275P_C2984107.pdf >>> >>> PDF Page 23/24 (20/21 in the footer) has the Host Interface Timing >>> Diagram. WL_REG_ON should only be enabled after 2 cycles from LPO. >>> That means with LPO being disabled WL_REG_ON cannot be enabled. I'm >>> pretty sure WL_REG_ON means WLAN_REGULATOR_ON, so the logic is not >>> powered. On page 27 (24 in the footer) there is also a PCIe Power-On >>> Timing diagram, which shows that WL_REG_ON must be enabled before >>> the PCIe refclk is enabled. >>> >>> So there is a specific power up sequence, which must be followed. >> >> The chip also has an (less accurate) internal LPO so the 32khz sleep clock >> in the diagram does not have to be an external clock. Maybe Ampak >> bootstrapped the chip to disable the internal clock. Dunno. >> >> What Andy needed back then to get firmware running was a change in the nvram >> file to force using the internal LPO, but the device was already visible on >> the PCIe bus. > > mh, I just tested again and I can no longer reproduce the LPO > requirement for PCIe detection. Maybe it was something else all > along (I did most of my tests quite some time ago). > Sorry for the noise. Hi Sebastian, Thanks for letting it know. Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 3/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2 2024-07-30 3:30 [PATCH v5 0/5] Add AP6275P wireless support Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P Jacobe Zang @ 2024-07-30 3:30 ` Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 5/5] wifi: brcmfmac: add flag for random seed during firmware download Jacobe Zang 4 siblings, 0 replies; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 3:30 UTC (permalink / raw) To: robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Jacobe Zang Khadas Edge2 uses the PCI-e Ampak AP6275P 2T2R Wi-Fi 6 module. The pcie@0 node can be used as Bridge1, so the wifi@0 node is used as a device under the Bridge1. Co-developed-by: Muhammed Efe Cetin <efectn@protonmail.com> Signed-off-by: Muhammed Efe Cetin <efectn@protonmail.com> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> --- .../boot/dts/rockchip/rk3588s-khadas-edge2.dts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts index dbddfc3bb4641..b80a552dad883 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588s-khadas-edge2.dts @@ -283,6 +283,22 @@ &pcie2x1l2 { reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>; vpcie3v3-supply = <&vcc3v3_pcie_wl>; status = "okay"; + + pcie@0,0 { + reg = <0x400000 0 0 0 0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + device_type = "pci"; + bus-range = <0x40 0x4f>; + + wifi: wifi@0,0 { + compatible = "pci14e4,449d"; + reg = <0x410000 0 0 0 0>; + clocks = <&hym8563>; + clock-names = "lpo"; + }; + }; }; &pwm11 { -- 2.34.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support 2024-07-30 3:30 [PATCH v5 0/5] Add AP6275P wireless support Jacobe Zang ` (2 preceding siblings ...) 2024-07-30 3:30 ` [PATCH v5 3/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2 Jacobe Zang @ 2024-07-30 3:30 ` Jacobe Zang 2024-07-30 5:10 ` Stefan Wahren 2024-07-30 18:46 ` Arend van Spriel 2024-07-30 3:30 ` [PATCH v5 5/5] wifi: brcmfmac: add flag for random seed during firmware download Jacobe Zang 4 siblings, 2 replies; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 3:30 UTC (permalink / raw) To: robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Jacobe Zang WiFi modules often require 32kHz clock to function. Add support to enable the clock to PCIe driver. Co-developed-by: Ondrej Jirman <megi@xff.cz> Signed-off-by: Ondrej Jirman <megi@xff.cz> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c index e406e11481a62..6246e3fd7399f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c @@ -6,6 +6,7 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_net.h> +#include <linux/clk.h> #include <defs.h> #include "debug.h" @@ -70,6 +71,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, { struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio; struct device_node *root, *np = dev->of_node; + struct clk *clk; const char *prop; int irq; int err; @@ -113,6 +115,12 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, of_node_put(root); } + clk = devm_clk_get_optional_enabled(dev, "lpo"); + if (!IS_ERR_OR_NULL(clk)) { + brcmf_dbg(INFO, "enabling 32kHz clock\n"); + clk_set_rate(clk, 32768); + } + if (!np || !of_device_is_compatible(np, "brcm,bcm4329-fmac")) return; -- 2.34.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support 2024-07-30 3:30 ` [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support Jacobe Zang @ 2024-07-30 5:10 ` Stefan Wahren 2024-07-30 18:46 ` Arend van Spriel 1 sibling, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2024-07-30 5:10 UTC (permalink / raw) To: Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick Hi Jacobe, Am 30.07.24 um 05:30 schrieb Jacobe Zang: > WiFi modules often require 32kHz clock to function. Add support to > enable the clock to PCIe driver. > > Co-developed-by: Ondrej Jirman <megi@xff.cz> > Signed-off-by: Ondrej Jirman <megi@xff.cz> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c > index e406e11481a62..6246e3fd7399f 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c > @@ -6,6 +6,7 @@ > #include <linux/of.h> > #include <linux/of_irq.h> > #include <linux/of_net.h> > +#include <linux/clk.h> > > #include <defs.h> > #include "debug.h" > @@ -70,6 +71,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, > { > struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio; > struct device_node *root, *np = dev->of_node; > + struct clk *clk; > const char *prop; > int irq; > int err; > @@ -113,6 +115,12 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, > of_node_put(root); > } > > + clk = devm_clk_get_optional_enabled(dev, "lpo"); > + if (!IS_ERR_OR_NULL(clk)) { > + brcmf_dbg(INFO, "enabling 32kHz clock\n"); > + clk_set_rate(clk, 32768); > + } even if the clock is optional, there should be a proper error handling (e.g. the -EPROBE_DEFER case). Best regards > + > if (!np || !of_device_is_compatible(np, "brcm,bcm4329-fmac")) > return; > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support 2024-07-30 3:30 ` [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support Jacobe Zang 2024-07-30 5:10 ` Stefan Wahren @ 2024-07-30 18:46 ` Arend van Spriel 2024-07-30 23:57 ` Jacobe Zang 1 sibling, 1 reply; 25+ messages in thread From: Arend van Spriel @ 2024-07-30 18:46 UTC (permalink / raw) To: Jacobe Zang, robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick On 7/30/2024 5:30 AM, Jacobe Zang wrote: > WiFi modules often require 32kHz clock to function. Add support to > enable the clock to PCIe driver. > > Co-developed-by: Ondrej Jirman <megi@xff.cz> > Signed-off-by: Ondrej Jirman <megi@xff.cz> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c > index e406e11481a62..6246e3fd7399f 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c > @@ -6,6 +6,7 @@ > #include <linux/of.h> > #include <linux/of_irq.h> > #include <linux/of_net.h> > +#include <linux/clk.h> > > #include <defs.h> > #include "debug.h" > @@ -70,6 +71,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, > { > struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio; > struct device_node *root, *np = dev->of_node; > + struct clk *clk; > const char *prop; > int irq; > int err; > @@ -113,6 +115,12 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, > of_node_put(root); > } > > + clk = devm_clk_get_optional_enabled(dev, "lpo"); > + if (!IS_ERR_OR_NULL(clk)) { > + brcmf_dbg(INFO, "enabling 32kHz clock\n"); > + clk_set_rate(clk, 32768); > + } > + > if (!np || !of_device_is_compatible(np, "brcm,bcm4329-fmac")) > return; This if-statement should be the first check in brcmf_of_probe(). Can you include that change in your patch. Regards, Arend ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support 2024-07-30 18:46 ` Arend van Spriel @ 2024-07-30 23:57 ` Jacobe Zang 0 siblings, 0 replies; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 23:57 UTC (permalink / raw) To: Arend van Spriel, robh@kernel.org, krzk+dt@kernel.org, heiko@sntech.de, kvalo@kernel.org, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, conor+dt@kernel.org Cc: efectn@protonmail.com, dsimic@manjaro.org, jagan@edgeble.ai, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, arend@broadcom.com, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, megi@xff.cz, duoming@zju.edu.cn, bhelgaas@google.com, minipli@grsecurity.net, brcm80211@lists.linux.dev, brcm80211-dev-list.pdl@broadcom.com, Nick Xie >> WiFi modules often require 32kHz clock to function. Add support to >> enable the clock to PCIe driver. >> >> Co-developed-by: Ondrej Jirman <megi@xff.cz> >> Signed-off-by: Ondrej Jirman <megi@xff.cz> >> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> >> --- >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c >> index e406e11481a62..6246e3fd7399f 100644 >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c >> @@ -6,6 +6,7 @@ >> #include <linux/of.h> >> #include <linux/of_irq.h> >> #include <linux/of_net.h> >> +#include <linux/clk.h> >> >> #include <defs.h> >> #include "debug.h" >> @@ -70,6 +71,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, >> { >> struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio; >> struct device_node *root, *np = dev->of_node; >> + struct clk *clk; >> const char *prop; >> int irq; >> int err; >> @@ -113,6 +115,12 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type, >> of_node_put(root); >> } >> >> + clk = devm_clk_get_optional_enabled(dev, "lpo"); >> + if (!IS_ERR_OR_NULL(clk)) { >> + brcmf_dbg(INFO, "enabling 32kHz clock\n"); >> + clk_set_rate(clk, 32768); >> + } >> + >> if (!np || !of_device_is_compatible(np, "brcm,bcm4329-fmac")) >> return; > > This if-statement should be the first check in brcmf_of_probe(). Can you > include that change in your patch. Sure. I will change it ;-) --- Best Regards Jacobe ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 5/5] wifi: brcmfmac: add flag for random seed during firmware download 2024-07-30 3:30 [PATCH v5 0/5] Add AP6275P wireless support Jacobe Zang ` (3 preceding siblings ...) 2024-07-30 3:30 ` [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support Jacobe Zang @ 2024-07-30 3:30 ` Jacobe Zang 4 siblings, 0 replies; 25+ messages in thread From: Jacobe Zang @ 2024-07-30 3:30 UTC (permalink / raw) To: robh, krzk+dt, heiko, kvalo, davem, edumazet, kuba, pabeni, conor+dt Cc: efectn, dsimic, jagan, devicetree, linux-arm-kernel, linux-rockchip, linux-kernel, arend, linux-wireless, netdev, megi, duoming, bhelgaas, minipli, brcm80211, brcm80211-dev-list.pdl, nick, Jacobe Zang, Arend van Spriel Providing the random seed to firmware was tied to the fact that the device has a valid OTP, which worked for some Apple chips. However, it turns out the BCM43752 device also needs the random seed in order to get firmware running. Suspect it is simply tied to the firmware branch used for the device. Introducing a mechanism to allow setting it for a device through the device table. Co-developed-by: Ondrej Jirman <megi@xff.cz> Signed-off-by: Ondrej Jirman <megi@xff.cz> Co-developed-by: Arend van Spriel <arend.vanspriel@broadcom.com> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com> --- .../broadcom/brcm80211/brcmfmac/pcie.c | 52 ++++++++++++++++--- .../broadcom/brcm80211/include/brcm_hw_ids.h | 2 + 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 06698a714b523..938632daf30a9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -66,6 +66,7 @@ BRCMF_FW_DEF(4365C, "brcmfmac4365c-pcie"); BRCMF_FW_DEF(4366B, "brcmfmac4366b-pcie"); BRCMF_FW_DEF(4366C, "brcmfmac4366c-pcie"); BRCMF_FW_DEF(4371, "brcmfmac4371-pcie"); +BRCMF_FW_CLM_DEF(43752, "brcmfmac43752-pcie"); BRCMF_FW_CLM_DEF(4377B3, "brcmfmac4377b3-pcie"); BRCMF_FW_CLM_DEF(4378B1, "brcmfmac4378b1-pcie"); BRCMF_FW_CLM_DEF(4378B3, "brcmfmac4378b3-pcie"); @@ -104,6 +105,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = { BRCMF_FW_ENTRY(BRCM_CC_43664_CHIP_ID, 0xFFFFFFF0, 4366C), BRCMF_FW_ENTRY(BRCM_CC_43666_CHIP_ID, 0xFFFFFFF0, 4366C), BRCMF_FW_ENTRY(BRCM_CC_4371_CHIP_ID, 0xFFFFFFFF, 4371), + BRCMF_FW_ENTRY(BRCM_CC_43752_CHIP_ID, 0xFFFFFFFF, 43752), BRCMF_FW_ENTRY(BRCM_CC_4377_CHIP_ID, 0xFFFFFFFF, 4377B3), /* revision ID 4 */ BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0x0000000F, 4378B1), /* revision ID 3 */ BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0xFFFFFFE0, 4378B3), /* revision ID 5 */ @@ -358,6 +360,7 @@ struct brcmf_pciedev_info { u16 value); struct brcmf_mp_device *settings; struct brcmf_otp_params otp; + bool fwseed; #ifdef DEBUG u32 console_interval; bool console_active; @@ -1720,14 +1723,14 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo, memcpy_toio(devinfo->tcm + address, nvram, nvram_len); brcmf_fw_nvram_free(nvram); - if (devinfo->otp.valid) { + if (devinfo->fwseed) { size_t rand_len = BRCMF_RANDOM_SEED_LENGTH; struct brcmf_random_seed_footer footer = { .length = cpu_to_le32(rand_len), .magic = cpu_to_le32(BRCMF_RANDOM_SEED_MAGIC), }; - /* Some Apple chips/firmwares expect a buffer of random + /* Some chips/firmwares expect a buffer of random * data to be present before NVRAM */ brcmf_dbg(PCIE, "Download random seed\n"); @@ -2399,6 +2402,37 @@ static void brcmf_pcie_debugfs_create(struct device *dev) } #endif +struct brcmf_pcie_drvdata { + enum brcmf_fwvendor vendor; + bool fw_seed; +}; + +enum { + BRCMF_DRVDATA_CYW, + BRCMF_DRVDATA_BCA, + BRCMF_DRVDATA_WCC, + BRCMF_DRVDATA_WCC_SEED, +}; + +static const struct brcmf_pcie_drvdata drvdata[] = { + [BRCMF_DRVDATA_CYW] = { + .vendor = BRCMF_FWVENDOR_CYW, + .fw_seed = false, + }, + [BRCMF_DRVDATA_BCA] = { + .vendor = BRCMF_FWVENDOR_BCA, + .fw_seed = false, + }, + [BRCMF_DRVDATA_WCC] = { + .vendor = BRCMF_FWVENDOR_WCC, + .fw_seed = false, + }, + [BRCMF_DRVDATA_WCC_SEED] = { + .vendor = BRCMF_FWVENDOR_WCC, + .fw_seed = true, + }, +}; + /* Forward declaration for pci_match_id() call */ static const struct pci_device_id brcmf_pcie_devid_table[]; @@ -2477,9 +2511,10 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) bus->bus_priv.pcie = pcie_bus_dev; bus->ops = &brcmf_pcie_bus_ops; bus->proto_type = BRCMF_PROTO_MSGBUF; - bus->fwvid = id->driver_data; bus->chip = devinfo->coreid; bus->wowl_supported = pci_pme_capable(pdev, PCI_D3hot); + bus->fwvid = drvdata[id->driver_data].vendor; + devinfo->fwseed = drvdata[id->driver_data].fw_seed; dev_set_drvdata(&pdev->dev, bus); ret = brcmf_alloc(&devinfo->pdev->dev, devinfo->settings); @@ -2665,14 +2700,14 @@ static const struct dev_pm_ops brcmf_pciedrvr_pm = { BRCM_PCIE_VENDOR_ID_BROADCOM, (dev_id), \ PCI_ANY_ID, PCI_ANY_ID, \ PCI_CLASS_NETWORK_OTHER << 8, 0xffff00, \ - BRCMF_FWVENDOR_ ## fw_vend \ + BRCMF_DRVDATA_ ## fw_vend \ } #define BRCMF_PCIE_DEVICE_SUB(dev_id, subvend, subdev, fw_vend) \ { \ BRCM_PCIE_VENDOR_ID_BROADCOM, (dev_id), \ (subvend), (subdev), \ PCI_CLASS_NETWORK_OTHER << 8, 0xffff00, \ - BRCMF_FWVENDOR_ ## fw_vend \ + BRCMF_DRVDATA_ ## fw_vend \ } static const struct pci_device_id brcmf_pcie_devid_table[] = { @@ -2700,9 +2735,10 @@ static const struct pci_device_id brcmf_pcie_devid_table[] = { BRCMF_PCIE_DEVICE(BRCM_PCIE_4366_5G_DEVICE_ID, BCA), BRCMF_PCIE_DEVICE(BRCM_PCIE_4371_DEVICE_ID, WCC), BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_DEVICE_ID, CYW), - BRCMF_PCIE_DEVICE(BRCM_PCIE_4377_DEVICE_ID, WCC), - BRCMF_PCIE_DEVICE(BRCM_PCIE_4378_DEVICE_ID, WCC), - BRCMF_PCIE_DEVICE(BRCM_PCIE_4387_DEVICE_ID, WCC), + BRCMF_PCIE_DEVICE(BRCM_PCIE_4377_DEVICE_ID, WCC_SEED), + BRCMF_PCIE_DEVICE(BRCM_PCIE_4378_DEVICE_ID, WCC_SEED), + BRCMF_PCIE_DEVICE(BRCM_PCIE_4387_DEVICE_ID, WCC_SEED), + BRCMF_PCIE_DEVICE(BRCM_PCIE_43752_DEVICE_ID, WCC_SEED), { /* end: all zeroes */ } }; diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h index 44684bf1b9acc..c1e22c589d85e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h @@ -52,6 +52,7 @@ #define BRCM_CC_43664_CHIP_ID 43664 #define BRCM_CC_43666_CHIP_ID 43666 #define BRCM_CC_4371_CHIP_ID 0x4371 +#define BRCM_CC_43752_CHIP_ID 43752 #define BRCM_CC_4377_CHIP_ID 0x4377 #define BRCM_CC_4378_CHIP_ID 0x4378 #define BRCM_CC_4387_CHIP_ID 0x4387 @@ -94,6 +95,7 @@ #define BRCM_PCIE_4366_5G_DEVICE_ID 0x43c5 #define BRCM_PCIE_4371_DEVICE_ID 0x440d #define BRCM_PCIE_43596_DEVICE_ID 0x4415 +#define BRCM_PCIE_43752_DEVICE_ID 0x449d #define BRCM_PCIE_4377_DEVICE_ID 0x4488 #define BRCM_PCIE_4378_DEVICE_ID 0x4425 #define BRCM_PCIE_4387_DEVICE_ID 0x4433 -- 2.34.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2024-07-31 18:27 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-30 3:30 [PATCH v5 0/5] Add AP6275P wireless support Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 1/5] dt-bindings: net: wireless: brcm4329-fmac: add pci14e4,449d Jacobe Zang 2024-07-30 6:03 ` Krzysztof Kozlowski 2024-07-30 3:30 ` [PATCH v5 2/5] dt-bindings: net: wireless: brcm4329-fmac: add clock description for AP6275P Jacobe Zang 2024-07-30 6:03 ` Krzysztof Kozlowski 2024-07-30 6:37 ` Arend Van Spriel 2024-07-30 9:01 ` Krzysztof Kozlowski 2024-07-30 9:52 ` Arend Van Spriel 2024-07-30 10:00 ` Jacobe Zang 2024-07-30 10:08 ` Arend Van Spriel 2024-07-30 10:17 ` Jacobe Zang 2024-07-30 10:18 ` Krzysztof Kozlowski 2024-07-30 11:16 ` Arend Van Spriel 2024-07-30 17:38 ` Sebastian Reichel 2024-07-31 12:57 ` Arend van Spriel 2024-07-31 13:54 ` Sebastian Reichel 2024-07-31 15:12 ` Arend Van Spriel 2024-07-31 17:50 ` Sebastian Reichel 2024-07-31 18:27 ` Arend van Spriel 2024-07-30 3:30 ` [PATCH v5 3/5] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2 Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 4/5] wifi: brcmfmac: Add optional lpo clock enable support Jacobe Zang 2024-07-30 5:10 ` Stefan Wahren 2024-07-30 18:46 ` Arend van Spriel 2024-07-30 23:57 ` Jacobe Zang 2024-07-30 3:30 ` [PATCH v5 5/5] wifi: brcmfmac: add flag for random seed during firmware download Jacobe Zang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox