Re: [GIT PULL 2/5] dt-bindings: power: Add power-domain header for JH7110

Geert Uytterhoeven
2023-09-26T10:55:29Z

Hi Conor, Changhuang,

On Wed, Sep 13, 2023 at 3:58 PM Conor Dooley <conor@kernel.org> wrote:
> From: Changhuang Liang <changhuang.liang@starfivetech.com>
>
> Add power-domain header for JH7110 SoC, it can use to operate dphy
> power.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Thanks for your patch, which is now commit 41b66b54a72bd796
("dt-bindings: power: Add power-domain header for JH7110")
in pmdomain/next.

Conor: looks like you forgot to update the patch description, as promised in
https://lore.kernel.org/all/20230519-irk-dwelled-6a499c482e62@spud?

> --- a/include/dt-bindings/power/starfive,jh7110-pmu.h
> +++ b/include/dt-bindings/power/starfive,jh7110-pmu.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> /*
> - * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + * Copyright (C) 2022-2023 StarFive Technology Co., Ltd.
> * Author: Walker Chen <walker.chen@starfivetech.com>
> */
> #ifndef __DT_BINDINGS_POWER_JH7110_POWER_H__
> @@ -14,4 +14,7 @@
> #define JH7110_PD_ISP 5
> #define JH7110_PD_VENC 6
>
> +#define JH7110_PD_DPHY_TX 0
> +#define JH7110_PD_DPHY_RX 1

These values are conflicting with similarly-named values above:

#define JH7110_PD_SYSTOP 0
#define JH7110_PD_CPU 1

Upon closer look, and diving into the driver, this header file contains
definitions for two separate number spaces: the first set is meant
to be used in PM Domain specifiers referring to a node compatible
with "starfive,jh7110-pmu", while the second set is meant to be
used in PM Domain specifiers referring to a node compatible with
"starfive,jh7110-aon-syscon".
I think it would be nice to have this reflected in the names of
the defines (e.g. JH7110_PD_AON_DPHY_TX), and/or to add comments
clearly delimiting the two separate number spaces, and explaining
where they apply to.

> +
> #endif

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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