On Fri, Jan 19, 2024 at 10:28:30AM +0100, AngeloGioacchino Del Regno wrote: > Il 18/01/24 23:00, Frank Wunderlich ha scritto: > > > Gesendet: Donnerstag, 18. Januar 2024 um 17:49 Uhr > > > Von: "Conor Dooley" <conor@xxxxxxxxxx> > > > On Wed, Jan 17, 2024 at 07:41:10PM +0100, Frank Wunderlich wrote: > > > > From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx> > > > > > > > > Add reset constants for using as index in driver and dts. > > > > > > > > Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx> > > > > --- > > > > v3: > > > > - add pcie reset id as suggested by angelo > > > > > > > > v2: > > > > - add missing commit message and SoB > > > > - change value of infrareset to 0 > > > > --- > > > > include/dt-bindings/reset/mediatek,mt7988-resets.h | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h > > > > index 493301971367..0eb152889a89 100644 > > > > --- a/include/dt-bindings/reset/mediatek,mt7988-resets.h > > > > +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h > > > > @@ -10,4 +10,10 @@ > > > > /* ETHWARP resets */ > > > > #define MT7988_ETHWARP_RST_SWITCH 0 > > > > > > > > +/* INFRA resets */ > > > > +#define MT7988_INFRA_RST0_PEXTP_MAC_SWRST 0 > > > > +#define MT7988_INFRA_RST1_THERM_CTRL_SWRST 1 > > > > > > These are just "random" numbers, why not continue the numbering from the > > > ETHWARP? > > > > i can do...basicly these consts are used in DTS and driver only as index. > > > > @angelo what do you think? I though also in leaving some space to allow grouping RST0 and RST1 > > when more consts are added, else the numbers are mixed up. > > > > so e.g. let RST0 start at 20 and RST1 at 40 (or even higher, because RST0 and RST1 can have up to 32 resets). > > That will allow adding more reset constants between my values and having raising numbers. > > The resets are organized on a per-reset-controller basis, so, the ETHWARP > reset controller's first reset is RST_SWITCH, the second one is RST_something_else, > etc. while the first reset of the INFRA reset controller is PEXTP_MAC_SWRST. > > That's why ETHWARP has a reset index 0 and INFRA also starts at 0. > I think that the numbering is good as it is, and having one driver start at index 5 > while the other starts at index 12 would only overcomplicate registering the resets > in each driver, or waste bytes by making unnecessarily large arrays, for (imo) no > good reason. > > This is one header, but it should "in theory" be more than one... so we would have > one for each hardware block - but that'd make the reset directory over-crowded, as > other MediaTek SoCs have got even more resets in even more hardware blocks than the > MT7988. That'd be something like ~4 reset headers per SoC (and will increase with > newer ones)... > ...and this is why we have one binding header for resets. That's okay. The commit message leaves me, who clearly isn't a mediatek guy, with no information as to why these are not one contiguous set. IMO being for different reset controllers entirely is fine. > On the topic of leaving space to allow grouping RST0/RST1: -> No. <- > The indices have to start from zero and have to be sequential, with no holes. Agreed.
Attachment:
signature.asc
Description: PGP signature
