On 03/02/2018 11:46 PM, Sean Wang wrote:
> On Fri, 2018-03-02 at 09:40 -0600, Rob Herring wrote:
>> On Fri, Feb 23, 2018 at 06:16:33PM +0800, sean.w...@mediatek.com wrote:
>>> From: Sean Wang <sean.w...@mediatek.com>
>>>
>>> Since those LEDs are parts of PMIC MT6323, it is reasonable to merge
>>> those LEDs node definition back into mt6323.dtsi. This way can improve
>>> the reusability of those nodes among different boards with the same PMIC.
>>>
>>> Signed-off-by: Sean Wang <sean.w...@mediatek.com>
>>> Cc: Lee Jones <lee.jo...@linaro.org>
>>> ---
>>> arch/arm/boot/dts/mt6323.dtsi | 26
>>> ++++++++++++++++++++++++-
>>> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 28
>>> ---------------------------
>>> 2 files changed, 25 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/mt6323.dtsi b/arch/arm/boot/dts/mt6323.dtsi
>>> index 7c783d6..44c5642 100644
>>> --- a/arch/arm/boot/dts/mt6323.dtsi
>>> +++ b/arch/arm/boot/dts/mt6323.dtsi
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * Copyright (c) 2017 MediaTek Inc.
>>> + * Copyright (c) 2017-2018 MediaTek Inc.
>>> * Author: John Crispin <j...@phrozen.org>
>>> * Sean Wang <sean.w...@mediatek.com>
>>> * This program is free software; you can redistribute it and/or modify
>>> @@ -237,5 +237,29 @@
>>> regulator-enable-ramp-delay = <216>;
>>> };
>>> };
>>> +
>>> + leds {
>>> + compatible = "mediatek,mt6323-led";
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + led@0 {
>>> + reg = <0>;
>>> + label = "mt6323:isink:green";
>>
>> Changing the label potentially breaks userspace. And the fact that it is
>> a green LED is very much board specific. So I think the old location was
>> correct. You could put the parent node here and leave these child nodes
>> in the board specific dts file.
>>
>
> yes, I really made a mistake on breaking userspace with a label change.
>
> I will keep the parent node here and then leave these child nodes into
> board specific dts files. thanks for your idea!
>
Sorry I didn't see your email when I wrote my first comment. Yes this sounds
like the proper way to do it.
Regards,
Matthias
>>
>>> + default-state = "off";
>>> + };
>>> +
>>> + led@1 {
>>> + reg = <1>;
>>> + label = "mt6323:isink:red";
>>> + default-state = "off";
>>> + };
>>> +
>>> + led@2 {
>>> + reg = <2>;
>>> + label = "mt6323:isink:blue";
>>> + default-state = "off";
>>> + };
>>> + };
>>> };
>>> };
>>> diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
>>> b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
>>> index 75e61c4..767b225 100644
>>> --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
>>> +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
>>> @@ -483,34 +483,6 @@
>>> status = "okay";
>>> };
>>>
>>> -&pwrap {
>>> - mt6323 {
>>> - mt6323led: led {
>>> - compatible = "mediatek,mt6323-led";
>>> - #address-cells = <1>;
>>> - #size-cells = <0>;
>>> -
>>> - led@0 {
>>> - reg = <0>;
>>> - label = "bpi-r2:isink:green";
>>> - default-state = "off";
>>> - };
>>> -
>>> - led@1 {
>>> - reg = <1>;
>>> - label = "bpi-r2:isink:red";
>>> - default-state = "off";
>>> - };
>>> -
>>> - led@2 {
>>> - reg = <2>;
>>> - label = "bpi-r2:isink:blue";
>>> - default-state = "off";
>>> - };
>>> - };
>>> - };
>>> -};
>>> -
>>> &spi0 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&spi0_pins_a>;
>>> --
>>> 2.7.4
>>>
>
>
