Message-ID: <ad8812e6-7dc2-5575-c44d-3f4f62aeb9e9@linaro.org>
Date:   Fri, 3 Feb 2023 09:17:49 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v8 5/9] dt-bindings: qcom-qce: document clocks and
 clock-names as optional
Content-Language: en-US
To:     Vladimir Zapolskiy <vladimir.zapolskiy@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>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <22f191c4-5346-8fe7-690d-9422775bb2d5@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.

Best regards,
Krzysztof