devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Firmware for Bluetooth (and wifi)
       [not found]                                     ` <CAGb2v6594vsKCd9E8apojQXE8h6WJj4-doJ8wa85Nt3EFZhXHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-01-26 21:39                                       ` Arend van Spriel
       [not found]                                         ` <52E580A8.4060600-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Arend van Spriel @ 2014-01-26 21:39 UTC (permalink / raw)
  To: Chen-Yu Tsai, linux-sunxi, devicetree-u79uwXL29TY76Z2rM5mHXA

On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
> Hi,
> 
> On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Hi,
>>
>> On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:
>>
>> <snip>
>>
>>
>>>> Quick update, I've just tested:
>>>> https://github.com/wens/linux/commits/wip/sunxi-next-wifi
>>>
>>>
>>> About this, I would like to move WiFi power control to a regulator,
>>> and controlled by sunxi-mci via vmmc-supply (not supported ATM)
>>
>>
>> Actually the sunxi-mci.c driver already has support for an optional
>> regulator called vmmc.
>>
>> I like this idea, I've done a version of the dt patch using a regulator
>> instead of rfkill here:
>> https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07
>>
>> This works as advertised and IMHO is the better solution.
> 
> I have a version in another branch I haven't pushed. I had it using an
> always-on regulator. I can adjust it to use vmmc.
> 
> BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
> in mmc core.
> We should re-use code if possible, wouldn't you agree?
> 
>>>> About the oob interrupt stuff not working, AFAIK you should set a pinctrl
>>>> function (not input, but a function like mmc is a function) on the pin in
>>>> question
>>>> for it to work as external interrupt, I believe you're not doing so in
>>>> your
>>>> dts.
>>>
>>>
>>> The pinctrl driver seems to set the function when the interrupt is
>>> enabled.
>>> Unfortunately we don't have any documentation/examples on how to use them.
>>> I will look into that later.
>>
>>
>> Hmm, but you also have a pinctrl entry in the dts setting it up as
>> gpio-input,
>> maybe that conflicts ?
> 
> I made a version with pinctrl entry setup as "irq", got an interrupt,
> but then the whole thing hung. Looks like pinctrl irqchip was not
> properly handling chained interrupts. I have done a simple fix, and I
> hope to test it tomorrow. Then I'll do some more testing with different
> configurations and hopefully write some documents.

Hi Chen-Yu,

I had a look at github tree from Hans with your DT support patch for
brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
the bindings are still experimental, but I would prefer you to use brcm
instead of broadcom in the 'compatible' entry as found in
Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
but I guess that train has left and it is tricky to change it now.
In the brcmfmac patch you also use multiple of_device ids. Could we make
it more generic, ie. compatible = "brcm,brcmfmac" or "brcm,wifi-fullmac"
or whatever...

Regards,
Arend

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Firmware for Bluetooth (and wifi)
       [not found]                                         ` <52E580A8.4060600-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
@ 2014-01-27  0:20                                           ` Tomasz Figa
       [not found]                                             ` <52E5A631.4030204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Tomasz Figa @ 2014-01-27  0:20 UTC (permalink / raw)
  To: Arend van Spriel, Chen-Yu Tsai, linux-sunxi,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Chen-Yu, Arend,

