Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Christian Lamparter]

On Sunday, December 30, 2018 12:17:25 PM CET Petr Štetiar wrote:
> In DTS Checklist[1] we're now demanding proper generic node names, as
> the name of a node should reflect the function of the device and use
> generic name for that[2]. Everybody seems to be copy&pasting from DTS
> files available in the repository today, so let's unify that naming
> there as well and provide proper examples.
> 
> 1. https://openwrt.org/submitting-patches#dts_checklist
> 2. 
> https://github.com/devicetree-org/devicetree-specification/blob/master/source/devicetree-basics.rst#generic-names-recommendation
> 
> Signed-off-by: Petr Štetiar 
> ---

Another source of the "gpio-keys" and the "gpio_keys_polled" 
node names are the binding examples in

https://www.kernel.org/doc/Documentation/devicetree/bindings/input/gpio-keys-polled.txt
https://www.kernel.org/doc/Documentation/devicetree/bindings/input/gpio-keys.txt

As a result, these node-names are really common in a lot of upstream bindings
(I grepped around 350 gpio(_|-)keys* nodes located in various .dts and .dtsi
files under linux-kernel/arch/... ).

I hope you know what you are up against because unless you also do the changes
upstream this will happen again and again. :\ / :)

Regards,
Christian



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Petr Štetiar]

Christian Lamparter  [2018-12-31 17:41:34]:

> I hope you know what you are up against because unless you also do the changes
> upstream this will happen again and again. :\ / :)

My plan is to first wait for comments here, see if it gets merged eventualy
and then start poking upstream. I still didn't received any feedback yet(good
sign?) on my last `treewide: dts: Remove default-state=off property...`[1]
upstream attempt so I don't know if it's worth the effort.

Anyway, I guess, that in most of the cases, people are just copy&pasting from
the DTS files from the OpenWrt repository and some of them even wonder[2] why
they need to use generic `leds` node names if it's not the case in the rest of
the DTS files in OpenWrt tree:

 @mkresin: would you please rename the node to the generic "leds".
 @arapov: @mkresin, by the way, the rest of *.dts in ramips/ using gpio-leds.
  Do you still think it is good to deviate from the rest here?

1. https://patchwork.kernel.org/patch/10732465/
2. https://github.com/openwrt/openwrt/pull/1686#discussion_r244512451
3. https://openwrt.org/submitting-patches#dts_checklist

-- ynezz

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Christian Lamparter]

On Tuesday, January 1, 2019 6:07:40 PM CET Petr Štetiar wrote:
> Christian Lamparter  [2018-12-31 17:41:34]:
> 
> > I hope you know what you are up against because unless you also do the 
> > changes
> > upstream this will happen again and again. :\ / :)
> 
> My plan is to first wait for comments here, see if it gets merged eventualy
> and then start poking upstream. I still didn't received any feedback yet(good
> sign?) on my last `treewide: dts: Remove default-state=off property...`[1]
> upstream attempt so I don't know if it's worth the effort.
Hm, interesting. I usually get replies within a few days. Granted, I have never
sent anything that big in a single mail to multiple mailinglists and 
maintainers. I would try to split up the patch into multiple patches so that
each maintainer has the chance to act on just his own turf. 
Keep in mind that linux-kernel is heavily compartmentalized. The device-tree
maintainers mainly just ack/review patches for the subsystem maintainers.
This is done in order to prevent the conflicts between the various trees when
they get staged into -next and ultimately wander into the kernel during the
"merge window". 

I guess if you still want to follow through you could start to update the
binding documents. But, I do understand that you don't want to waste 
anymore time with it.

> Anyway, I guess, that in most of the cases, people are just copy&pasting from
> the DTS files from the OpenWrt repository and some of them even wonder[2] why
> they need to use generic `leds` node names if it's not the case in the rest of
> the DTS files in OpenWrt tree:
> 
>  @mkresin: would you please rename the node to the generic "leds".
>  @arapov: @mkresin, by the way, the rest of *.dts in ramips/ using gpio-leds.
>   Do you still think it is good to deviate from the rest here?
> 
> 1. https://patchwork.kernel.org/patch/10732465/
> 2. https://github.com/openwrt/openwrt/pull/1686#discussion_r244512451
True, I think you noticed that it's a surprisingly long and weird difficult
process to push these sort of changes upstream unless you are directly
involved there. However it's much easier to comment on the daily patches/PRs
and make sure that new boards/dts are up to spec with the latest craze and
also, you get the chance to interact with the commiters a bit.

> 3. https://openwrt.org/submitting-patches#dts_checklist
^^ I know that one only too well.
"The name of a node should reflect the function of the device and not its 
model. "
I c&p that from the device-tree spec and linked to it so devs know from where
these seemingly arbitrary rules come from. While looking at the checklist, I 
noticed that one of the "SPDX license tag" check is already automated in the
upstream scripts/checkpatch.pl... And now, I wish that the script could also
act on default-state = "off", the "gpio-keys-polled" and "gpio-leds" 
node names, etc.

Oh well.

Regards,
Christian



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Petr Štetiar]

Christian Lamparter  [2019-01-03 18:27:40]:

> I would try to split up the patch into multiple patches so that
> each maintainer has the chance to act on just his own turf. 

