Message ID | 20230913-grumbly-rewrite-34c85539f2ed@spud (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Missed starfive pmdomain changes for v6.6, now for v6.7 | expand |
Context | Check | Description |
---|---|---|
conchuod/tree_selection | fail | Failed to apply to next/pending-fixes, riscv/for-next or riscv/master |
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
On Tue, Sep 26, 2023 at 12:55:01PM +0200, Geert Uytterhoeven wrote: > 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? *sigh*, that's what happens when you apply the patches several months after they were sent I guess. I'll try to make sure that that doesn't happen again. > > --- 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". Yeah, apologies for the confusion there. I maybe should have requested an additional file, since the relevant compatibles are different. > 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. Since this is 6.7 material, there's still plenty of time remaining to make either of these changes. Changhuang Liang, do you think you would be able to send a patch implementing either of the suggestions made by Geert? Thanks, Conor.
On 2023/9/26 18:55, Geert Uytterhoeven wrote: > 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. > Hi Geert, Where is the website link for the pmdomain/next? Thanks, Changhuang
>>> #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". > > Yeah, apologies for the confusion there. I maybe should have > requested an additional file, since the relevant compatibles > are different. > >> 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. > > Since this is 6.7 material, there's still plenty of time remaining to > make either of these changes. Changhuang Liang, do you think you would > be able to send a patch implementing either of the suggestions made by > Geert? > Hi Conor, I will send a patch to fix this confusion. Thanks, Changhuang
Hi Changhuang, On Wed, Sep 27, 2023 at 3:25 AM Changhuang Liang <changhuang.liang@starfivetech.com> wrote: > On 2023/9/26 18:55, Geert Uytterhoeven wrote: > > 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. > > Where is the website link for the pmdomain/next? As per https://elixir.bootlin.com/linux/v6.6-rc3/source/MAINTAINERS#L8729 it can be found at https://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm.git/log/?h=next Gr{oetje,eeting}s, Geert
On 2023/9/26 18:55, Geert Uytterhoeven wrote: > 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. > Hi Geert, I have sent a series to fix this, but I forget to send it to you. https://patchwork.kernel.org/project/linux-pm/patch/20230927130734.9921-2-changhuang.liang@starfivetech.com/ Thanks, Changhuang
diff --git a/include/dt-bindings/power/starfive,jh7110-pmu.h b/include/dt-bindings/power/starfive,jh7110-pmu.h index 132bfe401fc8..341e2a0676ba 100644 --- 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 + #endif