On 26.01.2014 22:39, Arend van Spriel wrote:
> On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> Hi,
>>>
>>> On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:
>>>
>>> <snip>
>>>
>>>
>>>>> Quick update, I've just tested:
>>>>> https://github.com/wens/linux/commits/wip/sunxi-next-wifi
>>>>
>>>>
>>>> About this, I would like to move WiFi power control to a regulator,
>>>> and controlled by sunxi-mci via vmmc-supply (not supported ATM)
>>>
>>>
>>> Actually the sunxi-mci.c driver already has support for an optional
>>> regulator called vmmc.
>>>
>>> I like this idea, I've done a version of the dt patch using a regulator
>>> instead of rfkill here:
>>> https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07
>>>
>>> This works as advertised and IMHO is the better solution.
>>
>> I have a version in another branch I haven't pushed. I had it using an
>> always-on regulator. I can adjust it to use vmmc.
>>
>> BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
>> in mmc core.
>> We should re-use code if possible, wouldn't you agree?
>>
>>>>> About the oob interrupt stuff not working, AFAIK you should set a pinctrl
>>>>> function (not input, but a function like mmc is a function) on the pin in
>>>>> question
>>>>> for it to work as external interrupt, I believe you're not doing so in
>>>>> your
>>>>> dts.
>>>>
>>>>
>>>> The pinctrl driver seems to set the function when the interrupt is
>>>> enabled.
>>>> Unfortunately we don't have any documentation/examples on how to use them.
>>>> I will look into that later.
>>>
>>>
>>> Hmm, but you also have a pinctrl entry in the dts setting it up as
>>> gpio-input,
>>> maybe that conflicts ?
>>
>> I made a version with pinctrl entry setup as "irq", got an interrupt,
>> but then the whole thing hung. Looks like pinctrl irqchip was not
>> properly handling chained interrupts. I have done a simple fix, and I
>> hope to test it tomorrow. Then I'll do some more testing with different
>> configurations and hopefully write some documents.
>
> Hi Chen-Yu,
>
> I had a look at github tree from Hans with your DT support patch for
> brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
> the bindings are still experimental, but I would prefer you to use brcm
> instead of broadcom in the 'compatible' entry as found in
> Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
> exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
> but I guess that train has left and it is tricky to change it now.
> In the brcmfmac patch you also use multiple of_device ids. Could we make
> it more generic, ie. compatible = "brcm,brcmfmac" or "brcm,wifi-fullmac"
> or whatever...

brcmfmac and wifi-fullmac strings sound to generic for me. What happens 
if an incompatible brcmfmac chip shows up? I'd rather use something like 
"bcm43xx" to cover existing chip family, if all the bcm43xx chips are 
compatible, or something more specific otherwise.

By the way, you might find this thread interesting:

http://thread.gmane.org/gmane.linux.kernel.mmc/24728/focus=24783

In addition to the general idea of handling power control, I have also 
posted a link to my patch adding DT support to brcmfmac driver.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Firmware for Bluetooth (and wifi)
       [not found]                                             ` <52E5A631.4030204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-01-27  4:09                                               ` Chen-Yu Tsai
  2014-01-27  9:21                                               ` Arend van Spriel
  1 sibling, 0 replies; 7+ messages in thread
From: Chen-Yu Tsai @ 2014-01-27  4:09 UTC (permalink / raw)
  To: Tomasz Figa; +Cc: Arend van Spriel, linux-sunxi, devicetree

Hi Arend, Tomasz,

On Mon, Jan 27, 2014 at 8:20 AM, Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Chen-Yu, Arend,
>
>
> On 26.01.2014 22:39, Arend van Spriel wrote:
>>
>> On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
>>>
>>> Hi,
>>>
>>> On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:
>>>>
>>>> <snip>
>>>>
>>>>
>>>>>> Quick update, I've just tested:
>>>>>> https://github.com/wens/linux/commits/wip/sunxi-next-wifi
>>>>>
>>>>>
>>>>>
>>>>> About this, I would like to move WiFi power control to a regulator,
>>>>> and controlled by sunxi-mci via vmmc-supply (not supported ATM)
>>>>
>>>>
>>>>
>>>> Actually the sunxi-mci.c driver already has support for an optional
>>>> regulator called vmmc.
>>>>
>>>> I like this idea, I've done a version of the dt patch using a regulator
>>>> instead of rfkill here:
>>>>
>>>> https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07
>>>>
>>>> This works as advertised and IMHO is the better solution.
>>>
>>>
>>> I have a version in another branch I haven't pushed. I had it using an
>>> always-on regulator. I can adjust it to use vmmc.
>>>
>>> BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
>>> in mmc core.
>>> We should re-use code if possible, wouldn't you agree?
>>>
>>>>>> About the oob interrupt stuff not working, AFAIK you should set a
>>>>>> pinctrl
>>>>>> function (not input, but a function like mmc is a function) on the pin
>>>>>> in
>>>>>> question
>>>>>> for it to work as external interrupt, I believe you're not doing so in
>>>>>> your
>>>>>> dts.
>>>>>
>>>>>
>>>>>
>>>>> The pinctrl driver seems to set the function when the interrupt is
>>>>> enabled.
>>>>> Unfortunately we don't have any documentation/examples on how to use
>>>>> them.
>>>>> I will look into that later.
>>>>
>>>>
>>>>
>>>> Hmm, but you also have a pinctrl entry in the dts setting it up as
>>>> gpio-input,
>>>> maybe that conflicts ?
>>>
>>>
>>> I made a version with pinctrl entry setup as "irq", got an interrupt,
>>> but then the whole thing hung. Looks like pinctrl irqchip was not
>>> properly handling chained interrupts. I have done a simple fix, and I
>>> hope to test it tomorrow. Then I'll do some more testing with different
>>> configurations and hopefully write some documents.
>>
>>
>> Hi Chen-Yu,
>>
>> I had a look at github tree from Hans with your DT support patch for
>> brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
>> the bindings are still experimental, but I would prefer you to use brcm
>> instead of broadcom in the 'compatible' entry as found in
>> Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
>> exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
>> but I guess that train has left and it is tricky to change it now.

I will fix this, though the only reason I did this is for the
OOB interrupts.

>> In the brcmfmac patch you also use multiple of_device ids. Could we make
>> it more generic, ie. compatible = "brcm,brcmfmac" or "brcm,wifi-fullmac"
>> or whatever...
>
>
> brcmfmac and wifi-fullmac strings sound to generic for me. What happens if
> an incompatible brcmfmac chip shows up? I'd rather use something like
> "bcm43xx" to cover existing chip family, if all the bcm43xx chips are
> compatible, or something more specific otherwise.
>
> By the way, you might find this thread interesting:
>
> http://thread.gmane.org/gmane.linux.kernel.mmc/24728/focus=24783

Yeah I've been following this thread.

> In addition to the general idea of handling power control, I have also
> posted a link to my patch adding DT support to brcmfmac driver.

Actually since that thread started, I prefer handling the regulators and
possibly clocks in the mmc controller, as Olof posted. But that's just
my opinion.


Cheers,
ChenYu

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Firmware for Bluetooth (and wifi)
       [not found]                                             ` <52E5A631.4030204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2014-01-27  4:09                                               ` Chen-Yu Tsai
@ 2014-01-27  9:21                                               ` Arend van Spriel
       [not found]                                                 ` <52E62510.5090103-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
       [not found]                                                 ` <52E630F4.8090805@gmail.com >
  1 sibling, 2 replies; 7+ messages in thread
From: Arend van Spriel @ 2014-01-27  9:21 UTC (permalink / raw)
  To: Tomasz Figa, Chen-Yu Tsai, linux-sunxi,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 01/27/2014 01:20 AM, Tomasz Figa wrote:
> Hi Chen-Yu, Arend,
> 
> On 26.01.2014 22:39, Arend van Spriel wrote:
>> On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
>>> Hi,
>>>
>>> On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> wrote:
>>>> Hi,
>>>>
>>>> On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:
>>>>
>>>> <snip>
>>>>
>>>>
>>>>>> Quick update, I've just tested:
>>>>>> https://github.com/wens/linux/commits/wip/sunxi-next-wifi
>>>>>
>>>>>
>>>>> About this, I would like to move WiFi power control to a regulator,
>>>>> and controlled by sunxi-mci via vmmc-supply (not supported ATM)
>>>>
>>>>
>>>> Actually the sunxi-mci.c driver already has support for an optional
>>>> regulator called vmmc.
>>>>
>>>> I like this idea, I've done a version of the dt patch using a regulator
>>>> instead of rfkill here:
>>>> https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07
>>>>
>>>>
>>>> This works as advertised and IMHO is the better solution.
>>>
>>> I have a version in another branch I haven't pushed. I had it using an
>>> always-on regulator. I can adjust it to use vmmc.
>>>
>>> BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
>>> in mmc core.
>>> We should re-use code if possible, wouldn't you agree?
>>>
>>>>>> About the oob interrupt stuff not working, AFAIK you should set a
>>>>>> pinctrl
>>>>>> function (not input, but a function like mmc is a function) on the
>>>>>> pin in
>>>>>> question
>>>>>> for it to work as external interrupt, I believe you're not doing
>>>>>> so in
>>>>>> your
>>>>>> dts.
>>>>>
>>>>>
>>>>> The pinctrl driver seems to set the function when the interrupt is
>>>>> enabled.
>>>>> Unfortunately we don't have any documentation/examples on how to
>>>>> use them.
>>>>> I will look into that later.
>>>>
>>>>
>>>> Hmm, but you also have a pinctrl entry in the dts setting it up as
>>>> gpio-input,
>>>> maybe that conflicts ?
>>>
>>> I made a version with pinctrl entry setup as "irq", got an interrupt,
>>> but then the whole thing hung. Looks like pinctrl irqchip was not
>>> properly handling chained interrupts. I have done a simple fix, and I
>>> hope to test it tomorrow. Then I'll do some more testing with different
>>> configurations and hopefully write some documents.
>>
>> Hi Chen-Yu,
>>
>> I had a look at github tree from Hans with your DT support patch for
>> brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
>> the bindings are still experimental, but I would prefer you to use brcm
>> instead of broadcom in the 'compatible' entry as found in
>> Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
>> exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
>> but I guess that train has left and it is tricky to change it now.
>> In the brcmfmac patch you also use multiple of_device ids. Could we make
>> it more generic, ie. compatible = "brcm,brcmfmac" or "brcm,wifi-fullmac"
>> or whatever...
> 
> brcmfmac and wifi-fullmac strings sound to generic for me. What happens
> if an incompatible brcmfmac chip shows up? I'd rather use something like
> "bcm43xx" to cover existing chip family, if all the bcm43xx chips are
> compatible, or something more specific otherwise.

The brcmfmac driver that consumes these DT nodes will have a closer look
at the device obtaining the chipid during the probe and determine if it
can support it. So the compatible string indicates that the device needs
a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
wireless driver.

> By the way, you might find this thread interesting:
> 
> http://thread.gmane.org/gmane.linux.kernel.mmc/24728/focus=24783
> 
> In addition to the general idea of handling power control, I have also
> posted a link to my patch adding DT support to brcmfmac driver.

You peaked my interest and I will catch up reading the thread.

Regards,
Arend

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Firmware for Bluetooth (and wifi)
       [not found]                                                 ` <52E62510.5090103-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
@ 2014-01-27 10:12                                                   ` Tomasz Figa
  0 siblings, 0 replies; 7+ messages in thread
From: Tomasz Figa @ 2014-01-27 10:12 UTC (permalink / raw)
  To: Arend van Spriel, Chen-Yu Tsai, linux-sunxi,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 27.01.2014 10:21, Arend van Spriel wrote:
