diff mbox series

[1/2,v7] dt-bindings: watchdog: marvell GTI system watchdog driver

Message ID 20230508131515.19403-1-bbhushan2@marvell.com
State New
Headers show
Series [1/2,v7] dt-bindings: watchdog: marvell GTI system watchdog driver | expand

Commit Message

Bharat Bhushan May 8, 2023, 1:15 p.m. UTC
Add binding documentation for the Marvell GTI system
watchdog driver.

Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
---
v7:
 - Corrected compatible to have soc name
 - Converted marvell,wdt-timer-index to optional

 .../watchdog/marvell,octeontx2-wdt.yaml       | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml

Comments

Bharat Bhushan May 9, 2023, 9:01 a.m. UTC | #1
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, May 9, 2023 1:38 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org;
> linux@roeck-us.net; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Subject: Re: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system
> watchdog driver
> 
> On 09/05/2023 09:26, Bharat Bhushan wrote:
> 
> 
> >>> +properties:
> >>> +  compatible:
> >>> +    oneOf:
> >>> +      - const: marvell,octeontx2-wdt
> >>
> >> Why is this alone? Judging by the enum below, octeontx2 is not specific.
> >>
> >>> +      - items:
> >>> +          - enum:
> >>> +              - marvell,octeontx2-95xx-wdt
> >>> +              - marvell,octeontx2-96xx-wdt
> >>> +              - marvell,octeontx2-98xx-wdt
> >>
> >> We don't allow wildcards in general
> >
> > Marvell have octeontx2 series of processor which have watchdog timer.
> > In 95xx,98xx,96xx are the processors in octeontx2 series of processor. So
> octeontx2-95xx is on soc, octeontx2-96xx is another and so on.
> 
> No, 95xx is not a processor. Otherwise please point me to exact product
> datasheet. Hint: I checked it.

Looks like 95xx data sheet is not public, will remove in that case.

> 
> >
> >>
> >>> +          - const: marvell,octeontx2-wdt
> >>> +      - const: marvell,cn10k-wdt
> >>
> >> Same question - why is this alone?
> > Same here, Marvell have cn10k series of processors and cn10kx and cnf10kx are
> the processor in this series.
> 
> I don't understand how does it explain my concern. This is alone because there
> are series of processors? How is that related?

Tried to make it look like other drivers. Let's keep it simple, we want to enable this only for below ones

properties:
  compatible:
    enum:
     - marvell,cn10k-wdt
     - marvell,octeontx2-wdt

Are you good with that?

Thanks
-Bharat

> 
> >
> > One of the difference between octeontx2 and cn10k series processor is
> number of timers available. Which within the available set of timers one of the
> timer is programmed to be watchdog timer.
> 
> Wrap your replies. It's difficult to read.
> 
> >
> > Can you please propose how you want these compatible to be defined?
> 
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__elixir.bootlin.com_linux_v6.4-
> 2Drc1_source_Documentation_devicetree_bindings_soc_qcom_qcom-
> 2Ceud.yaml-
> 23L19&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGbCLmy
> 2YezyK7O3Hv_t2heGnouBw&m=sbDC9A17UO1l_M7xW5546TQhAMxoejy6M_sv
> PitOn_9sOxb0ru3H7X9eEW00Gqna&s=tQrCoVSHNEOd9CkEeu6leJbmP0rtbL5Vd
> WlE9GQ-GTI&e=
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski May 9, 2023, 1:06 p.m. UTC | #2
On 09/05/2023 11:01, Bharat Bhushan wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Tuesday, May 9, 2023 1:38 PM
>> To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org;
>> linux@roeck-us.net; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com>
>> Subject: Re: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system
>> watchdog driver
>>
>> On 09/05/2023 09:26, Bharat Bhushan wrote:
>>
>>
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    oneOf:
>>>>> +      - const: marvell,octeontx2-wdt
>>>>
>>>> Why is this alone? Judging by the enum below, octeontx2 is not specific.
>>>>
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - marvell,octeontx2-95xx-wdt
>>>>> +              - marvell,octeontx2-96xx-wdt
>>>>> +              - marvell,octeontx2-98xx-wdt
>>>>
>>>> We don't allow wildcards in general
>>>
>>> Marvell have octeontx2 series of processor which have watchdog timer.
>>> In 95xx,98xx,96xx are the processors in octeontx2 series of processor. So
>> octeontx2-95xx is on soc, octeontx2-96xx is another and so on.
>>
>> No, 95xx is not a processor. Otherwise please point me to exact product
>> datasheet. Hint: I checked it.
> 
> Looks like 95xx data sheet is not public, will remove in that case.

