Re: [PATCH] dt-bindings: usb: add yaml file for maxim,max3421

[Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>]

On 10/9/25 22:34, Krzysztof Kozlowski wrote:
> On 09/10/2025 03:15, Rodrigo Gobbi wrote:
>> Convert maxim,max3421.txt to yaml format with a few extra properties like
> 
> 
> Here and in subject, please do not use yaml at all. Look at other
> commits, this is supposed to be:
> 
> dt-bindings: usb: maxim,max3421: convert to DT schema
> 
> (and all other things like "file for" are redundant")
> 
>> maxim,vbus-en-pin, maxim,gpx-pin, reset pin and supplies. Also add a
> 
> Why new properties? You must explain not only the difference but WHY you
> are doing this.
> 
> WHY is the most important question/answer.

The reason was that the device (the IC) supports that. Is it
enough to justify? I mean, is a plausible answer in this case? If yes,
I agree that my commit msg was not right since I didn`t mention that.

> 
>> maxim,max3421e compatible with a fallback, since the actually PN is with
>> the 'e' suffix.
> 
> We don't add PNs usually, unless there is a need. So again, why?
> 

The PN of this is Maxim3421e, Maxim3421 without `e` doesn`t exists as far as I`ve
searched it. If it exists, it`s a different thing. In this case, I would expect that
the compatible string should be something that "matches" the device, but in this
case, the compatible string is without the 'e'. In that way, I was suggesting in this patch to
allow a more precisely compatible string to no break anyone using the original one. But if 
it was a bad idea here, I can drop that for sure.

>> +  spi-max-frequency:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> No, drop $ref. Do you see any binding like that? No, there is none.

I`ve a previous patch recently at [1], that added a "similar" thing of frequency:

+  sampling-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 2500000
+    maximum: 20000000
+    description:
+      Default sampling frequency of the ADC in Hz.

In that case, $ref and description were added. Why that case is different from this one here?
[1] https://lore.kernel.org/all/20250522204130.21604-1-rodrigo.gobbi.7@gmail.com/

> 
>> +
>> +  maxim,vbus-en-pin:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      One of eight GPOUT pins to control external VBUS power and the polarity
>> +      of the active level. It's an array of GPIO number and the active level of it.
>> +    minItems: 2
>> +    maxItems: 2
>> +
>> +  maxim,gpx-pin:
> 
> I don't understand. There is no need for this property. Drop.

During my other reviews of new bindings, my final premise was that we should add every "capability" of
a device (the IC) regardless of the driver support. In this case, the maxim,gpx-pin, is an example of that,
the device supports that despite the driver support. I`m wondering here why we cannot add that here.

Tks and best regards.
Rodrigo.