On 8/19/2024 6:42 PM, Sebastian Reichel wrote:
Hi,
I tested this on RK3588 EVB1 and the driver is working fine. The DT
bindings are not correct, though:
linux/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dtb: wifi@0,0:
compatible: 'oneOf' conditional failed, one must be fixed:
['pci14e4,449d', 'brcm,bcm4329-fmac'] is too long
'pci14e4,449d' is not one of ['brcm,bcm43143-fmac', 'brcm,bcm4341b0-fmac',
'brcm,bcm4341b4-fmac', 'brcm,bcm4341b5-fmac', 'brcm,bcm4329-fmac',
'brcm,bcm4330-fmac', 'brcm,bcm4334-fmac', 'brcm,bcm43340-fmac',
'brcm,bcm4335-fmac', 'brcm,bcm43362-fmac', 'brcm,bcm4339-fmac',
'brcm,bcm43430a0-fmac', 'brcm,bcm43430a1-fmac', 'brcm,bcm43455-fmac',
'brcm,bcm43456-fmac', 'brcm,bcm4354-fmac', 'brcm,bcm4356-fmac',
'brcm,bcm4359-fmac', 'brcm,bcm4366-fmac', 'cypress,cyw4373-fmac',
'cypress,cyw43012-fmac', 'infineon,cyw43439-fmac']
from schema $id: http://devicetree.org/schemas/net/wireless/brcm,bcm4329-fmac.yaml#
It's easy to see the problem in the binding. It does not expect a
fallback string after the PCI ID based compatible. Either the
pci14e4,449d entry must be added to the first enum in the binding,
which has the fallback compatible, or the fallback compatible
should not be added to DTS.
Never understood why we ended up with such a large list. When the
binding was introduced there was one compatible, ie. brcm,bcm4329-fmac.
People wanted all the other flavors because it described a specific wifi
chip and no other reason whatsoever. The PCI ID based compatible do
obfuscate that info so those are even less useful in my opinion.
If the fallback compatible is missing in DTS, the compatible check in
brcmf_of_probe() fails and the lpo clock is not requested resulting
in the firmware startup failing. So that would require further
driver changes.
Right. The text based bindings file in 5.12 kernel clearly says:
Required properties:
- compatible : Should be "brcm,bcm4329-fmac".
In 5.13 kernel this was replaced by the json-schema yaml file. The PCI
ID based enum which was added later does also list brcm,bcm4329-fmac so
why does that not work for the compatible list ['pci14e4,449d',
'brcm,bcm4329-fmac']? Looking at the compatible property in yaml which I
stripped a bit for brevity:
properties:
compatible:
oneOf:
- items:
- enum:
- brcm,bcm43143-fmac
- brcm,bcm4329-fmac
- infineon,cyw43439-fmac
- const: brcm,bcm4329-fmac
- enum:
- brcm,bcm4329-fmac
- pci14e4,43dc # BCM4355
- pci14e4,4464 # BCM4364
- pci14e4,4488 # BCM4377
- pci14e4,4425 # BCM4378
- pci14e4,4433 # BCM4387
So how should I read this. Searching for some sort of syntax description
I found [1] which has an example schema with description that has a
similarly complicated compatible property. From that I think the above
should be changed to:
properties:
compatible:
oneOf:
- items:
- enum:
- brcm,bcm43143-fmac
- - brcm,bcm4329-fmac
- infineon,cyw43439-fmac
- const: brcm,bcm4329-fmac
+ - items:
- enum:
- - brcm,bcm4329-fmac
- pci14e4,43dc # BCM4355
- pci14e4,4464 # BCM4364
- pci14e4,4488 # BCM4377
- pci14e4,4425 # BCM4378
- pci14e4,4433 # BCM4387
+ - const: brcm,bcm4329-fmac
+ - const: brcm,bcm4329-fmac
This poses a constraint in which the last string in the compatible list
is always 'brcm,bcm4329-fmac' even if it is the only string. At least
that is my understanding so if my understanding is wrong feel free to
correct me on this.
[1] https://docs.kernel.org/devicetree/bindings/writing-schema.html
Greetings,
-- Sebastian
