[PATCH v1 11/19] dt-bindings: reset: npcm: Add support for NPCM8XX

[Geert Uytterhoeven]

Hi Tomer,

On Mon, May 23, 2022 at 4:03 PM Tomer Maimon <tmaimon77 at gmail.com> wrote:
> On Mon, 23 May 2022 at 12:01, Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org> wrote:
>> On 22/05/2022 17:50, Tomer Maimon wrote:
>> > Add binding document and device tree binding
>> > constants for Nuvoton BMC NPCM8XX reset controller.
>> >
>> > Signed-off-by: Tomer Maimon <tmaimon77 at gmail.com>

>> > --- /dev/null
>> > +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
>> > @@ -0,0 +1,124 @@
>> > +/* SPDX-License-Identifier: GPL-2.0 */
>> > +// Copyright (c) 2022 Nuvoton Technology corporation.
>> > +
>> > +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
>> > +#define _DT_BINDINGS_NPCM8XX_RESET_H
>> > +
>> > +#define NPCM8XX_RESET_IPSRST1                0x20
>> > +#define NPCM8XX_RESET_IPSRST2                0x24
>> > +#define NPCM8XX_RESET_IPSRST3                0x34
>> > +#define NPCM8XX_RESET_IPSRST4                0x74
>>
>> What are these? All IDs should be incremental, decimal and start from 0.
>
> Register offset, we use the same method in NPCM7xx. please refer
> https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h
>
> and the driver asserts the reset according to the reset include definitions

So if they're easy to look up the values, you could do without the
definitions? Cfr. the interrupts properties in .dtsi files, where we
typically just use the hardcoded numbers.

If you do decide to keep them, a comment explaining their origins
would be useful.

>> > +
>> > +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */
>> > +#define NPCM8XX_RESET_GDMA0          3
>>
>> IDs start from 0 and do not have holes.
>
> This represents the reset BIT in the reset register.

Likewise, I think it's a good idea to document that in a comment, cfr.
https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/power/r8a7795-sysc.h#L8

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds