Re: [PATCH] arm64: dts: qcom: sm8650: add description of CCI controllers

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

On 4/10/24 18:26, neil.armstrong@linaro.org wrote:
> On 10/04/2024 17:19, Vladimir Zapolskiy wrote:
>> Hi Neil,
>>
>> On 4/10/24 16:50, neil.armstrong@linaro.org wrote:
>>> On 10/04/2024 15:11, Vladimir Zapolskiy wrote:
>>>> On 4/10/24 10:52, Neil Armstrong wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/04/2024 09:49, Vladimir Zapolskiy wrote:
>>>>>> Qualcomm SM8650 SoC has three CCI controllers with two I2C busses
>>>>>> connected to each of them.
>>>>>>
>>>>>> The CCI controllers on SM8650 are compatible with the ones found on
>>>>>> many other older generations of Qualcomm SoCs.
>>>>>>
>>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>>> ---
>>>>>> The change is based and depends on a patch series from Jagadeesh Kona:
>>>>>>
>>>>>>       https://lore.kernel.org/linux-arm-msm/20240321092529.13362-1-quic_jkona@quicinc.com/
>>>>>>
>>>>>> It might be an option to add this change right to the series,
>>>>>> since it anyway requires a respin.
>>>>>>
>>>>>> A new compatible value "qcom,sm8650-cci" is NOT added to
>>>>>> Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml , because
>>>>>> the controller IP description and selection is covered by a generic
>>>>>> compatible value "qcom,msm8996-cci".
>>>>>
>>>>> You'll still need to add qcom,sm8650-cci to the "CCI v2" list in qcom,i2c-cci.yaml,
>>>>> otherwise the DTBS check fail, even if the fallback is already present.
>>>>
>>>> I do recognize the problem related to a build time warning, my motivation was
>>>> to follow the rationale described in commit 3e383dce513f
>>>> ("Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible"").
>>>>
>>>> For a similar sc8280xp-cci case it was asked by Konrad to drop a new
>>>> compatible, I kindly ask the reviewers and maintainers to stick to one
>>>> of the two contradicting asks.
>>>
>>> This is totally different, this commit added a new compatible that is used in the driver,
>>> while here, you use a per-soc compatible that is (for now), only used in DT and uses
>>
>> I'm confused, please elaborate what do you mean above by "this commit" and "here".
>> Could you please be more specific to avoid any possible disambiguation?
> 
> "this" refer to "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible".
> 
>> If you refer to the driver drivers/i2c/busses/i2c-qcom-cci.c, then there is no
>> difference between sc8280xp-cci and sm8650-cci. What is the total difference,
>> which you found?
> 
> If there's no difference between sc8280xp-cci and sm8650-cci, then the policy says
> you need to _not_ add a new compatible in the driver, which is what you did here.
> 
>>
>>> the generic "qcom,msm8996-cci" as a fallback because it is considered as beeing 99%
>>> compatible and no software change is needed.
>>>
>>
>> I have no objections to revert a "Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible""
>> commit and to update the change for sm8650-cci accordingly, but as I've
>> already said it would be good to have and follow one common approach for both
>> cases, since I based my change on the maintainer's decision from the past.
> 
> The "new" policy is to use a fallback of an already defined compatible if no driver change
> is needed, this is the case for the last year so far.
> And updating the yaml bindings for the new per-soc compatible is also a year-old
> policy, upstreaming of SM8550, SM8650 and X1E80100 have been done following this policy

I'm sorry, I'm still failing to understand it, it's trivial to check that there is no
"sc8280xp-cci" in Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml description.

Despite my multiple asks I did not get an answer from anybody, if commit 3e383dce513f
("Revert "dt-bindings: i2c: qcom-cci: Document sc8280xp compatible"") is wrong and
shall be reverted or not.

Since my point of discussion is all about the commit 3e383dce513f, because sm8650-cci
change is based on it, I hope that my original understanding that commit 3e383dce513f
shall be reverted, I'll send the change shorty.

> in order to :
> 1) reduce useless driver changes
> 2) have a fully verifiable DT against bindings, so we can ensure the DT is 100% valid against the bindings
> 

-- 
Best wishes,
Vladimir