> On 01/27/2014 01:20 AM, Tomasz Figa wrote:
>> Hi Chen-Yu, Arend,
>>
>> On 26.01.2014 22:39, Arend van Spriel wrote:
>>> On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
>>>> Hi,
>>>>
>>>> On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> wrote:
>>>>> Hi,
>>>>>
>>>>> On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:
>>>>>
>>>>> <snip>
>>>>>
>>>>>
>>>>>>> Quick update, I've just tested:
>>>>>>> https://github.com/wens/linux/commits/wip/sunxi-next-wifi
>>>>>>
>>>>>>
>>>>>> About this, I would like to move WiFi power control to a regulator,
>>>>>> and controlled by sunxi-mci via vmmc-supply (not supported ATM)
>>>>>
>>>>>
>>>>> Actually the sunxi-mci.c driver already has support for an optional
>>>>> regulator called vmmc.
>>>>>
>>>>> I like this idea, I've done a version of the dt patch using a regulator
>>>>> instead of rfkill here:
>>>>> https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07
>>>>>
>>>>>
>>>>> This works as advertised and IMHO is the better solution.
>>>>
>>>> I have a version in another branch I haven't pushed. I had it using an
>>>> always-on regulator. I can adjust it to use vmmc.
>>>>
>>>> BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
>>>> in mmc core.
>>>> We should re-use code if possible, wouldn't you agree?
>>>>
>>>>>>> About the oob interrupt stuff not working, AFAIK you should set a
>>>>>>> pinctrl
>>>>>>> function (not input, but a function like mmc is a function) on the
>>>>>>> pin in
>>>>>>> question
>>>>>>> for it to work as external interrupt, I believe you're not doing
>>>>>>> so in
>>>>>>> your
>>>>>>> dts.
>>>>>>
>>>>>>
>>>>>> The pinctrl driver seems to set the function when the interrupt is
>>>>>> enabled.
>>>>>> Unfortunately we don't have any documentation/examples on how to
>>>>>> use them.
>>>>>> I will look into that later.
>>>>>
>>>>>
>>>>> Hmm, but you also have a pinctrl entry in the dts setting it up as
>>>>> gpio-input,
>>>>> maybe that conflicts ?
>>>>
>>>> I made a version with pinctrl entry setup as "irq", got an interrupt,
>>>> but then the whole thing hung. Looks like pinctrl irqchip was not
>>>> properly handling chained interrupts. I have done a simple fix, and I
>>>> hope to test it tomorrow. Then I'll do some more testing with different
>>>> configurations and hopefully write some documents.
>>>
>>> Hi Chen-Yu,
>>>
>>> I had a look at github tree from Hans with your DT support patch for
>>> brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
>>> the bindings are still experimental, but I would prefer you to use brcm
>>> instead of broadcom in the 'compatible' entry as found in
>>> Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
>>> exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
>>> but I guess that train has left and it is tricky to change it now.
>>> In the brcmfmac patch you also use multiple of_device ids. Could we make
>>> it more generic, ie. compatible = "brcm,brcmfmac" or "brcm,wifi-fullmac"
>>> or whatever...
>>
>> brcmfmac and wifi-fullmac strings sound to generic for me. What happens
>> if an incompatible brcmfmac chip shows up? I'd rather use something like
>> "bcm43xx" to cover existing chip family, if all the bcm43xx chips are
>> compatible, or something more specific otherwise.
>
> The brcmfmac driver that consumes these DT nodes will have a closer look
> at the device obtaining the chipid during the probe and determine if it
> can support it. So the compatible string indicates that the device needs
> a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
> wireless driver.

The compatible string should guarantee that the chip ID register holds a 
valid value, so just "wifi-fullmac" or "brcmfmac" sounds too generic to 
me. The string must specify the family of chips with this chip ID scheme 
in a reasonably precise way. "brcm,bcm43xx-fmac" maybe? I still see a 
risk of, say, BCM43999 showing up, which would be a completely different 
chip. while having the model matching the pattern.

However this is something for you Broadcom guys to know better, so feel 
free to suggest anything better.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Firmware for Bluetooth (and wifi)
       [not found]                                                   ` <52E630F4.8090805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2014-01-28 10:11                                                     ` Arend van Spriel
  0 siblings, 0 replies; 7+ messages in thread
