Re: [PATCH] brcmfmac: add support for external 32khz clock

[Arend van Spriel <arend.vanspriel@broadcom.com>]

On 11/7/2017 7:52 AM, Stefan Wahren wrote:
> Hi,
>
>> Kalle Valo <kvalo@codeaurora.org> hat am 7. November 2017 um 03:18 geschrieben:
>>
>>
>> Stefan Wahren <stefan.wahren@i2se.com> writes:
>>
>>>> Simon Shields <simon@lineageos.org> hat am 4. November 2017 um 14:24 geschrieben:
>>>>
>>>>
>>>> Some boards use an external 32khz clock for low-power
>>>> mode timing. Make sure the clock is powered on while the chipset
>>>> is active.
>>>>
>>>> Signed-off-by: Simon Shields <simon@lineageos.org>
>>>> ---
>>>>   .../devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt     |  2 ++
>>>>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h      |  2 ++
>>>>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c          |  5 +++++
>>>>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c        | 10 ++++++++++
>>>>   4 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>> index b2bd4704f859..37add5e29272 100644
>>>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>> @@ -17,6 +17,8 @@ Optional properties:
>>>>   	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".
>>>> + - clocks : external 32khz clock
>>>> + - clock-names : name of the external 32khz clock, must be "32khz"
>>>
>>> sorry for the nitpicking, but according to the datasheet [1] it's
>>> 32768 Hz. Apart from that i suggest to use a functional name for the
>>> clock like "low_power" or something else, which is more flexible and
>>> future-proof.

It is called LPO (low-power oscillator) in our documentation so my 
suggestion would be "ext-lpo". The recommended value is 32*1024 Hz, but 
it is not a must.

>>>
>>> Btw this binding needs to be a separate patch, which should go to the
>>> devicetree guys.
>>
>> Previously I have applied binding documentation changes which the DT
>> maintainers have acked, that's why I specifically asked to Cc device
>> tree list. Has something changed?
>
> as long as the changes has been acked this should be okay. I was referring to point 1 in this guideline [1].

Yeah. As happens regularly in requirements management only the 
requirement is documented and not the motivation behind it.

Regards,
Arend

> [1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/submitting-patches.txt?h=v4.14-rc8
>
>>
>> --
>> Kalle Valo