RE: [EXT] Re: [PATCH 1/2 v5] dt-bindings: watchdog: marvell GTI system watchdog driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux