2017-08-29 21:43:32

by Antony Antony

[permalink] [raw]
Subject: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

Add device tree binding documentation for Broadcom BCM43430
AMPAK AP6212A has this chip.

e.g
compatible = "brcm,bcm43430-fmac";

NanoPi NEO Plus2 has an AMPAK AP6212A WiFi module
with BCM43430 rev=1 inside.
brcmf_chip_recognition found AXI chip: BCM43430, rev=1
brcmf_ops_sdio_probe sdio vendor ID: 0x02d0
brcmf_ops_sdio_probe sdio device ID: 0xa9a6

Signed-off-by: Antony Antony <[email protected]>
---
Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
NanoPi Neo Plus2 Kernel DT propsed patch
https://patchwork.kernel.org/patch/9925387/

diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
index 590f622..9107e1a 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
@@ -6,7 +6,9 @@ connects the device to the system.

Required properties:

- - compatible : Should be "brcm,bcm4329-fmac".
+ - compatible : should be one of the following:
+ * "brcm,bcm4329-fmac"
+ * "brcm,bcm43430-fmac"

Optional properties:
- brcm,drive-strength : drive strength used for SDIO pins on device in mA
--
2.9.3


2017-08-30 02:28:49

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

Hi,

On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]> wrote:
> Add device tree binding documentation for Broadcom BCM43430
> AMPAK AP6212A has this chip.
>
> e.g
> compatible = "brcm,bcm43430-fmac";
>
> NanoPi NEO Plus2 has an AMPAK AP6212A WiFi module
> with BCM43430 rev=1 inside.
> brcmf_chip_recognition found AXI chip: BCM43430, rev=1
> brcmf_ops_sdio_probe sdio vendor ID: 0x02d0
> brcmf_ops_sdio_probe sdio device ID: 0xa9a6
>
> Signed-off-by: Antony Antony <[email protected]>
> ---
> Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> NanoPi Neo Plus2 Kernel DT propsed patch
> https://patchwork.kernel.org/patch/9925387/
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index 590f622..9107e1a 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -6,7 +6,9 @@ connects the device to the system.
>
> Required properties:
>
> - - compatible : Should be "brcm,bcm4329-fmac".
> + - compatible : should be one of the following:
> + * "brcm,bcm4329-fmac"
> + * "brcm,bcm43430-fmac"

You updated the bindings, but not the driver. So it's not actually
going to work. More specifically, OOB interrupts won't work.

IIRC, The compatible string for this particular case, as it was
originally proposed, only serves as a placeholder for the driver
to check against. None of the instances in sunxi device trees
match the actual chip model. Actual model matching is done
through SDIO, as you've already seen.

ChenYu


>
> Optional properties:
> - brcm,drive-strength : drive strength used for SDIO pins on device in mA
> --
> 2.9.3
>

2017-08-30 12:02:21

by Antony Antony

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

hi,

On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
> On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]> wrote:

> > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > @@ -6,7 +6,9 @@ connects the device to the system.
> >
> > Required properties:
> >
> > - - compatible : Should be "brcm,bcm4329-fmac".
> > + - compatible : should be one of the following:
> > + * "brcm,bcm4329-fmac"
> > + * "brcm,bcm43430-fmac"
>
> You updated the bindings, but not the driver. So it's not actually
> going to work. More specifically, OOB interrupts won't work.
>

understood, ignore this patch for now. Thanks Chen-Yu.

> IIRC, The compatible string for this particular case, as it was
> originally proposed, only serves as a placeholder for the driver
> to check against. None of the instances in sunxi device trees
> match the actual chip model. Actual model matching is done
> through SDIO, as you've already seen.

yes it seems SDIO driveer code is smarter, once it initialize
brcm,bcm4329-fmac it ignore the DT info and read the chip details to locate
firmware file.

I also noticed other boards using bcm4329-fmac in similar situations.
https://patchwork.kernel.org/patch/9739181/

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7

I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see where
it goes.

regards,
-antony

2017-09-04 17:37:20

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

