Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <910c152d-5e1a-4667-2f0a-a1524f51958c@linaro.org>
Date:   Thu, 3 Nov 2022 11:56:20 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.1
Subject: Re: [PATCH 05/10] dt-bindings: interconnect: Add sm8350, sc8280xp and
 generic OSM L3 compatibles
To:     Bjorn Andersson <andersson@kernel.org>
Cc:     Bjorn Andersson <quic_bjorande@quicinc.com>,
        Georgi Djakov <djakov@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Sibi Sankar <quic_sibis@quicinc.com>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Mike Tipton <quic_mdtipton@quicinc.com>,
        Johan Hovold <johan+linaro@kernel.org>,
        linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20221028034155.5580-1-quic_bjorande@quicinc.com>
 <20221028034155.5580-6-quic_bjorande@quicinc.com>
 <a364b343-fa19-348c-bc38-e8b44061890b@linaro.org>
 <20221103034410.GB5525@core-thresher1.qualcomm.com>
 <f3882934-9f95-39f1-83e4-6ce9efeb089c@linaro.org>
 <20221103154653.67mgsey57uvdcvx3@builder.lan>
Content-Language: en-US
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20221103154653.67mgsey57uvdcvx3@builder.lan>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/11/2022 11:46, Bjorn Andersson wrote:
> On Thu, Nov 03, 2022 at 08:25:17AM -0400, Krzysztof Kozlowski wrote:
>> On 02/11/2022 23:44, Bjorn Andersson wrote:
>>> On Fri, Oct 28, 2022 at 06:12:29PM -0400, Krzysztof Kozlowski wrote:
>>>> On 27/10/2022 23:41, Bjorn Andersson wrote:
>>>>> Add EPSS L3 compatibles for sm8350 and sc8280xp, but while at it also
>>>>> introduce generic compatible for both qcom,osm-l3 and qcom,epss-l3.
>>>>>
>>>>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>>>>> ---
>>>>>  .../bindings/interconnect/qcom,osm-l3.yaml    | 22 +++++++++++++------
>>>>>  1 file changed, 15 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
>>>>> index bf538c0c5a81..ae0995341a78 100644
>>>>> --- a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
>>>>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
>>>>> @@ -16,13 +16,21 @@ description:
>>>>>  
>>>>>  properties:
>>>>>    compatible:
>>>>> -    enum:
>>>>> -      - qcom,sc7180-osm-l3
>>>>> -      - qcom,sc7280-epss-l3
>>>>> -      - qcom,sc8180x-osm-l3
>>>>> -      - qcom,sdm845-osm-l3
>>>>> -      - qcom,sm8150-osm-l3
>>>>> -      - qcom,sm8250-epss-l3
>>>>> +    oneOf:
>>>>> +      items:
>>>>
>>>> oneOf expects a list, so this should be "    - items"
>>>>
>>>
>>> Ahh, thanks. Must have missed running the dt_binding_check on this one.
>>>
>>>>> +        - enum:
>>>>> +            - qcom,sc7180-osm-l3
>>>>> +            - qcom,sc8180x-osm-l3
>>>>> +            - qcom,sdm845-osm-l3
>>>>> +            - qcom,sm8150-osm-l3
>>>>> +        - const: qcom,osm-l3
>>>>
>>>> The concept is good, but are you sure all SoCs will be compatible with
>>>> generic osm-l3?
>>>
>>> Per the current implementation yes, worst case if one or more of them isn't the
>>> more specific compatible can be used to alter the behavior of that platform.
>>>
>>>> Why not using dedicated compatible of one soc, e.g. the
>>>> oldest here? We already did like that for BWMON, DMA and few others.
>>>>
>>>
>>> Because if we say compatible = "qcom,sc8180x-osm-l3", "qcom,sdm845-osm-l3" and
>>> there is a quirk needed for "qcom,sdm845-osm-l3" we're forced to add a "special
>>> case" every other *-osm-l3 in the driver.
>>>
>>> This way we can have a generic implementation for the qcom,osm-l3 and if we
>>> realize that we need to quirk something for the oldest platform, we can do so
>>> without affecting the others.
>>
>> True. This also means we do not really know which one is the generic
>> implementation :)
>>
> 
> There currently is an implementation without platform specific quirks, I
> call that the generic implementation and suggest that we refer to that
> using "qcom,osm-l3".>
> If we instead were to use sdm845 as the generic compatible, and there
> turns out to be a need for a quirk for this platform, you:
> 
> 1) no longer have a generic implementation, but 4 platform-specific
>    implementations

It's okay because there is no such thing anymore as "generic
implementation". If this happened, your generic compatible is not
specific enough. It does not represent any specific hardware.

Adding generic compatibles just to make driver binding easier, is a bit
in contrast with DT which should focus on hardware description.

> 
> 2) have 3 platforms claiming to be compatible with the quirked (now
>    specialized) implementation, which they clearly aren't anymore

Yes, that's the problem and this is why I mentioned that we do not know
the generic implementation. If we knew that sdm845 is the generic, we
would not expect specific quirks for it.

If you cannot make sdm845 generic because of unknown quirk, then you
just do not know which one is generic implementation and that compatible
is not specific enough... Or it is specific only to current Linux driver.

> Therefor I favor using generic names for generic compatibles.

They make driver development easier but they hide the real match between
hardware and compatible.

Best regards,
Krzysztof