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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6c1779aa-c90c-2160-f8b9-497fb8c32dc5@ieee.org>
Date:   Mon, 26 Jul 2021 10:59:03 -0500
From:   Alex Elder <elder@...e.org>
To:     Rob Herring <robh@...nel.org>, Alex Elder <elder@...aro.org>
Cc:     bjorn.andersson@...aro.org, agross@...nel.org, davem@...emloft.net,
        kuba@...nel.org, evgreen@...omium.org, cpratapa@...eaurora.org,
        subashab@...eaurora.org, elder@...nel.org,
        linux-arm-msm@...r.kernel.org, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 1/3] dt-bindings: net: qcom,ipa: make imem
 interconnect optional

On 7/23/21 3:52 PM, Rob Herring wrote:
> On Mon, Jul 19, 2021 at 04:24:54PM -0500, Alex Elder wrote:
>> On some newer SoCs, the interconnect between IPA and SoC internal
>> memory (imem) is not used.  Reflect this in the binding by moving
>> the definition of the "imem" interconnect to the end and defining
>> minItems to be 2 for both the interconnects and interconnect-names
>> properties.
>>
>> Signed-off-by: Alex Elder <elder@...aro.org>
>> ---
>>   .../devicetree/bindings/net/qcom,ipa.yaml      | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
>> index ed88ba4b94df5..4853ab7017bd9 100644
>> --- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml
>> +++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
>> @@ -87,16 +87,18 @@ properties:
>>         - const: ipa-setup-ready
>>   
>>     interconnects:
>> +    minItems: 2
>>       items:
>> -      - description: Interconnect path between IPA and main memory
>> -      - description: Interconnect path between IPA and internal memory
>> -      - description: Interconnect path between IPA and the AP subsystem
>> +      - description: Path leading to system memory
>> +      - description: Path between the AP and IPA config space
>> +      - description: Path leading to internal memory
>>   
>>     interconnect-names:
>> +    minItems: 2
>>       items:
>>         - const: memory
>> -      - const: imem
>>         - const: config
>> +      - const: imem
> 
> What about existing users? This will generate warnings. Doing this for
> the 2nd item would avoid the need for .dts updates:
> 
> - enum: [ imem, config ]

If I understand correctly, the effect of this would be that
the second item can either be "imem" or "config", and the third
(if present) could only be "imem"?

And you're saying that otherwise, existing users (the only
one it applies to at the moment is "sdm845.dtsi") would
produce warnings, because the interconnects are listed
in an order different from what the binding specifies.

Is that correct?

If so, what you propose suggests "imem" could be listed twice.
It doesn't make sense, and maybe it's precluded in other ways
so that's OK.  But I'd be happy to update "sdm845.dtsi" to
address your concern.  (Maybe that's something you would rather
avoid?)

Also, I need to make a separate update to "sm8350.dtsi" because
that was defined before I understood what I do now about the 
interconnects.  It uses the wrong names, and should combine
its first two interconnects into just one.

					-Alex

> 
> Rob
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