Message-ID: <43c7cf3f-f463-90da-23f9-f6b76d9f729f@linaro.org>
Date:   Fri, 3 Feb 2023 10:26:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.0.2
Subject: Re: [PATCH v8 5/9] dt-bindings: qcom-qce: document clocks and
 clock-names as optional
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Thara Gopinath <thara.gopinath@gmail.com>,
        Bhupesh Sharma <bhupesh.sharma@linaro.org>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Andy Gross <agross@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-crypto@vger.kernel.org,
        Neil Armstrong <neil.armstrong@linaro.org>
References: <20230202135036.2635376-1-vladimir.zapolskiy@linaro.org>
 <20230202135036.2635376-6-vladimir.zapolskiy@linaro.org>
 <32c23da1-45f0-82a4-362d-ae5c06660e20@linaro.org>
 <22f191c4-5346-8fe7-690d-9422775bb2d5@linaro.org>
 <ad8812e6-7dc2-5575-c44d-3f4f62aeb9e9@linaro.org>
From:   Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
In-Reply-To: <ad8812e6-7dc2-5575-c44d-3f4f62aeb9e9@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Krzysztof,

On 2/3/23 10:17, Krzysztof Kozlowski wrote:
> On 02/02/2023 23:27, Vladimir Zapolskiy wrote:
>> Hi Krzysztof,
>>
>> On 2/2/23 15:53, Krzysztof Kozlowski wrote:
>>> On 02/02/2023 14:50, Vladimir Zapolskiy wrote:
>>>> From: Neil Armstrong <neil.armstrong@linaro.org>
>>>>
>>>> On certain Snapdragon processors, the crypto engine clocks are enabled by
>>>> default by security firmware.
>>>
>>> Then probably we should not require them only on these variants.
>>
>> the rationale is clear, but here comes a minor problem, older platforms
>> require clocks, when newer ones do not. When a generic SoC-specific compatible
>> is introduced, let say "qcom,ipq4019-qce", it itself requires the clocks,
>> but then newer platforms can not be based on this particular compatible,
>> otherwise they will require clocks and this comes as invalid.
>>
>> How to resolve it properly, shall there be another generic SoC-specific
>> compatible without clocks and NOT based on that "qcom,ipq4019-qce" compatible?
>>
>> By the way, QCE on SM8150 also shall not need the clocks.
> 
> Assuming you have:
> 1. ipq4019 requiring clocks
> 2. msm8996 compatible with ipq4019, requiring clocks
> 3. ipq6018 compatible with ipq4019, not requiring clocks
> 
> allOf:
>    - if:
>        properties:
>          compatible:
>            enum:
>               - ipq4019-qce
>      then:
>        required:
>          - clocks
> 
>    - if:
>        properties:
>          compatible:
>            contains:
>              enum:
>                 - msm8996-qce
>      then:
>        required:
>          - clocks
> 
> That's not pretty.
> 
> Another solution is to make non-clock-requiring variants as their own
> family:
> 
> 1. msm8996-qce, ipq4019-qce
> 2. sm8550-qce, sm8150-qce
> 
> and then in the driver you need two entries - ipq4019 and sm8150.
> 
> I like the latter, because for clock-requiring variants your driver
> should actually get them and require. For non-clock-requiring variants,
> you just skip the clocks (do not fail). Therefore you need different
> driver data for these two families.

many thanks for the detailed explanation, the first of two solutions will
be even more clumsy and convoluted, since there should be lists split into
two baskets.

Thus I will go with the second variant and add two family compatibles.

--
Best wishes,
Vladimir