On 04-09-17 16:42, Antony Antony wrote:
> Hi Arend,
>
> On Fri, Sep 01, 2017 at 11:30:59PM +0200, Arend van Spriel wrote:
>>>> Hi Rob,
>>>>
>>>> Actually the Broadcom wifi chips themselves are discoverable. So once the
>>>> driver has access to the register space of the device it can determine the
>>>> actual chip, its revision, and exactly what cores (and their revision) are
>>>> present in the chip. Hence there is a single compatible string as there is
>>>> no need to convey the same information through device tree data.
>>>
>>> In my expereince this how it seems to work.
>>>
>>> I jsut discovered s/brcm,bcm4329-fmac/brcm/ can load the broadcom driver.
>>>
>>> brcmf: wifi@1 {
>>> reg = <1>;
>>> compatible = "brcm";
>>> };
>>>
>>> This looks better to me. Maxime, Would this work?
>>
>> I have no idea what you are saying here. To what exactly do you apply that
>> substitute. In brcmfmac I have:
>
> Thanks for pointing me to the relevent code. I was asking about DT support I
> submitted.
>
>>
>> if (!np || bus_type != BRCMF_BUSTYPE_SDIO ||
>> !of_device_is_compatible(np, "brcm,bcm4329-fmac"))
>> return;
>>
>> In my perception using "brcm" goes against DT compatible naming convention.
>
> It seems the driver has a special place holder, bcm4329-fmac.
> I am still wondering what is acceptable in the DT.
>
> compatible = ""brcm,bcm4329-fmac";
> or
> compatible = "brcm";
>
> The last one does not generate any warnings.

Ok. So what warning do you get when using "brcm,bcm4329-fmac"? Or did
you really try ""brcm,bcm4329-fmac" as mentioned above?

Regards,
Arend

2017-09-01 16:49:14

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

On Wed, Aug 30, 2017 at 02:02:18PM +0200, Antony Antony wrote:
> hi,
>
> On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
> > On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]> wrote:
>
> > > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > > @@ -6,7 +6,9 @@ connects the device to the system.
> > >
> > > Required properties:
> > >
> > > - - compatible : Should be "brcm,bcm4329-fmac".
> > > + - compatible : should be one of the following:
> > > + * "brcm,bcm4329-fmac"
> > > + * "brcm,bcm43430-fmac"
> >
> > You updated the bindings, but not the driver. So it's not actually
> > going to work. More specifically, OOB interrupts won't work.
> >
>
> understood, ignore this patch for now. Thanks Chen-Yu.
>
> > IIRC, The compatible string for this particular case, as it was
> > originally proposed, only serves as a placeholder for the driver
> > to check against. None of the instances in sunxi device trees
> > match the actual chip model. Actual model matching is done
> > through SDIO, as you've already seen.
>
> yes it seems SDIO driveer code is smarter, once it initialize
> brcm,bcm4329-fmac it ignore the DT info and read the chip details to locate
> firmware file.
>
> I also noticed other boards using bcm4329-fmac in similar situations.
> https://patchwork.kernel.org/patch/9739181/
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7
>
> I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see where
> it goes.

Adding the compatible or instead of? The former would be better. You
should still have the actual chip in case you do have some difference to
handle.

Rob

2017-09-01 21:38:54

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

On Fri, Sep 1, 2017 at 2:10 PM, Arend van Spriel
<[email protected]> wrote:
> On 01-09-17 18:49, Rob Herring wrote:
>>
>> On Wed, Aug 30, 2017 at 02:02:18PM +0200, Antony Antony wrote:
>>>
>>> hi,
>>>
>>> On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
>>>>
>>>> On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]>
>>>> wrote:
>>>
>>>
>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> @@ -6,7 +6,9 @@ connects the device to the system.
>>>>>
>>>>> Required properties:
>>>>>
>>>>> - - compatible : Should be "brcm,bcm4329-fmac".
>>>>> + - compatible : should be one of the following:
>>>>> + * "brcm,bcm4329-fmac"
>>>>> + * "brcm,bcm43430-fmac"
>>>>
>>>>
>>>> You updated the bindings, but not the driver. So it's not actually
>>>> going to work. More specifically, OOB interrupts won't work.
>>>>
>>>
>>> understood, ignore this patch for now. Thanks Chen-Yu.
>>>
>>>> IIRC, The compatible string for this particular case, as it was
>>>> originally proposed, only serves as a placeholder for the driver
>>>> to check against. None of the instances in sunxi device trees
>>>> match the actual chip model. Actual model matching is done
>>>> through SDIO, as you've already seen.
>>>
>>>
>>> yes it seems SDIO driveer code is smarter, once it initialize
>>> brcm,bcm4329-fmac it ignore the DT info and read the chip details to
>>> locate
>>> firmware file.
>>>
>>> I also noticed other boards using bcm4329-fmac in similar situations.
>>> https://patchwork.kernel.org/patch/9739181/
>>>
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7
>>>
>>> I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see
>>> where
>>> it goes.
>>
>>
>> Adding the compatible or instead of? The former would be better. You
>> should still have the actual chip in case you do have some difference to
>> handle.
>
>
> Hi Rob,
>
> Actually the Broadcom wifi chips themselves are discoverable. So once the
> driver has access to the register space of the device it can determine the
> actual chip, its revision, and exactly what cores (and their revision) are
> present in the chip. Hence there is a single compatible string as there is
> no need to convey the same information through device tree data.

