linux-kernel - Re: [PATCH 1/2] dt-bindings: reset: Updated binding for Versal-NET reset driver

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]

Message-ID: <ee81e955-32be-66ea-377b-263ee60a2632@linaro.org>
Date:   Mon, 17 Jul 2023 22:47:53 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Conor Dooley <conor@...nel.org>,
        Piyush Mehta <piyush.mehta@....com>
Cc:     robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, p.zabel@...gutronix.de, michal.simek@....com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        git@....com
Subject: Re: [PATCH 1/2] dt-bindings: reset: Updated binding for Versal-NET
 reset driver

On 17/07/2023 20:40, Conor Dooley wrote:
> On Mon, Jul 17, 2023 at 04:53:47PM +0530, Piyush Mehta wrote:
>> Added documentation and Versal-NET reset indices to describe about
>> Versal-Net reset driver bindings.
>>
>> In Versal-NET all reset indices includes Class, SubClass, Type, Index
>> information whereas class refers to clock, reset, power etc.,
>> Underlying firmware in Versal have such classification and expects
>> the ID to be this way.
>> [13:0] - Index bits
>> [19:14] - Type bits
>> [25:20] - SubClass bits
>> [31:26] - Class bits.
> 
> Riight.. I'm not sure that describing these as "indices" is really all
> that valid, given only 13:0 are actually the index.
> I'd be inclined to say that the type/class/subclass stuff should not be
> part of the dt-bindings, and instead looked up inside the driver
> depending on the index.
> 
> Hopefully Rob or Krzysztof can comment further.

This confuses me as well. I don't understand why do you need it in the
bindings. Nothing uses these values, so storing them as bindings seems
pointless.
> 
> Thanks,
> Conor.
> 
>>
>> Signed-off-by: Piyush Mehta <piyush.mehta@....com>
>> ---
>>  .../bindings/reset/xlnx,zynqmp-reset.yaml     |   4 +
>>  .../reset/xlnx-versal-net-resets.h            | 127 ++++++++++++++++++
>>  2 files changed, 131 insertions(+)
>>  create mode 100644 include/dt-bindings/reset/xlnx-versal-net-resets.h
>>
>> diff --git a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.yaml b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.yaml
>> index 0d50f6a54af3..b996fc1d4e53 100644
>> --- a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.yaml
>> +++ b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.yaml
>> @@ -27,11 +27,15 @@ description: |
>>    For list of all valid reset indices for Versal
>>    <dt-bindings/reset/xlnx-versal-resets.h>
>>  
>> +  For list of all valid reset indices for Versal-NET
>> +  <dt-bindings/reset/xlnx-versal-net-resets.h>
>> +
>>  properties:
>>    compatible:
>>      enum:
>>        - xlnx,zynqmp-reset
>>        - xlnx,versal-reset
>> +      - xlnx,versal-net-reset
>>  
>>    "#reset-cells":
>>      const: 1
>> diff --git a/include/dt-bindings/reset/xlnx-versal-net-resets.h b/include/dt-bindings/reset/xlnx-versal-net-resets.h
>> new file mode 100644
>> index 000000000000..b3e7d5e4c33e
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/xlnx-versal-net-resets.h
>> @@ -0,0 +1,127 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + *  Copyright (C) 2023 Advanced Micro Devices, Inc. All Rights Reserved.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_VERSAL_NET_RESETS_H
>> +#define _DT_BINDINGS_VERSAL_NET_RESETS_H
>> +
>> +#define PM_RST_PMC_POR                          (0xc30c001U)

IDs start from 0 and are incremented by 1.

Best regards,
Krzysztof