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

From: Krzysztof Kozlowski
Date: Tue May 09 2023 - 09:06:31 EST


On 09/05/2023 11:01, Bharat Bhushan wrote:
>
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>> Sent: Tuesday, May 9, 2023 1:38 PM
>> To: Bharat Bhushan <bbhushan2@xxxxxxxxxxx>; wim@xxxxxxxxxxxxxxxxxx;
>> linux@xxxxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
>> linux-watchdog@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx; Sunil Kovvuri Goutham <sgoutham@xxxxxxxxxxx>
>> 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