So if a chip has different power on/off sequencing you can discover that?

I realize that most often you don't need it, but a more specific
compatible is there in case you do and so it doesn't require a DTB
update to handle some difference. But you can keep using one
compatible because I can't really enforce any of that.

Rob

2017-09-01 19:10:44

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

On 01-09-17 18:49, Rob Herring wrote:
> On Wed, Aug 30, 2017 at 02:02:18PM +0200, Antony Antony wrote:
>> hi,
>>
>> On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
>>> On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]> wrote:
>>
>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>> @@ -6,7 +6,9 @@ connects the device to the system.
>>>>
>>>> Required properties:
>>>>
>>>> - - compatible : Should be "brcm,bcm4329-fmac".
>>>> + - compatible : should be one of the following:
>>>> + * "brcm,bcm4329-fmac"
>>>> + * "brcm,bcm43430-fmac"
>>>
>>> You updated the bindings, but not the driver. So it's not actually
>>> going to work. More specifically, OOB interrupts won't work.
>>>
>>
>> understood, ignore this patch for now. Thanks Chen-Yu.
>>
>>> IIRC, The compatible string for this particular case, as it was
>>> originally proposed, only serves as a placeholder for the driver
>>> to check against. None of the instances in sunxi device trees
>>> match the actual chip model. Actual model matching is done
>>> through SDIO, as you've already seen.
>>
>> yes it seems SDIO driveer code is smarter, once it initialize
>> brcm,bcm4329-fmac it ignore the DT info and read the chip details to locate
>> firmware file.
>>
>> I also noticed other boards using bcm4329-fmac in similar situations.
>> https://patchwork.kernel.org/patch/9739181/
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7
>>
>> I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see where
>> it goes.
>
> Adding the compatible or instead of? The former would be better. You
> should still have the actual chip in case you do have some difference to
> handle.

Hi Rob,

Actually the Broadcom wifi chips themselves are discoverable. So once
the driver has access to the register space of the device it can
determine the actual chip, its revision, and exactly what cores (and
their revision) are present in the chip. Hence there is a single
compatible string as there is no need to convey the same information
through device tree data.

Regards,
Arend

2017-09-04 14:42:37

by Antony Antony

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

Hi Arend,

On Fri, Sep 01, 2017 at 11:30:59PM +0200, Arend van Spriel wrote:
> > > Hi Rob,
> > >
> > > Actually the Broadcom wifi chips themselves are discoverable. So once the
> > > driver has access to the register space of the device it can determine the
> > > actual chip, its revision, and exactly what cores (and their revision) are
> > > present in the chip. Hence there is a single compatible string as there is
> > > no need to convey the same information through device tree data.
> >
> > In my expereince this how it seems to work.
> >
> > I jsut discovered s/brcm,bcm4329-fmac/brcm/ can load the broadcom driver.
> >
> > brcmf: wifi@1 {
> > reg = <1>;
> > compatible = "brcm";
> > };
> >
> > This looks better to me. Maxime, Would this work?
>
> I have no idea what you are saying here. To what exactly do you apply that
> substitute. In brcmfmac I have:

Thanks for pointing me to the relevent code. I was asking about DT support I
submitted.

>
> if (!np || bus_type != BRCMF_BUSTYPE_SDIO ||
> !of_device_is_compatible(np, "brcm,bcm4329-fmac"))
> return;
>
> In my perception using "brcm" goes against DT compatible naming convention.

It seems the driver has a special place holder, bcm4329-fmac.
I am still wondering what is acceptable in the DT.

compatible = ""brcm,bcm4329-fmac";
or
compatible = "brcm";

The last one does not generate any warnings.
Here it the DT I am proposing. https://patchwork.kernel.org/patch/9937263/

regards,
-antony

2017-09-01 22:01:46

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac



On 01-09-17 23:38, Rob Herring wrote:
> On Fri, Sep 1, 2017 at 2:10 PM, Arend van Spriel
> <[email protected]> wrote:
>> On 01-09-17 18:49, Rob Herring wrote:
>>>
>>> On Wed, Aug 30, 2017 at 02:02:18PM +0200, Antony Antony wrote:
>>>>
>>>> hi,
>>>>
>>>> On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
>>>>>
>>>>> On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]>
>>>>> wrote:
>>>>
>>>>
>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> +++
>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> @@ -6,7 +6,9 @@ connects the device to the system.
>>>>>>
>>>>>> Required properties:
>>>>>>
>>>>>> - - compatible : Should be "brcm,bcm4329-fmac".
>>>>>> + - compatible : should be one of the following:
>>>>>> + * "brcm,bcm4329-fmac"
>>>>>> + * "brcm,bcm43430-fmac"
>>>>>
>>>>>
>>>>> You updated the bindings, but not the driver. So it's not actually
>>>>> going to work. More specifically, OOB interrupts won't work.
>>>>>
>>>>
>>>> understood, ignore this patch for now. Thanks Chen-Yu.
>>>>
>>>>> IIRC, The compatible string for this particular case, as it was
>>>>> originally proposed, only serves as a placeholder for the driver
>>>>> to check against. None of the instances in sunxi device trees
>>>>> match the actual chip model. Actual model matching is done
>>>>> through SDIO, as you've already seen.
>>>>
>>>>
>>>> yes it seems SDIO driveer code is smarter, once it initialize
>>>> brcm,bcm4329-fmac it ignore the DT info and read the chip details to
>>>> locate
>>>> firmware file.
>>>>
>>>> I also noticed other boards using bcm4329-fmac in similar situations.
>>>> https://patchwork.kernel.org/patch/9739181/
>>>>
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7
>>>>
>>>> I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see
>>>> where
>>>> it goes.
>>>
>>>
>>> Adding the compatible or instead of? The former would be better. You
>>> should still have the actual chip in case you do have some difference to
>>> handle.
>>
>>
>> Hi Rob,
>>
>> Actually the Broadcom wifi chips themselves are discoverable. So once the
>> driver has access to the register space of the device it can determine the
>> actual chip, its revision, and exactly what cores (and their revision) are
>> present in the chip. Hence there is a single compatible string as there is
>> no need to convey the same information through device tree data.
>
> So if a chip has different power on/off sequencing you can discover that?
>
> I realize that most often you don't need it, but a more specific
> compatible is there in case you do and so it doesn't require a DTB
> update to handle some difference. But you can keep using one
> compatible because I can't really enforce any of that.

For SDIO chips the power sequencing is defined by power-seq-* in
bindings/mmc and handled by the MMC stack itself. So the wifi driver is
not dealing with that.

Regards,
Arend

2017-09-01 21:31:01

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

On 01-09-17 22:40, Antony Antony wrote:
> On Fri, Sep 01, 2017 at 09:10:41PM +0200, Arend van Spriel wrote:
>> On 01-09-17 18:49, Rob Herring wrote:
>>> On Wed, Aug 30, 2017 at 02:02:18PM +0200, Antony Antony wrote:
>>>> hi,
>>>>
>>>> On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
>>>>> On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]> wrote:
>>>>
>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> @@ -6,7 +6,9 @@ connects the device to the system.
>>>>>>
>>>>>> Required properties:
>>>>>>
>>>>>> - - compatible : Should be "brcm,bcm4329-fmac".
>>>>>> + - compatible : should be one of the following:
>>>>>> + * "brcm,bcm4329-fmac"
>>>>>> + * "brcm,bcm43430-fmac"
>>>>>
>>>>> You updated the bindings, but not the driver. So it's not actually
>>>>> going to work. More specifically, OOB interrupts won't work.
>>>>>
>>>>
>>>> understood, ignore this patch for now. Thanks Chen-Yu.
>>>>
>>>>> IIRC, The compatible string for this particular case, as it was
>>>>> originally proposed, only serves as a placeholder for the driver
>>>>> to check against. None of the instances in sunxi device trees
>>>>> match the actual chip model. Actual model matching is done
>>>>> through SDIO, as you've already seen.
>>>>
>>>> yes it seems SDIO driveer code is smarter, once it initialize
>>>> brcm,bcm4329-fmac it ignore the DT info and read the chip details to locate
>>>> firmware file.
>>>>
>>>> I also noticed other boards using bcm4329-fmac in similar situations.
>>>> https://patchwork.kernel.org/patch/9739181/
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7
>>>>
>>>> I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see where
>>>> it goes.
>>>
>>> Adding the compatible or instead of? The former would be better. You
>>> should still have the actual chip in case you do have some difference to
>>> handle.
>>
>> Hi Rob,
>>
>> Actually the Broadcom wifi chips themselves are discoverable. So once the
>> driver has access to the register space of the device it can determine the
>> actual chip, its revision, and exactly what cores (and their revision) are
>> present in the chip. Hence there is a single compatible string as there is
>> no need to convey the same information through device tree data.
>
> In my expereince this how it seems to work.
>
> I jsut discovered s/brcm,bcm4329-fmac/brcm/ can load the broadcom driver.
>
> brcmf: wifi@1 {
> reg = <1>;
> compatible = "brcm";
> };
>
> This looks better to me. Maxime, Would this work?