From: Arend van Spriel @ 2014-01-28 10:11 UTC (permalink / raw)
  To: Tomasz Figa, Chen-Yu Tsai, linux-sunxi,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 01/27/2014 11:12 AM, Tomasz Figa wrote:
>> The brcmfmac driver that consumes these DT nodes will have a closer look
>> at the device obtaining the chipid during the probe and determine if it
>> can support it. So the compatible string indicates that the device needs
>> a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
>> wireless driver.
> 
> The compatible string should guarantee that the chip ID register holds a
> valid value, so just "wifi-fullmac" or "brcmfmac" sounds too generic to

I am not sure I understand this requirement. Is the DT node claimed
somehow after of_find_matching_node() and unavailable to other drivers.

> me. The string must specify the family of chips with this chip ID scheme
> in a reasonably precise way. "brcm,bcm43xx-fmac" maybe? I still see a
> risk of, say, BCM43999 showing up, which would be a completely different
> chip. while having the model matching the pattern.

If a completely different chip, ie. BCM43999, shows up in a board the
device tree should not use "brcm,bcm43xx-fmac". That would be an error
in the dts file, right? All the devices listed in your bindings patch
are treated the same, ie. *compatible* on DT level and hence can have
the same compatible property.

In my opinion that is what the compatible property is about. It
identifies how a specific category of devices is accessed/configured. As
an example please see [1]. It shows one compatible string for a binding
that is used for different MPIC controllers.

Just to be clear, I like your suggestion to use "brcm,bcm43xx-fmac", but
felt you did not so added my explanation/point of view.

Regards,
Arend

[1] Documentation/devicetree/bindings/powerpc/fsl/mpic.txt

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Firmware for Bluetooth (and wifi)
       [not found]                                                     ` <52E78236.50702-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
@ 2014-01-28 10:41                                                       ` Hans de Goede
  0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2014-01-28 10:41 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Tomasz Figa, Chen-Yu Tsai,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi,

On 01/28/2014 11:11 AM, Arend van Spriel wrote:
> On 01/27/2014 11:12 AM, Tomasz Figa wrote:
>>> The brcmfmac driver that consumes these DT nodes will have a closer look
>>> at the device obtaining the chipid during the probe and determine if it
>>> can support it. So the compatible string indicates that the device needs
>>> a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
>>> wireless driver.
>>
>> The compatible string should guarantee that the chip ID register holds a
>> valid value, so just "wifi-fullmac" or "brcmfmac" sounds too generic to
> 
> I am not sure I understand this requirement. Is the DT node claimed
> somehow after of_find_matching_node() and unavailable to other drivers.
> 
>> me. The string must specify the family of chips with this chip ID scheme
>> in a reasonably precise way. "brcm,bcm43xx-fmac" maybe? I still see a
>> risk of, say, BCM43999 showing up, which would be a completely different
>> chip. while having the model matching the pattern.
> 
> If a completely different chip, ie. BCM43999, shows up in a board the
> device tree should not use "brcm,bcm43xx-fmac". That would be an error
> in the dts file, right? All the devices listed in your bindings patch
> are treated the same, ie. *compatible* on DT level and hence can have
> the same compatible property.
> 
> In my opinion that is what the compatible property is about. It
> identifies how a specific category of devices is accessed/configured. As
> an example please see [1]. It shows one compatible string for a binding
> that is used for different MPIC controllers.
> 
> Just to be clear, I like your suggestion to use "brcm,bcm43xx-fmac", but
> felt you did not so added my explanation/point of view.

The usual way to solve this is to have the dts file have a list of
compatibility strings going from specific to more generic, so for ie the
wifi on the cubietruck the dts file would contain:

compatible = "brcm,bcm43362", "brcm,bcm43xx-fmac";

And then the brcmfmac driver will contain .compatible = "brcm,bcm43xx-fmac"

If we then ever need to have some specific quirks in the driver the driver
can use of_device_is_compatible(dev->of_node, "brcm,bcm43362") to check for
the 43362.

Their could even be a completely separate driver for the "brcm,bcm43362",
with brcmfmac still claiming "brcm,bcm43xx-fmac", as matching is done
from left to right, so if there is a specific driver and a more generic
one the specific driver will win (assuming both are built-in / loaded
at probe time).

TL;DR: dts file should have:
  compatible = "brcm,bcm43362", "brcm,bcm43xx-fmac";
brcmfmac should have:
  .compatible = "brcm,bcm43xx-fmac",
So that we can add device specific quirks later (if necessary).

Regards,

Hans

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-01-28 10:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <52A040CE.5040706@schinagl.nl>
     [not found] ` <CAFXj5xjJ0DTa0Qo1avLboQF-e3TFv3aEr_jjkuZjinwuTzXykQ@mail.gmail.com>
     [not found]   ` <52A09B5B.70800@schinagl.nl>
     [not found]     ` <CAGRGNgWBgWqoPUdHSm2OXGb8r-ca5NX2z4_v0eG6oOMgJRd3HQ@mail.gmail.com>
     [not found]       ` <52B17973.1000608@broadcom.com>
     [not found]         ` <52B19F38.7060503@redhat.com>
     [not found]           ` <52B1CA51.4010202@broadcom.com>
     [not found]             ` <CAGb2v67cnP0yth0xxBKcaW_T2fF+4nP4r6h9NLqy5FV2x=hO=Q@mail.gmail.com>
     [not found]               ` <CAGb2v67mmYnyPeOd4Wwp1zHs7EC_Gr+gEdaaMWeKP5z_Z_SVGA@mail.gmail.com>
     [not found]                 ` <CAGb2v66646-1Sye=Eb04_SoeCGBU064OePcxDi7th1fqU8XfBg@mail.gmail.com>
     [not found]                   ` <52BD68BA.3080304@broadcom.com>
     [not found]                     ` <CAGb2v67Q=q4vyOftb+BLhAYR-6d04dDP3+kY3b88U8_vCvAmgQ@mail.gmail.com>
     [not found]                       ` <52CD12D4.5030008@broadcom.com>
     [not found]                         ` <CAGb2v64+5Rgs6JxD0iH3bcwXPOm-xyS_LSmWLfx0qiMQwreOeQ@mail.gmail.com>
     [not found]                           ` <52E19A27.7000402@redhat.com>
     [not found]                             ` <52E19E1C.1010402@redhat.com>
     [not found]                               ` <CAGb2v66KhGoGmoFkTw9PQYBtX 9YQbmxk4oy_tM4LM_UhZMkcZg@mail.gmail.com>
     [not found]                                 ` <52E5392B.80605@redhat.com>
     [not found]                                   ` <CAGb2v6594vsKCd9E8apojQXE8h6WJj4-doJ8wa85Nt3EFZhXHA@mail.gmail.com>
     [not found]                                     ` <CAGb2v6594vsKCd9E8apojQXE8h6WJj4-doJ8wa85Nt3EFZhXHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-01-26 21:39                                       ` Firmware for Bluetooth (and wifi) Arend van Spriel
     [not found]                                         ` <52E580A8.4060600-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2014-01-27  0:20                                           ` Tomasz Figa
     [not found]                                             ` <52E5A631.4030204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-27  4:09                                               ` Chen-Yu Tsai
2014-01-27  9:21                                               ` Arend van Spriel
     [not found]                                                 ` <52E62510.5090103-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2014-01-27 10:12                                                   ` Tomasz Figa
     [not found]                                                 ` <52E630F4.8090805@gmail.com >
     [not found]                                                   ` <52E630F4.8090805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-01-28 10:11                                                     ` Arend van Spriel
     [not found]                                                   ` <52E78236.50702@broadcom.! com>
     [not found]                                                     ` <52E78236.50702-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2014-01-28 10:41                                                       ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).