linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chen-Yu Tsai <wens@kernel.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: "Robin Murphy" <robin.murphy@arm.com>,
	"Chen-Yu Tsai" <wens@kernel.org>,
	"Johan Jonker" <jbx6244@gmail.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"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,
	devicetree <devicetree@vger.kernel.org>
Subject: Re: [PATCH 1/6] arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED triggers
Date: Mon, 13 Apr 2020 13:33:47 +0800	[thread overview]
Message-ID: <CAGb2v64bcVLmQPxfAUafa-Grbr1MRCVb0j=HYUrXdihbmVB2Mw@mail.gmail.com> (raw)
In-Reply-To: <20200406091313.GF31120@duo.ucw.cz>

Hi Pavel,

On Mon, Apr 6, 2020 at 5:13 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > > > 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
>
> It is enough to keep it in sync with all the triggers we actually use :-).
>
> > 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.
>
> But freeform seems acceptable, too.

I'd say free form is easier to manage. Being in the list of valid triggers
doesn't mean the kernel actually has support for it enabled.

> > > > > -             diy-led {
> > > > > +             diy_led: diy-led {
> > > > >                        label = "red:diy";
> > > > >                        gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
> > > > >                        default-state = "off";
> > > > >                        linux,default-trigger = "mmc1";
> > > > >                };
>
> This label probably should be "mmc1:red:activity" or something like that.

Is changing this after it has been in some kernel releases OK? Wouldn't
it be considered a break of sysfs ABI?

Also, is there some guideline on how to name the labels? For sunxi we've
been doing "${vendor}:${color}:${function}" since forever.

As far as I can tell, the hardware vendor [1] has no specific uses for
these two (red and yellow) LEDs designed in. And their GPIO lines are
simply labeled "DIY" (for the red one) and "YELLOW". So I'm not sure
if putting "our" interpretations and the default-trigger into the
label is wise.

For reference, the green one has its GPIO line labeled "WORK", and their
intention from [1] is to have it as some sort of power / activity indicator.
Hence it is named / labeled "work".

As for the node names, I think we can keep it as is for now. It's not
the preferred form, but there's really no need to change it either.
And some overlay or script might actually expect that name.

Regards
ChenYu


[1] http://wiki.t-firefly.com/en/ROC-RK3399-PC/driver_led.html

> > > > > -             yellow-led {
> > > > > +             yellow_led: yellow-led {
> > > > >                        label = "yellow:yellow-led";
> > > > >                        gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
> > > > >                        default-state = "off";
>
> And this label should be changed, too.
>
> Best regards,
>                                                                         Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

  reply	other threads:[~2020-04-13  5:34 UTC|newest]

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
2020-04-01 20:05         ` Jacek Anaszewski
2020-04-06  9:13         ` Pavel Machek
2020-04-13  5:33           ` Chen-Yu Tsai [this message]
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='CAGb2v64bcVLmQPxfAUafa-Grbr1MRCVb0j=HYUrXdihbmVB2Mw@mail.gmail.com' \
    --to=wens@kernel.org \
    --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=robin.murphy@arm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).