Hello Adrian,
On Mon, May 31, 2021 at 1:15 AM Adrian Schmutzler
<[email protected]> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: openwrt-devel [mailto:[email protected]]
> > On Behalf Of Sergey Ryazanov
> > Sent: Montag, 24. Mai 2021 01:07
> > To: Adrian Schmutzler <[email protected]>; Denis Kalashnikov
> > <[email protected]>
> > Cc: OpenWrt Development List <[email protected]>; Gabor
> > Juhos <[email protected]>; Koen Vandeputte
> > <[email protected]>
> > 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
> > <[email protected]> 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.
Well! These reasons for not adding broken nodes to DTS sounds
perfectly reasonable for me.
But what is the best way to document hard-to-find but not yet
functional GPIO lines then? Wiki? If so, should we add a wiki page URL
as a comment to the DTS to facilitate work of future contributors?
> 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.
>
> >
> > -------------------------------- 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
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
