[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