linux-kernel - Re: [PATCH] dt-bindings: mmc: fsl-imx-esdhc: allow more compatible combinations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Jan 2023 10:14:05 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Andreas Kemnade <andreas@...nade.info>
Cc:     ulf.hansson@...aro.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, shawnguo@...nel.org,
        s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
        linux-imx@....com, linux-mmc@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: mmc: fsl-imx-esdhc: allow more compatible
 combinations

On 08/01/2023 18:20, Andreas Kemnade wrote:
>>>>
>>>> No, half compatible is the A in such case.
>>>>  
>>> I think that there is some misunderstanding in here. I try once again.
>>>
>>> Define compatible with "X" here:
>>> To me it means:
>>>
>>> device fully works with flags defined in:
>>>
>>> static const struct esdhc_soc_data usdhc_X_data = { ... };
>>>
>>> with usdhc_X_data referenced in
>>>         { .compatible = "X", .data = &usdhc_X_data, },
>>>
>>>
>>> So if there is only "A" matching with above definition of compatibility
>>>   compatible = "A" would sound sane to me.
>>>
>>> And scrutinizing the flags more and not just wanting to achieve error-free
>>> dtbs_check, I think is this in most cases where there is only "A". 
>>>
>>> If there is "A" and "B" which match that compatibility definition, you
>>> say that only compatible = "A", "B" is allowed, but not compatible = "A".
>>> In that case I would have no problem with that.
>>>
>>> But if there is only "A" but no "B" matching the above definition, I would expect
>>> that only compatible = "A" is allowed but *not* compatible = "A", "B".  
>>
>> Sorry, I don't follow. I also do not understand what "matching" means in
>> these terms (binding driver? of_match?) and also I do not know what is
>> the "above definition".
>>
>> Devicetree spec defines the compatibility - so this is the definition.
>> There will be differences when applying it to different cases.
>>
> Ok, lets stop talking about A and B, lets be more specific.
> Hmm, I try to insert the missing bits here:
> 
> I am not convinced anymore that my patch is correct
> - for dtb compatible formality
> - for pure technical reasons
> 
> I am not convinced that your proposal is correct either.
> - for pure technical reasons (for same resan as you state)
> 
> Especially this part I consider faulty:
> +      - items:
> +          - const: fsl,imx6sx-usdhc
> +          - const: fsl,imx6sl-usdhc
> 
> Keyword: ESDHC_FLAG_STATE_LOST_IN_LPMODE (detailed that in
> an earlier mail).

I am not discussing here real compatibility for your hardware, as I
don't know whether 6SX is or is not compatible with 6SL. I am saying
that it either is or is not. Cannot be both at the same time.

Now for the question about 6SX+6SL. Rob responded here detailing when
compatibility of SX and SL is valid. If your hardware fits this case,
then remove the alone SX from enum and add SX+SL list like in this patch.

If your hardware does not fit, so there is no single 6SX client which
can use 6SL compatible and work somehow (e.g. half-speed, reduced
capabilities but still work correctly), then please bring back the DTS
patches. I am not sure if other people who commented on DTS, are
following our discussion here...

Best regards,
Krzysztof