I don't want to waste more of my time on such noop stuff, I've tried it so
let's see how it pans out :-)

> Keep in mind that linux-kernel is heavily compartmentalized. The device-tree
> maintainers mainly just ack/review patches for the subsystem maintainers.

Yea, just give me some feedback and I'm more then happy to do what is
necessary to finish this crusade, but until then I'll just put it on ice.

> However it's much easier to comment on the daily patches/PRs
> and make sure that new boards/dts are up to spec with the latest craze and
> also, you get the chance to interact with the commiters a bit.

Yea, almost every submission has some copy&pasted stuff (I'm guilty as well),
so just trying to make it easier for everyone. Provide good examples for
copy&pasting, saving some time of submitters, reviewers and commiters.

> While looking at the checklist, I noticed that one of the "SPDX license tag"
> check is already automated in the upstream scripts/checkpatch.pl... And now,
> I wish that the script could also act on default-state = "off", the
> "gpio-keys-polled" and "gpio-leds" node names, etc.

Indeed, it would be nice to automate this and other checks and integrate it
into GitHub's PR pipeline via some CI system. Well, one day :-)

-- ynezz

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Christian Lamparter]

On Thursday, January 3, 2019 10:39:08 PM CET Petr Štetiar wrote:
> Christian Lamparter  [2019-01-03 18:27:40]:
> 
> > I would try to split up the patch into multiple patches so that
> > each maintainer has the chance to act on just his own turf. 
> 
> I don't want to waste more of my time on such noop stuff, I've tried it so
> let's see how it pans out :-)

Well, I've split both patches (this and gpio-leds) up and put them into my
staging tree.



The at91, apm821xx, ipq40xx, ipq806x, layerscape and oxnas are probably
ready to be merged.

The situation with ramips, ath79 and lantiq is a bit more complicated:



because some of these devices also have a separated ath9k-leds 
and in theory these nodes "could be merged". However, this is
going to be a bad idea and so I left the "gpio-leds" in these cases
as is. Maybe "soc-leds" could be a better name, but I don't think
upstream will care much about that.

> 
> > Keep in mind that linux-kernel is heavily compartmentalized. The device-tree
> > maintainers mainly just ack/review patches for the subsystem maintainers.
> 
> Yea, just give me some feedback and I'm more then happy to do what is
> necessary to finish this crusade, but until then I'll just put it on ice.
Feedback for merging patches upstream or into openwrt? For upstream:
Have you talked to Rob or Mark? Usually, they do review device-tree
changes related to drivers and suchs relatively quickly. However, something
must have caused them to ignore the default-state = off patch. I can't
even find it in the devicetree patchwork. maybe it was too big?

> > While looking at the checklist, I noticed that one of the "SPDX license tag"
> > check is already automated in the upstream scripts/checkpatch.pl... And now,
> > I wish that the script could also act on default-state = "off", the
> > "gpio-keys-polled" and "gpio-leds" node names, etc.
> 
> Indeed, it would be nice to automate this and other checks and integrate it
> into GitHub's PR pipeline via some CI system. Well, one day :-)
Why not start there? Upstream has a accumulated a vast library of semantic
patches (http://coccinelle.lip6.fr/) and from what I can tell, these have 
a pretty good track record to get accepted. (I guess the main difficulty here
will be to check whenever spatch already supports dts/dtsi files or not).

Regards,
Christian



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Petr Štetiar]

Christian Lamparter  [2019-01-22 17:54:13]:

Hi,

> The at91, apm821xx, ipq40xx, ipq806x, layerscape and oxnas are probably
> ready to be merged.

a lot of additional work, thanks!

> The situation with ramips, ath79 and lantiq is a bit more complicated:
> 
> 
> 
> because some of these devices also have a separated ath9k-leds 
> and in theory these nodes "could be merged". However, this is
> going to be a bad idea and so I left the "gpio-leds" in these cases
> as is. Maybe "soc-leds" could be a better name, but I don't think
> upstream will care much about that.

I've noticed those during my sed session as well, but checked those manualy
and it looked fine, but maybe I've overlooked something, sorry for that.

> Feedback for merging patches upstream or into openwrt? For upstream:
> Have you talked to Rob or Mark? Usually, they do review device-tree
> changes related to drivers and suchs relatively quickly. 

Nope, the patch was sent just a few days ago (December 16th), so for me it's
still too early trying to ping someone about it.

> However, something must have caused them to ignore the default-state = off
> patch. I can't even find it in the devicetree patchwork. maybe it was too
> big?

It's hidden here https://patchwork.kernel.org/patch/10732469/ even with 'To:
devicet...@vger.kernel.org' header, so maybe the patchwork/mail pipeline
hickup?

> Why not start there? Upstream has a accumulated a vast library of semantic
> patches (http://coccinelle.lip6.fr/) and from what I can tell, these have 
> a pretty good track record to get accepted. (I guess the main difficulty here
> will be to check whenever spatch already supports dts/dtsi files or not).

Nice tool, but still without DT support.

-- ynezz

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] treewide: dts: Unify naming of gpio-keys nodes

[Christian Lamparter]

Hello,

I've just pushed the remaining patches. Except for lantiq.
But these are moving along in Mathias Kresin' staging tree:


Thanks,
Christian



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel