On 2021/02/09 7:53, Sean Anderson wrote:
> On 2/8/21 3:00 PM, Rob Herring wrote:
>> 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?
>
> Generally, no. Usually if the bindings differ from the kernel they are
> documented, but usually the device trees are just imported straight from
> the kernel. This is a bit of an unusual case in that the device tree is
> being imported from U-Boot instead of the other way around.
>
>>
>>> 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...
>
> I would like to keep Kernel and U-Boot device trees in-sync. However, if
> there are significant divergences, that becomes more difficult.
Sean,
Are you OK with removing the nodes without a driver ? I think they are the same
for the kernel and U-Boot but I have not checked in detail.
>
> --Sean
>
>>
>> Rob
>>
>
>
--
Damien Le Moal
Western Digital Research
