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://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42 You need anyway SoC specific compatibles. Once you add proper compatibles, you will see that property is not needed. Best regards, Krzysztof
