Re: [PATCH 09/14] arm64: dts: qcom: sm6115: Add UFS nodes

[Iskren Chernev <iskren.chernev@gmail.com>]

On 9/5/22 13:02, Krzysztof Kozlowski wrote:
> On 03/09/2022 19:04, Iskren Chernev wrote:
>>
>>
>> On 9/1/22 19:13, Krzysztof Kozlowski wrote:
>>> On 01/09/2022 10:24, Iskren Chernev wrote:
>>>> The SM6115 comes with UFS support, so add the related UFS and UFS PHY
>>>> nodes.
>>>>
>>>> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/sm6115.dtsi | 70 ++++++++++++++++++++++++++++
>>>>  1 file changed, 70 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>> index cde963c56ac9..491fffff8aa1 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
>>>> @@ -620,6 +620,76 @@ opp-202000000 {
>>>>  			};
>>>>  		};
>>>>
>>>> +		ufs_mem_hc: ufshc@4804000 {
>>>> +			compatible = "qcom,sm6115-ufshc", "qcom,ufshc",
>>>> +				     "jedec,ufs-2.0";
>>>> +			reg = <0x4804000 0x3000>, <0x4810000 0x8000>;
>>>> +			reg-names = "std", "ice";
>>>
>>> I could imagine that testing DTS against existing bindings might miss a
>>> lot, because we have still a lot of errors. But at least I would expect
>>> you test your DTS against your own bindings, which you submit here (and
>>> previously).
>>>
>>> You just wrote that ice is not allowed.
>>
>> OK, I'm an idiot. I didn't run the bindings checks, not against existing
>> bindings or my bindings or whatever. It's my fault.
>>
>> Ice should be allowed, I fixed the bindings in v2.
>>
>> For the record, running dtbs_checks is a PITA, not only because of the
>> thousands of warnings in unrelated code, but because it takes forever.
>
> You can limit it per schema and/or limit it per target, which would
> speed up things. Of course it depends on computer you have, but I don't
> find it slow on my laptop and I run them a lot...

For one file it's great. For all files (i.e make dtbs_check) it takes more than
30mins on my 4c/8t intel laptop. About limiting by schema... not really
useful for me (except if I write the schema, but then I use it in one dtb, so
it's easier to limit by dtb).

>>
>> Maybe the docs should be updated with instructions on how to run it on a single
>> (or a small subset) of DTBs. I had to comment out a lot of Makefile lines to
>> focus it on mine. It would really help if the binding check works more like
>> a compiler, not some magic spell hidden in a bunch of Makefiles.
>
> crosc.... make -j8 DT_SCHEMA_FILES=exynos-srom.yaml CHECK_DTBS=y
> qcom/sm8450-hdk.dtb

Aaah, very nice! Thank you!

>>
>> I'll list all remaining issues with description/explanation in v2. The fact
>> that some bindings break on all DTBs present doesn't help either.
>
> We're working on this... It's quite a lot of effort, especially when new
> warnings are being added. :)

I understand. Is there an up-for-grabs list, or any schema fixes are welcome?

> Best regards,
> Krzysztof