| Message ID | 20240104173930.13907-2-linux@fw-web.de (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add reset controller to mt7988 infracfg | expand |
Hi Frank, On Thu, Jan 04, 2024 at 06:39:29PM +0100, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > --- > include/dt-bindings/reset/mediatek,mt7988-resets.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h > index 493301971367..3f1e4ec07ad5 100644 > --- a/include/dt-bindings/reset/mediatek,mt7988-resets.h > +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h > @@ -10,4 +10,8 @@ > /* ETHWARP resets */ > #define MT7988_ETHWARP_RST_SWITCH 0 > > +/* INFRA resets */ > +#define MT7988_INFRA_RST0_THERM_CTRL_SWRST 9 I suppose this argument applies here as well: "IDs should start from 0 or 1 and increment by 1. If these are not IDs, then you do not need them in the bindings." https://lore.kernel.org/all/59629ec1-cc0c-4c5a-87cc-ea30d64ec191@linaro.org/ As a consequence, as what you are describing there are hardware bits rather than IDs used by the driver, you can just use a numeric constant in device tree instead of adding dt-bindings header. Or change the driver so RST0_THERM_CTRL_SWRST could be defined as 0.
On 04/01/2024 19:12, Daniel Golle wrote: > Hi Frank, > > On Thu, Jan 04, 2024 at 06:39:29PM +0100, Frank Wunderlich wrote: >> From: Frank Wunderlich <frank-w@public-files.de> >> >> --- >> include/dt-bindings/reset/mediatek,mt7988-resets.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h >> index 493301971367..3f1e4ec07ad5 100644 >> --- a/include/dt-bindings/reset/mediatek,mt7988-resets.h >> +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h >> @@ -10,4 +10,8 @@ >> /* ETHWARP resets */ >> #define MT7988_ETHWARP_RST_SWITCH 0 >> >> +/* INFRA resets */ >> +#define MT7988_INFRA_RST0_THERM_CTRL_SWRST 9 > > I suppose this argument applies here as well: > > "IDs should start from 0 or 1 and increment by 1. If these are not IDs, > then you do not need them in the bindings." > > https://lore.kernel.org/all/59629ec1-cc0c-4c5a-87cc-ea30d64ec191@linaro.org/ > > As a consequence, as what you are describing there are hardware bits If this is existing driver which already uses such pattern, then it is fine. I usually comment this on new drivers which can be changed. Best regards, Krzysztof
Hi > Gesendet: Donnerstag, 04. Januar 2024 um 20:19 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org> > Betreff: Re: [PATCH 1/2] dt-bindings: reset: mediatek: add MT7988 LVTS reset ID > > On 04/01/2024 19:12, Daniel Golle wrote: > > Hi Frank, > > > > On Thu, Jan 04, 2024 at 06:39:29PM +0100, Frank Wunderlich wrote: > >> From: Frank Wunderlich <frank-w@public-files.de> > >> > >> --- > >> include/dt-bindings/reset/mediatek,mt7988-resets.h | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h > >> index 493301971367..3f1e4ec07ad5 100644 > >> --- a/include/dt-bindings/reset/mediatek,mt7988-resets.h > >> +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h > >> @@ -10,4 +10,8 @@ > >> /* ETHWARP resets */ > >> #define MT7988_ETHWARP_RST_SWITCH 0 > >> > >> +/* INFRA resets */ > >> +#define MT7988_INFRA_RST0_THERM_CTRL_SWRST 9 > > > > I suppose this argument applies here as well: > > > > "IDs should start from 0 or 1 and increment by 1. If these are not IDs, > > then you do not need them in the bindings." > > > > https://lore.kernel.org/all/59629ec1-cc0c-4c5a-87cc-ea30d64ec191@linaro.org/ > > > > As a consequence, as what you are describing there are hardware bits > > If this is existing driver which already uses such pattern, then it is > fine. I usually comment this on new drivers which can be changed. this is a new driver so i guess i should change this like daniel suggests. As i want to use this constant in dts and driver i would like keep it as binding in the reset header, but because i use it only as index in the infra_idx_map array its value does not need to have the value needed in hardware. i kept it same to not have different values and for ordering purposes (when the other possible resets are added). so the way starting at 0 will be the preferred one for me, is this ok? regards Frank
diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h index 493301971367..3f1e4ec07ad5 100644 --- a/include/dt-bindings/reset/mediatek,mt7988-resets.h +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h @@ -10,4 +10,8 @@ /* ETHWARP resets */ #define MT7988_ETHWARP_RST_SWITCH 0 +/* INFRA resets */ +#define MT7988_INFRA_RST0_THERM_CTRL_SWRST 9 + #endif /* _DT_BINDINGS_RESET_CONTROLLER_MT7988 */ +
From: Frank Wunderlich <frank-w@public-files.de> --- include/dt-bindings/reset/mediatek,mt7988-resets.h | 4 ++++ 1 file changed, 4 insertions(+)