> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Sent: Friday, May 5, 2023 12:08 PM > To: Bharat Bhushan <bbhushan2@xxxxxxxxxxx>; wim@xxxxxxxxxxxxxxxxxx; > linux@xxxxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > linux-watchdog@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Subject: Re: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system > watchdog driver > > On 04/05/2023 19:10, Bharat Bhushan wrote:>>>>> +maintainers: > >>>>> + - Bharat Bhushan <bbhushan2@xxxxxxxxxxx> > >>>>> + > >>>>> +properties: > >>>>> + compatible: > >>>>> + const: marvell,gti-wdt > >> > >> So I guess we all thought "gti" means some soc. Now it is clear - you > >> miss specific compatibles. Generic blocks alone or wildcards are not allowed. > >> > >> And we have it clearly documented: > >> > >> https://urldefense.proofpoint.com/v2/url?u=https- > >> 3A__elixir.bootlin.com_linux_v6.1- > >> 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst > >> - > 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy > >> > 2YezyK7O3Hv_t2heGnouBw&m=uy18AXnbGKMIcyPkTnWrPZoVBj8yzyjlGeNemR > >> MFgMVqkT6-4JVU5hWsVbcKPzTJ&s=XkqN7nbFOrtRnOJVqrgEDA9zinZxML4- > >> qSYQPElzVeg&e= > > > > So Say currently Marvell have GTI watchdog timer in following series > > of SoCs > > - octeon > > - octeontx2 > > - cn10k > > > > Compatible for octeon series is "marvell,octeon-wdt" > > Compatible for octeontx2 series is "marvell,octeontx2-wdt" > > Compatible for cn10k series is "marvell,cn10k-wdt" > > > > So effectively we will have 3 compatibles, Is that correct? > > I don't know your SoCs. I assume you should know. > > > > >> > >>>>> + > >>>>> + reg: > >>>>> + maxItems: 1 > >>>>> + > >>>>> + interrupts: > >>>>> + maxItems: 1 > >>>>> + > >>>>> + wdt-timer-index: > >>>> > >>>> missing vendor prefix > >>> > >>> ack > >>> > >>>> > >>>> missing type > >>> > >>> Will add > >>> > >>>> > >>>>> + maxItems: 1 > >>>> > >>>> ??? > >>> > >>> Removed > >>> > >>>> > >>>>> + description: > >>>>> + This contains the timer number out of total 64 timers supported > >>>>> + by GTI hardware block. > >>>> > >>>> Why do you need it? What does it represent? > >>>> > >>>> We do not keep indices of devices other than something in reg, so > >>>> please justify why exception must be made here. > >>> > >>> Different platform have different number of GTI timers, for example > >>> some > >> platform have total 64 timer and other have 32 timers. > >>> So which GTI timer will be used for watchdog will depend on > >>> platform. So > >> added this property to enable this driver on platforms. > >> > >> This should be deducted from compatible. > > > > If I understood correctly, we should add different compatible for each soc and > use same to get the information we tried to get using "wdt-timer-index" > property, is that correct? > > > > But each series have many socs (10s) and GTI hardware is same except number > of timers they supports, so should we add that many compatibles or add a > property like this? > > Same story every time... and was discussed many, many times on the lists. > > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__elixir.bootlin.com_linux_v6.1- > 2Drc1_source_Documentation_devicetree_bindings_writing-2Dbindings.rst- > 23L42&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy > 2YezyK7O3Hv_t2heGnouBw&m=3aeejmHQ5C8TyLM5odlaq6KnLYHt4PhpJp70FQa > qbNf7w15KFHA3fmeDR2V-en4m&s=FKeW5tpOG- > CJoV_JKuqTa0k_tRrYWAQZG1UfqlW3c74&e= > > You need anyway SoC specific compatibles. Once you add proper compatibles, > you will see that property is not needed. Also on a given soc, firmware can configure one of 64 timer to be used as system watchdog time then compatible will not work. Thanks -Bharat > > > Best regards, > Krzysztof
