diff mbox series

[wireless-next,v2,01/12] dt-bindings: net: bcm4329-fmac: Add Apple properties & chips

Message ID E1oXg7N-0064ug-9l@rmk-PC.armlinux.org.uk (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series Add support for bcm4378 on Apple platforms | expand

Commit Message

Russell King (Oracle) Sept. 12, 2022, 9:52 a.m. UTC
From: Hector Martin <marcan@marcan.st>

This binding is currently used for SDIO devices, but these chips are
also used as PCIe devices on DT platforms and may be represented in the
DT. Re-use the existing binding and add chip compatibles used by Apple
T2 and M1 platforms (the T2 ones are not known to be used in DT
platforms, but we might as well document them).

Then, add properties required for firmware selection and calibration on
M1 machines.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
 1 file changed, 35 insertions(+), 4 deletions(-)

Comments

Alvin Šipraga Sept. 12, 2022, 11:59 a.m. UTC | #1
On Mon, Sep 12, 2022 at 10:52:41AM +0100, Russell King wrote:
> From: Hector Martin <marcan@marcan.st>
> 
> This binding is currently used for SDIO devices, but these chips are
> also used as PCIe devices on DT platforms and may be represented in the
> DT. Re-use the existing binding and add chip compatibles used by Apple
> T2 and M1 platforms (the T2 ones are not known to be used in DT
> platforms, but we might as well document them).
> 
> Then, add properties required for firmware selection and calibration on
> M1 machines.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> index 53b4153d9bfc..fec1cc9b9a08 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Broadcom BCM4329 family fullmac wireless SDIO devices
> +title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
>  
>  maintainers:
>    - Arend van Spriel <arend@broadcom.com>
> @@ -41,11 +41,17 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
>                - cypress,cyw4373-fmac
>                - cypress,cyw43012-fmac
>            - const: brcm,bcm4329-fmac
> -      - const: brcm,bcm4329-fmac
> +      - enum:
> +          - brcm,bcm4329-fmac
> +          - pci14e4,43dc  # BCM4355
> +          - pci14e4,4464  # BCM4364
> +          - pci14e4,4488  # BCM4377
> +          - pci14e4,4425  # BCM4378
> +          - pci14e4,4433  # BCM4387
>  
>    reg:
> -    description: SDIO function number for the device, for most cases
> -      this will be 1.
> +    description: SDIO function number for the device (for most cases
> +      this will be 1) or PCI device identifier.
>  
>    interrupts:
>      maxItems: 1
> @@ -85,6 +91,31 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
>        takes precedence.
>      type: boolean
>  
> +  brcm,cal-blob:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    description: A per-device calibration blob for the Wi-Fi radio. This
> +      should be filled in by the bootloader from platform configuration
> +      data, if necessary, and will be uploaded to the device if present.

Is this a leftover from a previous revision of the patchset? Because as
far as I can tell, the CLM blob is (still) being loaded via firmware,
and no additional parsing has been added for this particular OF
property. Should it be dropped?

The rest looks quite OK.

Kind regards,
Alvin

> +
> +  brcm,board-type:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: Overrides the board type, which is normally the compatible of
> +      the root node. This can be used to decouple the overall system board or
> +      device name from the board type for WiFi purposes, which is used to
> +      construct firmware and NVRAM configuration filenames, allowing for
> +      multiple devices that share the same module or characteristics for the
> +      WiFi subsystem to share the same firmware/NVRAM files. On Apple platforms,
> +      this should be the Apple module-instance codename prefixed by "apple,",
> +      e.g. "apple,honshu".

nit: s/WiFi/Wi-Fi/

> +
> +  apple,antenna-sku:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: Antenna SKU used to identify a specific antenna configuration
> +      on Apple platforms. This is use to build firmware filenames, to allow
> +      platforms with different antenna configs to have different firmware and/or
> +      NVRAM. This would normally be filled in by the bootloader from platform
> +      configuration data.
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.30.2
>
Russell King (Oracle) Sept. 12, 2022, 12:04 p.m. UTC | #2
On Mon, Sep 12, 2022 at 11:59:17AM +0000, Alvin Šipraga wrote:
> On Mon, Sep 12, 2022 at 10:52:41AM +0100, Russell King wrote:
> > From: Hector Martin <marcan@marcan.st>
> > 
> > This binding is currently used for SDIO devices, but these chips are
> > also used as PCIe devices on DT platforms and may be represented in the
> > DT. Re-use the existing binding and add chip compatibles used by Apple
> > T2 and M1 platforms (the T2 ones are not known to be used in DT
> > platforms, but we might as well document them).
> > 
> > Then, add properties required for firmware selection and calibration on
> > M1 machines.
> > 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Hector Martin <marcan@marcan.st>
> > Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
> >  1 file changed, 35 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > index 53b4153d9bfc..fec1cc9b9a08 100644
> > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > @@ -4,7 +4,7 @@
> >  $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >  
> > -title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > +title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
> >  
> >  maintainers:
> >    - Arend van Spriel <arend@broadcom.com>
> > @@ -41,11 +41,17 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> >                - cypress,cyw4373-fmac
> >                - cypress,cyw43012-fmac
> >            - const: brcm,bcm4329-fmac
> > -      - const: brcm,bcm4329-fmac
> > +      - enum:
> > +          - brcm,bcm4329-fmac
> > +          - pci14e4,43dc  # BCM4355
> > +          - pci14e4,4464  # BCM4364
> > +          - pci14e4,4488  # BCM4377
> > +          - pci14e4,4425  # BCM4378
> > +          - pci14e4,4433  # BCM4387
> >  
> >    reg:
> > -    description: SDIO function number for the device, for most cases
> > -      this will be 1.
> > +    description: SDIO function number for the device (for most cases
> > +      this will be 1) or PCI device identifier.
> >  
> >    interrupts:
> >      maxItems: 1
> > @@ -85,6 +91,31 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> >        takes precedence.
> >      type: boolean
> >  
> > +  brcm,cal-blob:
> > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > +    description: A per-device calibration blob for the Wi-Fi radio. This
> > +      should be filled in by the bootloader from platform configuration
> > +      data, if necessary, and will be uploaded to the device if present.
> 
> Is this a leftover from a previous revision of the patchset? Because as
> far as I can tell, the CLM blob is (still) being loaded via firmware,
> and no additional parsing has been added for this particular OF
> property. Should it be dropped?

It does appear to be unparsed, but I don't know whether it's needed for
the binding or not. I'll wait for the Asahi folk to review your comment
before possibly removing it.

Thanks!
Russell King (Oracle) Sept. 12, 2022, 2:01 p.m. UTC | #3
On Mon, Sep 12, 2022 at 01:04:58PM +0100, Russell King (Oracle) wrote:
> On Mon, Sep 12, 2022 at 11:59:17AM +0000, Alvin Šipraga wrote:
> > On Mon, Sep 12, 2022 at 10:52:41AM +0100, Russell King wrote:
> > > From: Hector Martin <marcan@marcan.st>
> > > 
> > > This binding is currently used for SDIO devices, but these chips are
> > > also used as PCIe devices on DT platforms and may be represented in the
> > > DT. Re-use the existing binding and add chip compatibles used by Apple
> > > T2 and M1 platforms (the T2 ones are not known to be used in DT
> > > platforms, but we might as well document them).
> > > 
> > > Then, add properties required for firmware selection and calibration on
> > > M1 machines.
> > > 
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Hector Martin <marcan@marcan.st>
> > > Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
> > >  1 file changed, 35 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > index 53b4153d9bfc..fec1cc9b9a08 100644
> > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > @@ -4,7 +4,7 @@
> > >  $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml#
> > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > >  
> > > -title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > +title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
> > >  
> > >  maintainers:
> > >    - Arend van Spriel <arend@broadcom.com>
> > > @@ -41,11 +41,17 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > >                - cypress,cyw4373-fmac
> > >                - cypress,cyw43012-fmac
> > >            - const: brcm,bcm4329-fmac
> > > -      - const: brcm,bcm4329-fmac
> > > +      - enum:
> > > +          - brcm,bcm4329-fmac
> > > +          - pci14e4,43dc  # BCM4355
> > > +          - pci14e4,4464  # BCM4364
> > > +          - pci14e4,4488  # BCM4377
> > > +          - pci14e4,4425  # BCM4378
> > > +          - pci14e4,4433  # BCM4387
> > >  
> > >    reg:
> > > -    description: SDIO function number for the device, for most cases
> > > -      this will be 1.
> > > +    description: SDIO function number for the device (for most cases
> > > +      this will be 1) or PCI device identifier.
> > >  
> > >    interrupts:
> > >      maxItems: 1
> > > @@ -85,6 +91,31 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > >        takes precedence.
> > >      type: boolean
> > >  
> > > +  brcm,cal-blob:
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +    description: A per-device calibration blob for the Wi-Fi radio. This
> > > +      should be filled in by the bootloader from platform configuration
> > > +      data, if necessary, and will be uploaded to the device if present.
> > 
> > Is this a leftover from a previous revision of the patchset? Because as
> > far as I can tell, the CLM blob is (still) being loaded via firmware,
> > and no additional parsing has been added for this particular OF
> > property. Should it be dropped?
> 
> It does appear to be unparsed, but I don't know whether it's needed for
> the binding or not. I'll wait for the Asahi folk to review your comment
> before possibly removing it.

Okay, the answer is, it is still very much part of the binding, and
the m1n1 boot loader populates it.

This series is a subset of a larger series (remember the previous 34
or 35 patch series?), so there are things in the binding document
which are not included in this series.

I don't think it makes sense to break up the binding document given
that it has already been reviewed several times in its current state,
should we really remove this one property and throw away all that
review effort.
Mark Kettenis Sept. 12, 2022, 2:13 p.m. UTC | #4
> Date: Mon, 12 Sep 2022 15:01:06 +0100
> From: "Russell King (Oracle)" <linux@armlinux.org.uk>
> 
> On Mon, Sep 12, 2022 at 01:04:58PM +0100, Russell King (Oracle) wrote:
> > On Mon, Sep 12, 2022 at 11:59:17AM +0000, Alvin Šipraga wrote:
> > > On Mon, Sep 12, 2022 at 10:52:41AM +0100, Russell King wrote:
> > > > From: Hector Martin <marcan@marcan.st>
> > > > 
> > > > This binding is currently used for SDIO devices, but these chips are
> > > > also used as PCIe devices on DT platforms and may be represented in the
> > > > DT. Re-use the existing binding and add chip compatibles used by Apple
> > > > T2 and M1 platforms (the T2 ones are not known to be used in DT
> > > > platforms, but we might as well document them).
> > > > 
> > > > Then, add properties required for firmware selection and calibration on
> > > > M1 machines.
> > > > 
> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Signed-off-by: Hector Martin <marcan@marcan.st>
> > > > Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > ---
> > > >  .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
> > > >  1 file changed, 35 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > index 53b4153d9bfc..fec1cc9b9a08 100644
> > > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > @@ -4,7 +4,7 @@
> > > >  $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml#
> > > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > >  
> > > > -title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > +title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
> > > >  
> > > >  maintainers:
> > > >    - Arend van Spriel <arend@broadcom.com>
> > > > @@ -41,11 +41,17 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > >                - cypress,cyw4373-fmac
> > > >                - cypress,cyw43012-fmac
> > > >            - const: brcm,bcm4329-fmac
> > > > -      - const: brcm,bcm4329-fmac
> > > > +      - enum:
> > > > +          - brcm,bcm4329-fmac
> > > > +          - pci14e4,43dc  # BCM4355
> > > > +          - pci14e4,4464  # BCM4364
> > > > +          - pci14e4,4488  # BCM4377
> > > > +          - pci14e4,4425  # BCM4378
> > > > +          - pci14e4,4433  # BCM4387
> > > >  
> > > >    reg:
> > > > -    description: SDIO function number for the device, for most cases
> > > > -      this will be 1.
> > > > +    description: SDIO function number for the device (for most cases
> > > > +      this will be 1) or PCI device identifier.
> > > >  
> > > >    interrupts:
> > > >      maxItems: 1
> > > > @@ -85,6 +91,31 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > >        takes precedence.
> > > >      type: boolean
> > > >  
> > > > +  brcm,cal-blob:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > +    description: A per-device calibration blob for the Wi-Fi radio. This
> > > > +      should be filled in by the bootloader from platform configuration
> > > > +      data, if necessary, and will be uploaded to the device if present.
> > > 
> > > Is this a leftover from a previous revision of the patchset? Because as
> > > far as I can tell, the CLM blob is (still) being loaded via firmware,
> > > and no additional parsing has been added for this particular OF
> > > property. Should it be dropped?
> > 
> > It does appear to be unparsed, but I don't know whether it's needed for
> > the binding or not. I'll wait for the Asahi folk to review your comment
> > before possibly removing it.
> 
> Okay, the answer is, it is still very much part of the binding, and
> the m1n1 boot loader populates it.
> 
> This series is a subset of a larger series (remember the previous 34
> or 35 patch series?), so there are things in the binding document
> which are not included in this series.
> 
> I don't think it makes sense to break up the binding document given
> that it has already been reviewed several times in its current state,
> should we really remove this one property and throw away all that
> review effort.

The OpenBSD driver already uses these properties.  So even if the
Linux driver doesn't use this yet, there is an existing implementation
that does.  That should be good enough for it to be included in the
binding isn't it?
Alvin Šipraga Sept. 12, 2022, 2:27 p.m. UTC | #5
Hi both,

On Mon, Sep 12, 2022 at 04:13:08PM +0200, Mark Kettenis wrote:
> > Date: Mon, 12 Sep 2022 15:01:06 +0100
> > From: "Russell King (Oracle)" <linux@armlinux.org.uk>
> > 
> > On Mon, Sep 12, 2022 at 01:04:58PM +0100, Russell King (Oracle) wrote:
> > > On Mon, Sep 12, 2022 at 11:59:17AM +0000, Alvin Šipraga wrote:
> > > > On Mon, Sep 12, 2022 at 10:52:41AM +0100, Russell King wrote:
> > > > > From: Hector Martin <marcan@marcan.st>
> > > > > 
> > > > > This binding is currently used for SDIO devices, but these chips are
> > > > > also used as PCIe devices on DT platforms and may be represented in the
> > > > > DT. Re-use the existing binding and add chip compatibles used by Apple
> > > > > T2 and M1 platforms (the T2 ones are not known to be used in DT
> > > > > platforms, but we might as well document them).
> > > > > 
> > > > > Then, add properties required for firmware selection and calibration on
> > > > > M1 machines.
> > > > > 
> > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Signed-off-by: Hector Martin <marcan@marcan.st>
> > > > > Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > > ---
> > > > >  .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
> > > > >  1 file changed, 35 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > index 53b4153d9bfc..fec1cc9b9a08 100644
> > > > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > @@ -4,7 +4,7 @@
> > > > >  $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml
> > > > >  $schema: http://devicetree.org/meta-schemas/core.yaml
> > > > >  
> > > > > -title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > > +title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
> > > > >  
> > > > >  maintainers:
> > > > >    - Arend van Spriel <arend@broadcom.com>
> > > > > @@ -41,11 +41,17 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > >                - cypress,cyw4373-fmac
> > > > >                - cypress,cyw43012-fmac
> > > > >            - const: brcm,bcm4329-fmac
> > > > > -      - const: brcm,bcm4329-fmac
> > > > > +      - enum:
> > > > > +          - brcm,bcm4329-fmac
> > > > > +          - pci14e4,43dc  # BCM4355
> > > > > +          - pci14e4,4464  # BCM4364
> > > > > +          - pci14e4,4488  # BCM4377
> > > > > +          - pci14e4,4425  # BCM4378
> > > > > +          - pci14e4,4433  # BCM4387
> > > > >  
> > > > >    reg:
> > > > > -    description: SDIO function number for the device, for most cases
> > > > > -      this will be 1.
> > > > > +    description: SDIO function number for the device (for most cases
> > > > > +      this will be 1) or PCI device identifier.
> > > > >  
> > > > >    interrupts:
> > > > >      maxItems: 1
> > > > > @@ -85,6 +91,31 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > >        takes precedence.
> > > > >      type: boolean
> > > > >  
> > > > > +  brcm,cal-blob:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > > +    description: A per-device calibration blob for the Wi-Fi radio. This
> > > > > +      should be filled in by the bootloader from platform configuration
> > > > > +      data, if necessary, and will be uploaded to the device if present.
> > > > 
> > > > Is this a leftover from a previous revision of the patchset? Because as
> > > > far as I can tell, the CLM blob is (still) being loaded via firmware,
> > > > and no additional parsing has been added for this particular OF
> > > > property. Should it be dropped?
> > > 
> > > It does appear to be unparsed, but I don't know whether it's needed for
> > > the binding or not. I'll wait for the Asahi folk to review your comment
> > > before possibly removing it.
> > 
> > Okay, the answer is, it is still very much part of the binding, and
> > the m1n1 boot loader populates it.
> > 
> > This series is a subset of a larger series (remember the previous 34
> > or 35 patch series?), so there are things in the binding document
> > which are not included in this series.
> > 
> > I don't think it makes sense to break up the binding document given
> > that it has already been reviewed several times in its current state,
> > should we really remove this one property and throw away all that
> > review effort.
> 
> The OpenBSD driver already uses these properties.  So even if the
> Linux driver doesn't use this yet, there is an existing implementation
> that does.  That should be good enough for it to be included in the
> binding isn't it?

Yes, I suspected that might be the case too. I think it's fine.

Thanks Russel for the clarification btw. Feel free to add my

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Kind regards,
Alvin
Russell King (Oracle) Sept. 12, 2022, 2:29 p.m. UTC | #6
On Mon, Sep 12, 2022 at 02:27:32PM +0000, Alvin Šipraga wrote:
> Hi both,
> 
> On Mon, Sep 12, 2022 at 04:13:08PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 12 Sep 2022 15:01:06 +0100
> > > From: "Russell King (Oracle)" <linux@armlinux.org.uk>
> > > 
> > > On Mon, Sep 12, 2022 at 01:04:58PM +0100, Russell King (Oracle) wrote:
> > > > On Mon, Sep 12, 2022 at 11:59:17AM +0000, Alvin Šipraga wrote:
> > > > > On Mon, Sep 12, 2022 at 10:52:41AM +0100, Russell King wrote:
> > > > > > From: Hector Martin <marcan@marcan.st>
> > > > > > 
> > > > > > This binding is currently used for SDIO devices, but these chips are
> > > > > > also used as PCIe devices on DT platforms and may be represented in the
> > > > > > DT. Re-use the existing binding and add chip compatibles used by Apple
> > > > > > T2 and M1 platforms (the T2 ones are not known to be used in DT
> > > > > > platforms, but we might as well document them).
> > > > > > 
> > > > > > Then, add properties required for firmware selection and calibration on
> > > > > > M1 machines.
> > > > > > 
> > > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > > Signed-off-by: Hector Martin <marcan@marcan.st>
> > > > > > Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > > > ---
> > > > > >  .../net/wireless/brcm,bcm4329-fmac.yaml       | 39 +++++++++++++++++--
> > > > > >  1 file changed, 35 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > > index 53b4153d9bfc..fec1cc9b9a08 100644
> > > > > > --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > > @@ -4,7 +4,7 @@
> > > > > >  $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml
> > > > > >  $schema: http://devicetree.org/meta-schemas/core.yaml
> > > > > >  
> > > > > > -title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > > > +title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
> > > > > >  
> > > > > >  maintainers:
> > > > > >    - Arend van Spriel <arend@broadcom.com>
> > > > > > @@ -41,11 +41,17 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > > >                - cypress,cyw4373-fmac
> > > > > >                - cypress,cyw43012-fmac
> > > > > >            - const: brcm,bcm4329-fmac
> > > > > > -      - const: brcm,bcm4329-fmac
> > > > > > +      - enum:
> > > > > > +          - brcm,bcm4329-fmac
> > > > > > +          - pci14e4,43dc  # BCM4355
> > > > > > +          - pci14e4,4464  # BCM4364
> > > > > > +          - pci14e4,4488  # BCM4377
> > > > > > +          - pci14e4,4425  # BCM4378
> > > > > > +          - pci14e4,4433  # BCM4387
> > > > > >  
> > > > > >    reg:
> > > > > > -    description: SDIO function number for the device, for most cases
> > > > > > -      this will be 1.
> > > > > > +    description: SDIO function number for the device (for most cases
> > > > > > +      this will be 1) or PCI device identifier.
> > > > > >  
> > > > > >    interrupts:
> > > > > >      maxItems: 1
> > > > > > @@ -85,6 +91,31 @@ title: Broadcom BCM4329 family fullmac wireless SDIO devices
> > > > > >        takes precedence.
> > > > > >      type: boolean
> > > > > >  
> > > > > > +  brcm,cal-blob:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > > > +    description: A per-device calibration blob for the Wi-Fi radio. This
> > > > > > +      should be filled in by the bootloader from platform configuration
> > > > > > +      data, if necessary, and will be uploaded to the device if present.
> > > > > 
> > > > > Is this a leftover from a previous revision of the patchset? Because as
> > > > > far as I can tell, the CLM blob is (still) being loaded via firmware,
> > > > > and no additional parsing has been added for this particular OF
> > > > > property. Should it be dropped?
> > > > 
> > > > It does appear to be unparsed, but I don't know whether it's needed for
> > > > the binding or not. I'll wait for the Asahi folk to review your comment
> > > > before possibly removing it.
> > > 
> > > Okay, the answer is, it is still very much part of the binding, and
> > > the m1n1 boot loader populates it.
> > > 
> > > This series is a subset of a larger series (remember the previous 34
> > > or 35 patch series?), so there are things in the binding document
> > > which are not included in this series.
> > > 
> > > I don't think it makes sense to break up the binding document given
> > > that it has already been reviewed several times in its current state,
> > > should we really remove this one property and throw away all that
> > > review effort.
> > 
> > The OpenBSD driver already uses these properties.  So even if the
> > Linux driver doesn't use this yet, there is an existing implementation
> > that does.  That should be good enough for it to be included in the
> > binding isn't it?
> 
> Yes, I suspected that might be the case too. I think it's fine.
> 
> Thanks Russel for the clarification btw. Feel free to add my
> 
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Thanks!
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
index 53b4153d9bfc..fec1cc9b9a08 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
@@ -4,7 +4,7 @@ 
 $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Broadcom BCM4329 family fullmac wireless SDIO devices
+title: Broadcom BCM4329 family fullmac wireless SDIO/PCIE devices
 
 maintainers:
   - Arend van Spriel <arend@broadcom.com>
@@ -41,11 +41,17 @@  title: Broadcom BCM4329 family fullmac wireless SDIO devices
               - cypress,cyw4373-fmac
               - cypress,cyw43012-fmac
           - const: brcm,bcm4329-fmac
-      - const: brcm,bcm4329-fmac
+      - enum:
+          - brcm,bcm4329-fmac
+          - pci14e4,43dc  # BCM4355
+          - pci14e4,4464  # BCM4364
+          - pci14e4,4488  # BCM4377
+          - pci14e4,4425  # BCM4378
+          - pci14e4,4433  # BCM4387
 
   reg:
-    description: SDIO function number for the device, for most cases
-      this will be 1.
+    description: SDIO function number for the device (for most cases
+      this will be 1) or PCI device identifier.
 
   interrupts:
     maxItems: 1
@@ -85,6 +91,31 @@  title: Broadcom BCM4329 family fullmac wireless SDIO devices
       takes precedence.
     type: boolean
 
+  brcm,cal-blob:
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    description: A per-device calibration blob for the Wi-Fi radio. This
+      should be filled in by the bootloader from platform configuration
+      data, if necessary, and will be uploaded to the device if present.
+
+  brcm,board-type:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: Overrides the board type, which is normally the compatible of
+      the root node. This can be used to decouple the overall system board or
+      device name from the board type for WiFi purposes, which is used to
+      construct firmware and NVRAM configuration filenames, allowing for
+      multiple devices that share the same module or characteristics for the
+      WiFi subsystem to share the same firmware/NVRAM files. On Apple platforms,
+      this should be the Apple module-instance codename prefixed by "apple,",
+      e.g. "apple,honshu".
+
+  apple,antenna-sku:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: Antenna SKU used to identify a specific antenna configuration
+      on Apple platforms. This is use to build firmware filenames, to allow
+      platforms with different antenna configs to have different firmware and/or
+      NVRAM. This would normally be filled in by the bootloader from platform
+      configuration data.
+
 required:
   - compatible
   - reg