[RFC v2 3/3] ath79: add support for Mikrotik RouterBoard 912G

Sergey Ryazanov ryazanov.s.a at gmail.com
Sun May 23 16:06:45 PDT 2021


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
<mail at 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.:

-------------------------------- 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



More information about the openwrt-devel mailing list