Re: [PATCH v2 1/3] dt-bindings: platform: Add bindings for Qcom's EC on IT8987

[Krzysztof Kozlowski]

On 22/12/2024 08:55, Maya Matuszczyk wrote:
> niedz., 22 gru 2024 o 07:40 Krzysztof Kozlowski <krzk@xxxxxxxxxx> napisał(a):
>>
>> On 22/12/2024 07:33, Krzysztof Kozlowski wrote:
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - const: qcom,x1e-it8987-ec
>>>
>>> That's not a SoC, so I don't understand:
>>> 1. referring to the SoC,
>>> 2. Having this alone and as fallback.
>>>
>>> It does not look like you tested the bindings, at least after quick
>>> look. Please run 'make dt_binding_check' (see
>>> Documentation/devicetree/bindings/writing-schema.rst for instructions).
>>> Maybe you need to update your dtschema and yamllint. Don't rely on
>>> distro packages for dtschema and be sure you are using the latest
>>> released dtschema.
>>
>> BTW, for sure Qualcomm did not develop/create it8987, so it cannot be
>> used here. Come with specific compatible for this given, one product:
>> embedded controller on one Lenovo laptop and use it also as filename.
> 
> Under these assumptions:
> 
> - Qualcomm developed the firmware running on the IT8987 in most x1e machines

No one here knows whether most x1e machines have this chip...

> - IT8987 is also used in other machines with a non-compatible firmware
> - The driver name should reflect the assumptions

I don't care about driver here, so you can use it for the driver but
these are not correct assumptions for the bindings.



> 
> I think the name qcom,x1e-it8987-ec is not the worst name for it, as
> "ite,it8987-ec" would imply compatibility with other devices running
> non-compatible firmware,
> and names specifying only the device wouldn't reflect the "firmware is
> based on what qcom did and it's driven the same way" part
> 
> Which other names do you think would fit this better?

I suggested the one in second reply:
lenovo,yoga-slim-whatever-model-it-is-ec

If you have any indication that Qualcomm firmware from a reference board
was used as a reference, it could be used as fallback, but I do not
believe you can have such indication considering downstream source code
does not exist and all other docs are confidential. Therefore lenovo EC
is the first implementation we see and we know anything about.


Best regards,
Krzysztof