Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 5, 2021 at 6:13 PM Damien Le Moal <Damien.LeMoal@xxxxxxx> wrote:
>
> On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
> [...]
> > > +                   otp0: nvmem@50420000 {
> > > +                           #address-cells = <1>;
> > > +                           #size-cells = <1>;
> > > +                           compatible = "canaan,k210-otp";
> > > +                           reg = <0x50420000 0x100>,
> > > +                                 <0x88000000 0x20000>;
> > > +                           reg-names = "reg", "mem";
> > > +                           clocks = <&sysclk K210_CLK_ROM>;
> > > +                           resets = <&sysrst K210_RST_ROM>;
> > > +                           read-only;
> > > +                           status = "disabled";
> >
> > Your disabled nodes seem a bit excessive. A device should really only be
> > disabled if it's a board level decision to use or not. I'd assume the
> > OTP is always there and usable.
>
> Please see below.
>
> >
> > > +
> > > +                           /* Bootloader */
> > > +                           firmware@00000 {
> >
> > Drop leading 0s.
> >
> > Is this memory mapped? If so, you are missing 'ranges' in the parent to
> > make it translateable.
> >
> > > +                                   reg = <0x00000 0xC200>;
> > > +                           };
> > > +
> > > +                           /*
> > > +                            * config string as described in RISC-V
> > > +                            * privileged spec 1.9
> > > +                            */
> > > +                           config-1-9@1c000 {
> > > +                                   reg = <0x1C000 0x1000>;
> > > +                           };
> > > +
> > > +                           /*
> > > +                            * Device tree containing only registers,
> > > +                            * interrupts, and cpus
> > > +                            */
> > > +                           fdt@1d000 {
> > > +                                   reg = <0x1D000 0x2000>;
> > > +                           };
> > > +
> > > +                           /* CPU/ROM credits */
> > > +                           credits@1f000 {
> > > +                                   reg = <0x1F000 0x1000>;
> > > +                           };
> > > +                   };
> > > +
> > > +                   dvp0: camera@50430000 {
> > > +                           compatible = "canaan,k210-dvp";
> >
> > No documented. Seems to be several of them.
>
> There are no Linux drivers for these undocumented nodes. That is why I did not
> add any documentation.

Documentation is required when dts files OR Linux drivers use them.

> make dtbs_check does not complain about that as long as
> the nodes are marked disabled.

'disabled' should only turn off required properties missing checks.
Undocumented compatible strings checks are about to get turned on now
that I've made it work without false positives.

> I kept these nodes to have the DTS in sync with
> U-Boot which has them.

That's a worthwhile goal. Doesn't u-boot require documenting bindings?

> Keeping them also creates documentation for the SoC
> since this device tree is more detailed than the SoC specsheet...

It's already 'documented' in u-boot it seems...

Rob



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux