diff mbox series

[01/34] dt-bindings: net: bcm4329-fmac: Add Apple properties & chips

Message ID 20211226153624.162281-2-marcan@marcan.st
State New
Headers show
Series brcmfmac: Support Apple T2 and M1 platforms | expand

Commit Message

Hector Martin Dec. 26, 2021, 3:35 p.m. UTC
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.

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../net/wireless/brcm,bcm4329-fmac.yaml       | 32 +++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)

Comments

Rob Herring (Arm) Dec. 26, 2021, 11:34 p.m. UTC | #1
On Mon, 27 Dec 2021 00:35:51 +0900, Hector Martin wrote:
> 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.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../net/wireless/brcm,bcm4329-fmac.yaml       | 32 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml: properties:apple,antenna-sku: '$def' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml: properties:apple,antenna-sku: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
		hint: A vendor boolean property can use "type: boolean"
	Additional properties are not allowed ('$def' was unexpected)
		hint: A vendor boolean property can use "type: boolean"
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml: properties:apple,antenna-sku: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
		hint: A vendor string property with exact values has an implicit type
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml: properties:apple,antenna-sku: 'oneOf' conditional failed, one must be fixed:
		'$ref' is a required property
		'allOf' is a required property
		hint: A vendor property needs a $ref to types.yaml
		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml: ignoring, error in schema: properties: apple,antenna-sku
Documentation/devicetree/bindings/mmc/mmc-controller.example.dt.yaml:0:0: /example-0/mmc@1c12000/wifi@1: failed to match any schema with compatible: ['brcm,bcm4329-fmac']
Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.example.dt.yaml:0:0: /example-0/mmc@80118000/wifi@1: failed to match any schema with compatible: ['brcm,bcm4334-fmac', 'brcm,bcm4329-fmac']
Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.example.dt.yaml:0:0: /example-0/mmc@80118000/wifi@1: failed to match any schema with compatible: ['brcm,bcm4334-fmac', 'brcm,bcm4329-fmac']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1573232

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring (Arm) Dec. 27, 2021, 4:36 p.m. UTC | #2
On Mon, Dec 27, 2021 at 12:35:51AM +0900, Hector Martin wrote:
> 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.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../net/wireless/brcm,bcm4329-fmac.yaml       | 32 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> index c11f23b20c4c..2530ff3e7b90 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>
> @@ -36,16 +36,22 @@ properties:
>                - brcm,bcm43455-fmac
>                - brcm,bcm43456-fmac
>                - brcm,bcm4354-fmac
> +              - brcm,bcm4355c1-fmac
>                - brcm,bcm4356-fmac
>                - brcm,bcm4359-fmac
> +              - brcm,bcm4364b2-fmac
> +              - brcm,bcm4364b3-fmac
> +              - brcm,bcm4377b3-fmac
> +              - brcm,bcm4378b1-fmac
> +              - brcm,bcm4387c2-fmac
>                - cypress,cyw4373-fmac
>                - cypress,cyw43012-fmac
>            - const: brcm,bcm4329-fmac
>        - const: brcm,bcm4329-fmac
>  
>    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
> @@ -75,6 +81,26 @@ properties:
>      items:
>        pattern: '^[A-Z][A-Z]-[A-Z][0-9A-Z]-[0-9]+$'
>  
> +  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.
> +
> +  apple,module-instance:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: Module codename used to identify a specific board on
> +      Apple platforms. This is used to build the firmware filenames, to allow
> +      different platforms to have different firmware and/or NVRAM config.
> +
> +  apple,antenna-sku:
> +    $def: /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.

Is there a known set of strings that can be defined?

There's also the somewhat standard 'firmware-name' property that serves 
similar purpose, but if there's multiple files, then I guess this 
approach is fine.

Rob
Mark Kettenis Dec. 29, 2021, 4:42 p.m. UTC | #3
> From: Hector Martin <marcan@marcan.st>
> Cc: Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>,
>         Alyssa Rosenzweig <alyssa@rosenzweig.io>,
>         Mark Kettenis <kettenis@openbsd.org>,
>         Rafał Miłecki <zajec5@gmail.com>,
>         Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>,
>         Linus Walleij <linus.walleij@linaro.org>,
>         Hans de Goede <hdegoede@redhat.com>,
>         "John W. Linville" <linville@tuxdriver.com>,
>         "Daniel (Deognyoun) Kim" <dekim@broadcom.com>,
>         "brian m. carlson" <sandals@crustytoothpaste.net>,
>         linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
>         devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>         linux-acpi@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com,
>         SHA-cyfmac-dev-list@infineon.com
> Date: Mon, 27 Dec 2021 00:35:51 +0900
> 
> 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.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../net/wireless/brcm,bcm4329-fmac.yaml       | 32 +++++++++++++++++--
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
> index c11f23b20c4c..2530ff3e7b90 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>
> @@ -36,16 +36,22 @@ properties:
>                - brcm,bcm43455-fmac
>                - brcm,bcm43456-fmac
>                - brcm,bcm4354-fmac
> +              - brcm,bcm4355c1-fmac
>                - brcm,bcm4356-fmac
>                - brcm,bcm4359-fmac
> +              - brcm,bcm4364b2-fmac
> +              - brcm,bcm4364b3-fmac
> +              - brcm,bcm4377b3-fmac
> +              - brcm,bcm4378b1-fmac
> +              - brcm,bcm4387c2-fmac
>                - cypress,cyw4373-fmac
>                - cypress,cyw43012-fmac
>            - const: brcm,bcm4329-fmac
>        - const: brcm,bcm4329-fmac

I suppose this helps with validation of device trees.  However, nodes
for PCI devices are not supposed to have a "compatible" property as
the PCI vendor and device IDs are supposed to be used to identify a
device.

That does raise the question how a schema for additional properties
for PCI device nodes is supposed to be defined...

>    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
> @@ -75,6 +81,26 @@ properties:
>      items:
>        pattern: '^[A-Z][A-Z]-[A-Z][0-9A-Z]-[0-9]+$'
>  
> +  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.
> +
> +  apple,module-instance:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: Module codename used to identify a specific board on
> +      Apple platforms. This is used to build the firmware filenames, to allow
> +      different platforms to have different firmware and/or NVRAM config.
> +
> +  apple,antenna-sku:
> +    $def: /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.33.0
> 
>
Hector Martin Jan. 4, 2022, 5:47 a.m. UTC | #4
On 2021/12/30 1:42, Mark Kettenis wrote:
>> From: Hector Martin <marcan@marcan.st>
>> Cc: Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>,
>>         Alyssa Rosenzweig <alyssa@rosenzweig.io>,
>>         Mark Kettenis <kettenis@openbsd.org>,
>>         Rafał Miłecki <zajec5@gmail.com>,
>>         Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>,
>>         Linus Walleij <linus.walleij@linaro.org>,
>>         Hans de Goede <hdegoede@redhat.com>,
>>         "John W. Linville" <linville@tuxdriver.com>,
>>         "Daniel (Deognyoun) Kim" <dekim@broadcom.com>,
>>         "brian m. carlson" <sandals@crustytoothpaste.net>,
>>         linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
>>         devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>>         linux-acpi@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com,
>>         SHA-cyfmac-dev-list@infineon.com
>> Date: Mon, 27 Dec 2021 00:35:51 +0900
>>
>> 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.
>>
>> Signed-off-by: Hector Martin <marcan@marcan.st>
>> ---
>>  .../net/wireless/brcm,bcm4329-fmac.yaml       | 32 +++++++++++++++++--
>>  1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
>> index c11f23b20c4c..2530ff3e7b90 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>
>> @@ -36,16 +36,22 @@ properties:
>>                - brcm,bcm43455-fmac
>>                - brcm,bcm43456-fmac
>>                - brcm,bcm4354-fmac
>> +              - brcm,bcm4355c1-fmac
>>                - brcm,bcm4356-fmac
>>                - brcm,bcm4359-fmac
>> +              - brcm,bcm4364b2-fmac
>> +              - brcm,bcm4364b3-fmac
>> +              - brcm,bcm4377b3-fmac
>> +              - brcm,bcm4378b1-fmac
>> +              - brcm,bcm4387c2-fmac
>>                - cypress,cyw4373-fmac
>>                - cypress,cyw43012-fmac
>>            - const: brcm,bcm4329-fmac
>>        - const: brcm,bcm4329-fmac
> 
> I suppose this helps with validation of device trees.  However, nodes
> for PCI devices are not supposed to have a "compatible" property as
> the PCI vendor and device IDs are supposed to be used to identify a
> device.
> 
> That does raise the question how a schema for additional properties
> for PCI device nodes is supposed to be defined...

Apparently using a "pciVVVV,DDDD" compatible is one way, see
bindings/net/wireless/qca,ath9k.yaml

There's apparently exactly one example of this in in-tree devicetrees:
boot/dts/rockchip/rk3399-gru-chromebook.dtsi

I guess this is the way to go then, unless Rob has a different idea :)
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 c11f23b20c4c..2530ff3e7b90 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>
@@ -36,16 +36,22 @@  properties:
               - brcm,bcm43455-fmac
               - brcm,bcm43456-fmac
               - brcm,bcm4354-fmac
+              - brcm,bcm4355c1-fmac
               - brcm,bcm4356-fmac
               - brcm,bcm4359-fmac
+              - brcm,bcm4364b2-fmac
+              - brcm,bcm4364b3-fmac
+              - brcm,bcm4377b3-fmac
+              - brcm,bcm4378b1-fmac
+              - brcm,bcm4387c2-fmac
               - cypress,cyw4373-fmac
               - cypress,cyw43012-fmac
           - const: brcm,bcm4329-fmac
       - const: brcm,bcm4329-fmac
 
   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
@@ -75,6 +81,26 @@  properties:
     items:
       pattern: '^[A-Z][A-Z]-[A-Z][0-9A-Z]-[0-9]+$'
 
+  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.
+
+  apple,module-instance:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: Module codename used to identify a specific board on
+      Apple platforms. This is used to build the firmware filenames, to allow
+      different platforms to have different firmware and/or NVRAM config.
+
+  apple,antenna-sku:
+    $def: /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