Re: [PATCH v2 1/4] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

[Arend van Spriel <arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>]

On 17-06-14 08:32, Hans de Goede wrote:
> Hi,
>
> On 06/16/2014 10:53 PM, Florian Fainelli wrote:
>> 2014-06-16 10:56 GMT-07:00 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>>> From: Arend van Spriel <arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>
>>> The Broadcom bcm43xx sdio devices are fullmac devices that may be
>>> integrated in ARM platforms. Currently, the brcmfmac driver for
>>> these devices support use of platform data. This patch specifies
>>> the bindings that allow this platform data to be expressed in the
>>> devicetree.
>>>
>>> Reviewed-by: Hante Meuleman <meuleman-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Franky (Zhenhui) Lin <frankyl-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Daniel (Deognyoun) Kim <dekim-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Pieter-Paul Giesberts <pieterpg-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Arend van Spriel <arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> [hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org: drop clk / reg_on gpio handling, as there is no consensus
>>>   on how to handle this yet]
>>> [hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org: move from bindings/staging to bindings]
>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>   .../bindings/net/wireless/brcm,bcm43xx-fmac.txt    | 29 ++++++++++++++++++++++
>>>   1 file changed, 29 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>> new file mode 100644
>>> index 0000000..6a0aaf2
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>> @@ -0,0 +1,29 @@
>>> +Broadcom BCM43xx Fullmac wireless SDIO devices
>>> +
>>> +This node provides properties for controlling the Broadcom wireless device. The
>>> +node is expected to be specified as a child node to the SDIO controller that
>>> +connects the device to the system.
>>> +
>>> +Required properties:
>>> +
>>> + - compatible : Should be "brcm,bcm43xx-fmac".
>>
>> In general, the use of a wildcard compatible string is discouraged
>> over the use of a more descriptive compatible string. So you should
>> find out what is the first chip that is compatible, and use that
>> compatible string as long as that compatibility remains.
>
> Right, Arend, what should we use then ?

In earlier discussions, we ended up with this compatible string. The 
properties are generic enough to be covered by this 'wildcard' string. 
However, I am not religious about it so if you feel strongly for an 
explicit string it could be brcm,bcm4329-fmac, but it does not have my 
preference.

>>> +
>>> +Optional properties:
>>> + - brcm,drive-strength : drive strength used for SDIO pins on device.
>>> +       (default = 6mA).
>>
>> It would not hurt if you did specify the unit of the
>> "brcm,drive-strength" property here. Also, if the default is 6, maybe
>> the example below should be fixed to reflect that so people do not
>> think that writing 4 to a register = 6 mA.
>
> Ok will fix in the next version.
>

I agree that explicitly specifying the unit of the property is a good 
addition. If that is done I don't think it is necessary to change the 
value used in the example.

Regards,
Arend

>>
>>> + - interrupt-parent : the phandle for the interrupt controller to which the
>>> +       device interrupts are connected.
>>> + - interrupts : specifies attributes for the out-of-band interrupt (host-wake).
>>> +       When not specified the device will use in-band SDIO interrupts.
>>> + - interrupt-names : name of the out-of-band interrupt, which must be set
>>> +       to "host-wake".
>>> +
>>> +Example:
>>> +
>>> +bcm4335 {
>>> +       compatible = "brcm,bcm43xx-fmac";
>>> +       brcm,drive-strength = <4>;
>>> +       interrupt-parent = <&gpx2>;
>>> +       interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>>> +       interrupt-names = "host-wake";
>>> +};
>>> --
>>> 2.0.0
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>>