I have no idea what you are saying here. To what exactly do you apply
that substitute. In brcmfmac I have:

if (!np || bus_type != BRCMF_BUSTYPE_SDIO ||
!of_device_is_compatible(np, "brcm,bcm4329-fmac"))
return;

In my perception using "brcm" goes against DT compatible naming convention.

Regards,
Arend

2017-09-01 20:40:36

by Antony Antony

[permalink] [raw]
Subject: Re: [PATCH] Documentation: dt-binding: net: wireless: add bcm43430-fmac

On Fri, Sep 01, 2017 at 09:10:41PM +0200, Arend van Spriel wrote:
> On 01-09-17 18:49, Rob Herring wrote:
> > On Wed, Aug 30, 2017 at 02:02:18PM +0200, Antony Antony wrote:
> > > hi,
> > >
> > > On Wed, Aug 30, 2017 at 10:28:20AM +0800, Chen-Yu Tsai wrote:
> > > > On Wed, Aug 30, 2017 at 5:43 AM, Antony Antony <[email protected]> wrote:
> > >
> > > > > a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > > > > +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> > > > > @@ -6,7 +6,9 @@ connects the device to the system.
> > > > >
> > > > > Required properties:
> > > > >
> > > > > - - compatible : Should be "brcm,bcm4329-fmac".
> > > > > + - compatible : should be one of the following:
> > > > > + * "brcm,bcm4329-fmac"
> > > > > + * "brcm,bcm43430-fmac"
> > > >
> > > > You updated the bindings, but not the driver. So it's not actually
> > > > going to work. More specifically, OOB interrupts won't work.
> > > >
> > >
> > > understood, ignore this patch for now. Thanks Chen-Yu.
> > >
> > > > IIRC, The compatible string for this particular case, as it was
> > > > originally proposed, only serves as a placeholder for the driver
> > > > to check against. None of the instances in sunxi device trees
> > > > match the actual chip model. Actual model matching is done
> > > > through SDIO, as you've already seen.
> > >
> > > yes it seems SDIO driveer code is smarter, once it initialize
> > > brcm,bcm4329-fmac it ignore the DT info and read the chip details to locate
> > > firmware file.
> > >
> > > I also noticed other boards using bcm4329-fmac in similar situations.
> > > https://patchwork.kernel.org/patch/9739181/
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts?h=v4.13-rc7
> > >
> > > I will resend "NanoPi NEO Plus2" dts with "brcm,bcm4329-fmac" and see where
> > > it goes.
> >
> > Adding the compatible or instead of? The former would be better. You
> > should still have the actual chip in case you do have some difference to
> > handle.
>
> Hi Rob,
>
> Actually the Broadcom wifi chips themselves are discoverable. So once the
> driver has access to the register space of the device it can determine the
> actual chip, its revision, and exactly what cores (and their revision) are
> present in the chip. Hence there is a single compatible string as there is
> no need to convey the same information through device tree data.

In my expereince this how it seems to work.

I jsut discovered s/brcm,bcm4329-fmac/brcm/ can load the broadcom driver.

brcmf: wifi@1 {
reg = <1>;
compatible = "brcm";
};

This looks better to me. Maxime, Would this work?

regards,
-antony