Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org]
> On Behalf Of Sergey Ryazanov
> Sent: Montag, 24. Mai 2021 01:07
> To: Adrian Schmutzler <m...@adrianschmutzler.de>; Denis Kalashnikov
> <denis281...@gmail.com>
> Cc: OpenWrt Development List <openwrt-devel@lists.openwrt.org>; Gabor
> Juhos <juh...@openwrt.org>; Koen Vandeputte
> <koen.vandepu...@citymesh.com>
> Subject: Re: [RFC v2 3/3] ath79: add support for Mikrotik RouterBoard 912G
> 
> Hello Adrian and Denis,
> 
> sorry for interfering with your conversation, I would like to discuss the best
> way to document not yet finished functionality. Please find my question and
> proposal below.
> 
> On Sun, May 23, 2021 at 12:01 PM Adrian Schmutzler
> <m...@adrianschmutzler.de> wrote:
> 
> [skipped]
> 
> >> +     beeper {
> >> +             status = "disabled";
> >> +             compatible = "gpio-beeper";
> >> +             gpios = <&ssr 5 GPIO_ACTIVE_HIGH>;
> >> +     };
> >
> > If it's broken, I'd not add it here but just name the correct GPIO number in
> the commit message.
> >
> >> +
> >> +     keys {
> >> +             status = "disabled";
> >
> > Like above: If it's broken, remove it, so nobody enables it accidentally and
> causes harm.
> > (But document how to set it up in the commit message, as you mostly
> > did already ...)
> 
> Factoring out realization details to the commit message is quite unusual, I
> personally assume that the commit message is a place where a committer
> should describe reasons for a particular change. While hard to understand
> things should be commented on in the code themself.
> 
> I agree with Adrian that a not yet finished code should not be committed to
> the master branch. But the device tree has a dualistic nature, on one hand it
> is a place for driver configuration, on the other hand it is a way to document
> board stuff interconnections. So maybe we should combine Denise's
> approach to document even a not yet fully supported component in the DTS
> with Adrian's suggestion to document why a component is not supported yet
> and place the reason to disable DTS node as comment inside the node? E.g.:

My main motivation is preventing too much bloat in the DTS files.
Nevertheless, typically, if stuff is not working it will either not be resolved 
ever or the solution will deviate from what people were thinking it would be 
initially (otherwise, they would have solved it back then). Thus, the DTS 
"implementation" of that part is either irrelevant (first case) or 
wrong/subject to change (second case) later. In both cases, I don't think it 
really makes sense to add it to the DTS in the first place.

Hovewer, I'm relatively flexible here. So if you really think it would be 
necessary to have this broken part of configuration in the DTS, I won't stop 
you because of that.

Best

Adrian

> 
> -------------------------------- 8< --------------------------- keys {
>     compatible = "gpio-keys-polled";
> 
>     reset {
>         ...
>         gpios = <&gpio 15 GPIO_ACTIVE_LOW>;
>         /*
>          * GPIO line #15 is shared between the reset button on input and
>          * the NAND ALE (via the latch) on output. We are unable to just
>          * enable the reset button. So, this node here is for
>          * documentation purposes only.
>          *
>          * [Here should be a text describing the possible ways to
>          * overcome shared line issues]
>          */
>         status = "disabled";
>     };
> };
> -------------------------------- 8< ---------------------------
> 
> This way we will keep all interconnection documentation in DTS and at the
> same time we will make sure that no sane user enables this node.
> 
> > > +             compatible = "gpio-keys";
> > > +             reset {
> > > +                     label = "reset";
> > > +                     linux,code = <KEY_RESTART>;
> > > +                     gpios = <&gpio 15 GPIO_ACTIVE_LOW>;
> > > +                     debounce-interval = <60>;
> > > +             };
> > > +     };
> 
> --
> Sergey
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Attachment: openpgp-digital-signature.asc
Description: PGP signature

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

Reply via email to