We can talk about 96xx. Can you point me to the SoC named exactly like
this? Hint: I checked it.


> 
>>
>>>
>>>>
>>>>> +          - const: marvell,octeontx2-wdt
>>>>> +      - const: marvell,cn10k-wdt
>>>>
>>>> Same question - why is this alone?
>>> Same here, Marvell have cn10k series of processors and cn10kx and cnf10kx are
>> the processor in this series.
>>
>> I don't understand how does it explain my concern. This is alone because there
>> are series of processors? How is that related?
> 
> Tried to make it look like other drivers. Let's keep it simple, we want to enable this only for below ones

Enable what? None of these explains why do you need this entry alone,
since it is covered by list further.

> 
> properties:
>   compatible:
>     enum:
>      - marvell,cn10k-wdt
>      - marvell,octeontx2-wdt
> 
> Are you good with that?

Not sure, it sounds like it ignores our entire discussion. What's the
name of the SoC? I see "OcteonTx2 CN9130", not "OcteonTx2". I linked
your previously guide how to write bindings. Did you read it?

Best regards,
Krzysztof
Sunil Kovvuri Goutham May 16, 2023, 10:06 a.m. UTC | #3
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, May 9, 2023 6:36 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org;
> linux@roeck-us.net; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Subject: Re: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system
> watchdog driver
> 
> On 09/05/2023 11:01, Bharat Bhushan wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Tuesday, May 9, 2023 1:38 PM
> >> To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org;
> >> linux@roeck-us.net; robh+dt@kernel.org;
> >> krzysztof.kozlowski+dt@linaro.org;
> >> linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com>
> >> Subject: Re: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell
> >> GTI system watchdog driver
> >>
> >> On 09/05/2023 09:26, Bharat Bhushan wrote:
> >>
> >>
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    oneOf:
> >>>>> +      - const: marvell,octeontx2-wdt
> >>>>
> >>>> Why is this alone? Judging by the enum below, octeontx2 is not specific.
> >>>>
> >>>>> +      - items:
> >>>>> +          - enum:
> >>>>> +              - marvell,octeontx2-95xx-wdt
> >>>>> +              - marvell,octeontx2-96xx-wdt
> >>>>> +              - marvell,octeontx2-98xx-wdt
> >>>>
> >>>> We don't allow wildcards in general
> >>>
> >>> Marvell have octeontx2 series of processor which have watchdog timer.
> >>> In 95xx,98xx,96xx are the processors in octeontx2 series of
> >>> processor. So
> >> octeontx2-95xx is on soc, octeontx2-96xx is another and so on.
> >>
> >> No, 95xx is not a processor. Otherwise please point me to exact
> >> product datasheet. Hint: I checked it.
> >
> > Looks like 95xx data sheet is not public, will remove in that case.
> 
> We can talk about 96xx. Can you point me to the SoC named exactly like this?
> Hint: I checked it.

To recap what Bharat mentioned before along with references to individual processors.
OcteonTx2 is a family of processors
https://www.marvell.com/products/data-processing-units.html
Please check for "OCTEON TX2 DPUs"
CN96xx and CN98xx are two silicon variants in this family.
https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-infrastructure-processors-octeon-tx2-cn92xx-cn96xx-cn98xx-product-brief-2020-02.pdf
And CNF95xx is another silicon variant in the same family.
https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-infrastructure-processors-octeon-fusion-cnf95xx-product-brief.pdf

Since the HW block is same in all the variants of silicons in this family, we would like to use a
generic string instead of different compatible string for each one. ie
- const: marvell,octeontx2-wdt
Hope this is okay.

Same with CN10K or Octeon10 family of silicons.
https://www.marvell.com/products/data-processing-units.html
Please check for "OCTEON 10"

CN103xx and CN106xx are two silicons in this family.
https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-octeon-10-dpu-platform-product-brief.pdf
Same with CNF105xx
https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-octeon-10-fusion-cnf105xx-product-brief.pdf

For this family we would like to use
- const: marvell,cn10k-wdt
as the compatible string, as it represents all silicon variants in this family.

Thanks,
Sunil.
Krzysztof Kozlowski May 16, 2023, 10:25 a.m. UTC | #4
On 16/05/2023 12:06, Sunil Kovvuri Goutham wrote:


>>>>> Marvell have octeontx2 series of processor which have watchdog timer.
>>>>> In 95xx,98xx,96xx are the processors in octeontx2 series of
>>>>> processor. So
>>>> octeontx2-95xx is on soc, octeontx2-96xx is another and so on.
>>>>
>>>> No, 95xx is not a processor. Otherwise please point me to exact
>>>> product datasheet. Hint: I checked it.
>>>
>>> Looks like 95xx data sheet is not public, will remove in that case.
>>
>> We can talk about 96xx. Can you point me to the SoC named exactly like this?
>> Hint: I checked it.
> 
> To recap what Bharat mentioned before along with references to individual processors.
> OcteonTx2 is a family of processors
> https://www.marvell.com/products/data-processing-units.html
> Please check for "OCTEON TX2 DPUs"
> CN96xx and CN98xx are two silicon variants in this family.
> https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-infrastructure-processors-octeon-tx2-cn92xx-cn96xx-cn98xx-product-brief-2020-02.pdf

This is a product brief which further suggests CN96xx is a family (or
sub-family).

"xx" is pretty often used as family, not as product. Otherwise how one
product CN92XX can come with 12-18 cores *in the same time*?

https://www.marvell.com/company/newsroom/marvell-announces-octeon-tx2-family-of-multi-core-infrastructure-processors.html
"Marvell’s CN91xx, CN92xx, CN96xx, and CN98xx processor families include:"

https://www.marvell.com/products/data-processing-units.html


> And CNF95xx is another silicon variant in the same family.
> https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-infrastructure-processors-octeon-fusion-cnf95xx-product-brief.pdf

Again, unspecific product brief. Your other briefs specify them clearer,
e.g. CN9130, CN9131

> 
> Since the HW block is same in all the variants of silicons in this family, we would like to use a
> generic string instead of different compatible string for each one. ie
> - const: marvell,octeontx2-wdt
> Hope this is okay.

https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42

> 
> Same with CN10K or Octeon10 family of silicons.
> https://www.marvell.com/products/data-processing-units.html
> Please check for "OCTEON 10"
> 
> CN103xx and CN106xx are two silicons in this family.

Are they? "Up to 8" cores, so how this can be one specific silicon? One
customer buys CN10300 with 8 cores, second buys exactly the same CN10300
and has 4 cores?

You are mixing families and specific devices.

