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

[Krzysztof Kozlowski]

On 16/05/2023 13:24, Sunil Kovvuri Goutham wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>> Sent: Tuesday, May 16, 2023 3:55 PM
>> To: Sunil Kovvuri Goutham <sgoutham@xxxxxxxxxxx>; Bharat Bhushan
>> <bbhushan2@xxxxxxxxxxx>; wim@xxxxxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx;
>> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; linux-
>> watchdog@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx
>> 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@xxxxxxxxxx/
https://lore.kernel.org/all/78651e07-6b3e-4243-8e1f-fcd1dfb3ffe1@xxxxxxxxxxxxxxxx/
https://lore.kernel.org/all/288f56ba9cfad46354203b7698babe91@xxxxxxxx/
https://lore.kernel.org/all/106e443a-e765-51fe-b556-e4e7e2aa771c@xxxxxxxxxx/

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