LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: "Chen-Yu Tsai" <wens@kernel.org>,
	"Johan Jonker" <jbx6244@gmail.com>,
	"Heiko Stübner" <heiko@sntech.de>
Cc: Rob Herring <robh+dt@kernel.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	jacek.anaszewski@gmail.com, pavel@ucw.cz,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/6] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
Date: Tue, 31 Mar 2020 12:07:07 +0100
Message-ID: <ccf35a92-7005-9c6d-a8a2-c17b714a60bc@arm.com> (raw)
In-Reply-To: <CAGb2v65ayZwN14S-Pzu2ip1K=fgzTbNB=ZzUcpou-jtv8m6vBA@mail.gmail.com>

[ +cc LED binding maintainers]

On 2020-03-29 5:36 pm, Chen-Yu Tsai wrote:
> On Fri, Mar 27, 2020 at 5:58 PM Johan Jonker <jbx6244@gmail.com> wrote:
>>
>> Hi Chen-Yu Tsai,
>>
>> The led node names need some changes.
>> 'linux,default-trigger' value does not fit.
>>
>>  From leds-gpio.yaml:
>>
>> patternProperties:
>>    # The first form is preferred, but fall back to just 'led' anywhere in the
>>    # node name to at least catch some child nodes.
>>    "(^led-[0-9a-f]$|led)":
>>      type: object
>>
>> Rename led nodenames to 'led-0' form
>>
>> Also include all mail lists found with:
>> ./scripts/get_maintainer.pl --nogit-fallback --nogit
>>
>> devicetree@vger.kernel.org
> 
> Oops...
> 
>> If you like change the rest of dts with leds as well...
>>
>>    DTC     arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dt.yaml
>>    CHECK   arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dt.yaml
>> arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dt.yaml: leds:
>> yellow-led:linux,default-trigger:0: 'mmc0' is not one of ['backlight',
>> 'default-on', 'heartbeat', 'disk-activity', 'ide-disk', 'timer', 'pattern']
>> arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dt.yaml: leds:
>> diy-led:linux,default-trigger:0: 'mmc1' is not one of ['backlight',
>> 'default-on', 'heartbeat', 'disk-activity', 'ide-disk', 'timer', 'pattern']
>>    DTC     arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dt.yaml
>>    CHECK   arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dt.yaml
>> arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dt.yaml: leds:
>> diy-led:linux,default-trigger:0: 'mmc2' is not one of ['backlight',
>> 'default-on', 'heartbeat', 'disk-activity', 'ide-disk', 'timer', 'pattern']
>> arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dt.yaml: leds:
>> yellow-led:linux,default-trigger:0: 'mmc1' is not one of ['backlight',
>> 'default-on', 'heartbeat', 'disk-activity', 'ide-disk', 'timer', 'pattern']
> 
> Maybe we should just get rid of linux,default-trigger then?

In this particular case, I'd say it's probably time to reevaluate the 
rather out-of-date binding. The apparent intent of the 
"linux,default-trigger" property seems to be to describe any trigger 
supported by Linux, so either the binding wants to be kept in sync with 
all the triggers Linux actually supports, or perhaps it should just be 
redefined as a free-form string. FWIW I'd be slightly inclined towards 
the latter, since the schema validator can't know whether the given 
trigger actually corresponds to the correct thing for whatever the LED 
is physically labelled on the board/case, nor whether the version(s) of 
Linux that people intend to use actually support that trigger (since it 
doesn't have to be the version contemporary with the schema definition), 
so strict validation of this particular property seems to be of limited 
value.

Robin.

> 
> Heiko?
> 
> ChenYu
> 
>> make -k ARCH=arm64 dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
>>
>>> From: Chen-Yu Tsai <wens@csie.org>
>>>
>>> With SDIO now enabled, the numbering of the existing MMC host controllers
>>> gets incremented by 1, as the SDIO host is the first one.
>>>
>>> Increment the numbering of the MMC LED triggers to match.
>>>
>>> Fixes: cf3c5397835f ("arm64: dts: rockchip: Enable sdio0 and uart0 on rk3399-roc-pc-mezzanine")
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>   arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 ++++++++
>>>   arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi          | 4 ++--
>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>> index 2acb3d500fb9..f0686fc276be 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc-mezzanine.dts
>>> @@ -38,6 +38,10 @@ vcc3v3_pcie: vcc3v3-pcie {
>>>        };
>>>   };
>>>
>>> +&diy_led {
>>> +     linux,default-trigger = "mmc2";
>>> +};
>>> +
>>>   &pcie_phy {
>>>        status = "okay";
>>>   };
>>> @@ -91,3 +95,7 @@ &uart0 {
>>>        pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
>>>        status = "okay";
>>>   };
>>> +
>>> +&yellow_led {
>>> +     linux,default-trigger = "mmc1";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> index 9f225e9c3d54..bc060ac7972d 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dtsi
>>> @@ -70,14 +70,14 @@ work-led {
>>>                        linux,default-trigger = "heartbeat";
>>>                };
>>>
>>> -             diy-led {
>>> +             diy_led: diy-led {
>>>                        label = "red:diy";
>>>                        gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
>>>                        default-state = "off";
>>>                        linux,default-trigger = "mmc1";
>>>                };
>>>
>>> -             yellow-led {
>>> +             yellow_led: yellow-led {
>>>                        label = "yellow:yellow-led";
>>>                        gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
>>>                        default-state = "off";
>>> --
>>> 2.25.1
>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27  3:04 [PATCH 0/6] arm64: dts: rockchip: misc. cleanups Chen-Yu Tsai
2020-03-27  3:04 ` [PATCH 1/6] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers Chen-Yu Tsai
2020-03-27  9:58   ` Johan Jonker
2020-03-29 16:36     ` Chen-Yu Tsai
2020-03-31 11:07       ` Robin Murphy [this message]
2020-04-01 20:05         ` Jacek Anaszewski
2020-04-06  9:13         ` Pavel Machek
2020-04-13  5:33           ` Chen-Yu Tsai
2020-03-27  3:04 ` [PATCH 2/6] arm64: dts: rockchip: rk3328: Replace RK805 PMIC node name with "pmic" Chen-Yu Tsai
2020-03-27 12:12   ` Johan Jonker
2020-03-27 12:15     ` Chen-Yu Tsai
2020-03-27  3:04 ` [PATCH 3/6] arm64: dts: rockchip: rk3328: drop non-existent gmac2phy pinmux options Chen-Yu Tsai
2020-03-27  3:04 ` [PATCH 4/6] arm64: dts: rockchip: rk3328: drop #address-cells, #size-cells from grf node Chen-Yu Tsai
2020-03-27  3:04 ` [PATCH 5/6] arm64: dts: rockchip: rk3399: drop #address-cells, #size-cells from pmugrf node Chen-Yu Tsai
2020-03-27  3:04 ` [PATCH 6/6] arm64: dts: rockchip: rk3399: Rename dwc3 device nodes to make dtc happy Chen-Yu Tsai
2020-04-19 11:36 ` [PATCH 0/6] arm64: dts: rockchip: misc. cleanups Heiko Stuebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ccf35a92-7005-9c6d-a8a2-c17b714a60bc@arm.com \
    --to=robin.murphy@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=jacek.anaszewski@gmail.com \
    --cc=jbx6244@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=pavel@ucw.cz \
    --cc=robh+dt@kernel.org \
    --cc=wens@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git