Best regards,
Krzysztof
Sunil Kovvuri Goutham May 16, 2023, 11:24 a.m. UTC | #5
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, May 16, 2023 3:55 PM
> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Bharat Bhushan
> <bbhushan2@marvell.com>; wim@linux-watchdog.org; linux@roeck-us.net;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux-
> watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system
> watchdog driver
> 
> On 16/05/2023 12:06, Sunil Kovvuri Goutham wrote:
> 
> 
> >>>>> Marvell have octeontx2 series of processor which have watchdog timer.
> >>>>> In 95xx,98xx,96xx are the processors in octeontx2 series of
> >>>>> processor. So
> >>>> octeontx2-95xx is on soc, octeontx2-96xx is another and so on.
> >>>>
> >>>> No, 95xx is not a processor. Otherwise please point me to exact
> >>>> product datasheet. Hint: I checked it.
> >>>
> >>> Looks like 95xx data sheet is not public, will remove in that case.
> >>
> >> We can talk about 96xx. Can you point me to the SoC named exactly like this?
> >> Hint: I checked it.
> >
> > To recap what Bharat mentioned before along with references to individual
> processors.
> > OcteonTx2 is a family of processors
> > https://www.marvell.com/products/data-processing-units.html
> > Please check for "OCTEON TX2 DPUs"
> > CN96xx and CN98xx are two silicon variants in this family.
> > https://www.marvell.com/content/dam/marvell/en/public-collateral/embed
> > ded-processors/marvell-infrastructure-processors-octeon-tx2-cn92xx-cn9
> > 6xx-cn98xx-product-brief-2020-02.pdf
> 
> This is a product brief which further suggests CN96xx is a family (or sub-family).
> 
> "xx" is pretty often used as family, not as product. Otherwise how one product
> CN92XX can come with 12-18 cores *in the same time*?

"xx" here suggests skews, some 92xx may have 18 cores and some may have
few cores fused out resulting in 12cores.

> 
> https://www.marvell.com/company/newsroom/marvell-announces-octeon-
> tx2-family-of-multi-core-infrastructure-processors.html
> "Marvell’s CN91xx, CN92xx, CN96xx, and CN98xx processor families include:"
> 
> https://www.marvell.com/products/data-processing-units.html
> 
> 
> > And CNF95xx is another silicon variant in the same family.
> > https://www.marvell.com/content/dam/marvell/en/public-collateral/embed
> > ded-processors/marvell-infrastructure-processors-octeon-fusion-cnf95xx
> > -product-brief.pdf
> 
> Again, unspecific product brief. Your other briefs specify them clearer, e.g.
> CN9130, CN9131

Not sure what you are looking for in the product brief, I pointed to the link just to show proof
of 95xx being an official product.

> 
> >
> > Since the HW block is same in all the variants of silicons in this
> > family, we would like to use a generic string instead of different
> > compatible string for each one. ie
> > - const: marvell,octeontx2-wdt
> > Hope this is okay.
> 
> 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=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=q3VKxXQKiboRw_F01ggTz
> HuhwawxR1P9_tMCN2FODU4&m=GGfz5QI8ldHRqao5OsrfuHZQso5LLNBeBxCZr
> Ai7Zow-
> qoKl_S1Yw90OWnSZ3FFx&s=kM0VFY1b15BYvp2EUapQjZ6Eb96aZ_yAE76EKCmA
> aEQ&e=
> 
> >
> > Same with CN10K or Octeon10 family of silicons.
> > https://www.marvell.com/products/data-processing-units.html
> > Please check for "OCTEON 10"
> >
> > CN103xx and CN106xx are two silicons in this family.
> 
> Are they? "Up to 8" cores, so how this can be one specific silicon? One customer
> buys CN10300 with 8 cores, second buys exactly the same CN10300 and has 4
> cores?
> 
> You are mixing families and specific devices.

I was making it simple to understand.

OcteonTx2 and Octeon10 (CN10K) are two generations of Octeon processors from Marvell.
Within each generation there are multiple silicon variants.
Again for each variant there are multiple skews.

Since the watchdog hardware block functionality is same across all of above
generations / variants / families / skews, is it okay to use below compatible strings ?

- const: marvell,octeontx2-wdt
- const: marvell,cn10k-wdt

Also this is the same naming we have been using in other drivers as well.
drivers/net/ethernet/marvell/octeontx2
drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c

drivers/perf/marvell_cn10k_ddr_pmu.c
static const struct of_device_id cn10k_ddr_pmu_of_match[] = {
        { .compatible = "marvell,cn10k-ddr-pmu", },
        { },
};

Thanks,
Sunil.
Krzysztof Kozlowski May 16, 2023, 11:42 a.m. UTC | #6
On 16/05/2023 13:24, Sunil Kovvuri Goutham wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Tuesday, May 16, 2023 3:55 PM
>> To: Sunil Kovvuri Goutham <sgoutham@marvell.com>; Bharat Bhushan
>> <bbhushan2@marvell.com>; wim@linux-watchdog.org; linux@roeck-us.net;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux-
>> watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [EXT] Re: [PATCH 1/2 v7] dt-bindings: watchdog: marvell GTI system
>> watchdog driver
>>
>> On 16/05/2023 12:06, Sunil Kovvuri Goutham wrote:
>>
>>
>>>>>>> Marvell have octeontx2 series of processor which have watchdog timer.
>>>>>>> In 95xx,98xx,96xx are the processors in octeontx2 series of
>>>>>>> processor. So
>>>>>> octeontx2-95xx is on soc, octeontx2-96xx is another and so on.
>>>>>>
>>>>>> No, 95xx is not a processor. Otherwise please point me to exact
>>>>>> product datasheet. Hint: I checked it.
>>>>>
>>>>> Looks like 95xx data sheet is not public, will remove in that case.
>>>>
>>>> We can talk about 96xx. Can you point me to the SoC named exactly like this?
>>>> Hint: I checked it.
>>>
>>> To recap what Bharat mentioned before along with references to individual
>> processors.
>>> OcteonTx2 is a family of processors
>>> https://www.marvell.com/products/data-processing-units.html
>>> Please check for "OCTEON TX2 DPUs"
>>> CN96xx and CN98xx are two silicon variants in this family.
>>> https://www.marvell.com/content/dam/marvell/en/public-collateral/embed
>>> ded-processors/marvell-infrastructure-processors-octeon-tx2-cn92xx-cn9
>>> 6xx-cn98xx-product-brief-2020-02.pdf
>>
>> This is a product brief which further suggests CN96xx is a family (or sub-family).
>>
>> "xx" is pretty often used as family, not as product. Otherwise how one product
>> CN92XX can come with 12-18 cores *in the same time*?
> 
> "xx" here suggests skews, some 92xx may have 18 cores and some may have
> few cores fused out resulting in 12cores.

Is the DTSI for them the same? IOW, 12-core and 18-core SoCs have
exactly the same DTSI with all properties being the same, valid and not
customized by firmware per skew? If we talk about ARM architecture, DTS
expects CPUs there, so I really wonder how do you write one DTS which
has in the same time 12 and 18 enabled CPUs. Remember - the same time
and not changed by firmware.

> 
>>
>>>
>>> Since the HW block is same in all the variants of silicons in this
>>> family, we would like to use a generic string instead of different
>>> compatible string for each one. ie
>>> - const: marvell,octeontx2-wdt
>>> Hope this is okay.
>>
>> 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=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=q3VKxXQKiboRw_F01ggTz
>> HuhwawxR1P9_tMCN2FODU4&m=GGfz5QI8ldHRqao5OsrfuHZQso5LLNBeBxCZr
>> Ai7Zow-
>> qoKl_S1Yw90OWnSZ3FFx&s=kM0VFY1b15BYvp2EUapQjZ6Eb96aZ_yAE76EKCmA
>> aEQ&e=
>>
>>>
>>> Same with CN10K or Octeon10 family of silicons.
>>> https://www.marvell.com/products/data-processing-units.html
>>> Please check for "OCTEON 10"
>>>
>>> CN103xx and CN106xx are two silicons in this family.
>>
>> Are they? "Up to 8" cores, so how this can be one specific silicon? One customer
>> buys CN10300 with 8 cores, second buys exactly the same CN10300 and has 4
>> cores?
>>
>> You are mixing families and specific devices.
> 
> I was making it simple to understand.
> 
> OcteonTx2 and Octeon10 (CN10K) are two generations of Octeon processors from Marvell.

I know. I don't think we talk about the same thing...

> Within each generation there are multiple silicon variants.
> Again for each variant there are multiple skews.
> 
> Since the watchdog hardware block functionality is same across all of above
> generations / variants / families / skews, is it okay to use below compatible strings ?

You got the link which explains it.

We had this discussions for thousands times. Just a few references from
bookmarks:

https://lore.kernel.org/all/20220822181701.GA89665-robh@kernel.org/
https://lore.kernel.org/all/78651e07-6b3e-4243-8e1f-fcd1dfb3ffe1@www.fastmail.com/
https://lore.kernel.org/all/288f56ba9cfad46354203b7698babe91@walle.cc/
https://lore.kernel.org/all/106e443a-e765-51fe-b556-e4e7e2aa771c@linaro.org/

Explain me how is this different. Please do not repeat the same
arguments as everywhere else, because we covered them.

> 
> - const: marvell,octeontx2-wdt
> - const: marvell,cn10k-wdt
> 
> Also this is the same naming we have been using in other drivers as well.
> drivers/net/ethernet/marvell/octeontx2
> drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c

Ah, the argument "others did it" or "in the past we did". If the
approach is buggy, does it mean you should duplicate the same buggy
approach to new bindings?

Best regards,
Krzysztof
Krzysztof Kozlowski May 16, 2023, 11:54 a.m. UTC | #7
On 16/05/2023 13:24, Sunil Kovvuri Goutham wrote:
> 
> Also this is the same naming we have been using in other drivers as well.
> drivers/net/ethernet/marvell/octeontx2
> drivers/net/ethernet/marvell/octeontx2/af/rvu_cn10k.c
> 
> drivers/perf/marvell_cn10k_ddr_pmu.c
> static const struct of_device_id cn10k_ddr_pmu_of_match[] = {
>         { .compatible = "marvell,cn10k-ddr-pmu", },
>         { },

BTW, I don't understand this part. We do not talk about fallback
compatible, so what does it prove? Of course driver will look like that,
but we established it some time ago, didn't we?

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml
new file mode 100644
index 000000000000..72951b10f1f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml
@@ -0,0 +1,80 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/marvell,octeontx2-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Global Timer (GTI) system watchdog
+
+allOf:
+  - $ref: watchdog.yaml#
+
+maintainers:
+  - Bharat Bhushan <bbhushan2@marvell.com>
+
+properties:
+  compatible:
+    oneOf:
+      - const: marvell,octeontx2-wdt
+      - items:
+          - enum:
+              - marvell,octeontx2-95xx-wdt
+              - marvell,octeontx2-96xx-wdt
+              - marvell,octeontx2-98xx-wdt
+          - const: marvell,octeontx2-wdt
+      - const: marvell,cn10k-wdt
+      - items:
+          - enum:
+              - marvell,cn10kx-wdt
+              - marvell,cnf10kx-wdt
+          - const: marvell,cn10k-wdt
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+
+  clock-names:
+    minItems: 1
+
+  marvell,wdt-timer-index:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 63
+    description:
+      An SoC have many timers (up to 64), firmware can reserve one or more timer
+      for some other use case and configures one of the global timer as watchdog
+      timer. Firmware will update this field with the timer number configured
+      as watchdog timer.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        watchdog@802000040000 {
+            compatible = "marvell,octeontx2-wdt";
+            reg = <0x00008020 0x00040000 0x00000000 0x00020000>;
+            interrupts = <GIC_SPI 38 IRQ_TYPE_EDGE_RISING>;
+            clocks = <&sclk>;
+            clock-names = "ref_clk";
+            marvell,wdt-timer-index = <63>;
+        };
+    };
+
+...