Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C
From:   Brad Larson <blarson@amd.com>
To:     <krzysztof.kozlowski@linaro.org>
CC:     <adrian.hunter@intel.com>, <alcooperx@gmail.com>,
        <andy.shevchenko@gmail.com>, <arnd@arndb.de>, <blarson@amd.com>,
        <brendan.higgins@linux.dev>, <briannorris@chromium.org>,
        <brijeshkumar.singh@amd.com>, <broonie@kernel.org>,
        <catalin.marinas@arm.com>, <davidgow@google.com>,
        <devicetree@vger.kernel.org>, <fancer.lancer@gmail.com>,
        <gerg@linux-m68k.org>, <gsomlo@gmail.com>, <krzk@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <lee.jones@linaro.org>,
        <lee@kernel.org>, <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-mmc@vger.kernel.org>,
        <linux-spi@vger.kernel.org>, <p.yadav@ti.com>,
        <p.zabel@pengutronix.de>, <piotrs@cadence.com>,
        <rdunlap@infradead.org>, <robh+dt@kernel.org>,
        <samuel@sholland.org>, <skhan@linuxfoundation.org>,
        <suravee.suthikulpanit@amd.com>, <thomas.lendacky@amd.com>,
        <tonyhuang.sunplus@gmail.com>, <ulf.hansson@linaro.org>,
        <vaishnav.a@ti.com>, <will@kernel.org>,
        <yamada.masahiro@socionext.com>
Subject: Re: [PATCH v10 02/15] dt-bindings: mmc: cdns: Add AMD Pensando Elba SoC
Date:   Mon, 6 Mar 2023 18:11:03 -0800
Message-ID: <20230307021103.25299-1-blarson@amd.com>
In-Reply-To: <ebbc1c7a-2a96-409c-0e92-ae0f57ae5335@linaro.org>
References: <ebbc1c7a-2a96-409c-0e92-ae0f57ae5335@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2023 02:11:23.5505
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: d3862969-b35e-4d13-6730-08db1eb14018
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource: BN8NAM11FT055.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5905
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 06/03/2023 8:28, Krzysztof Kozlowski wrote: 
> On 06/03/2023 05:07, Brad Larson wrote:
>> AMD Pensando Elba ARM 64-bit SoC is integrated with this IP and
>> explicitly controls byte-lane enables.
>> 
>> Signed-off-by: Brad Larson <blarson@amd.com>
>> ---
>> 
>> v10 changes:
>> - Move reset-names property definition next to existing resets prop
>> - Move allOf to the bottom and set resets/reset-names required only for pensando
>> - Fix reg maxItems for existing, must be 1
>> 
>> v9 changes:
>> - Add reset-names and resets properties
>> - Add if/then on property amd,pensando-elba-sd4hc to set reg property
>>   values for minItems and maxItems
>> 
>> ---
>>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 33 ++++++++++++++++---
>>  1 file changed, 29 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
>> index adacd0535c14..0c4d6d4b2b58 100644
>> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
>> @@ -9,19 +9,18 @@ title: Cadence SD/SDIO/eMMC Host Controller (SD4HC)
>>  maintainers:
>>    - Masahiro Yamada <yamada.masahiro@socionext.com>
>>  
>> -allOf:
>> -  - $ref: mmc-controller.yaml
>> -
>>  properties:
>>    compatible:
>>      items:
>>        - enum:
>> +          - amd,pensando-elba-sd4hc
>>            - microchip,mpfs-sd4hc
>>            - socionext,uniphier-sd4hc
>>        - const: cdns,sd4hc
>>  
>>    reg:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    interrupts:
>>      maxItems: 1
>> @@ -30,8 +29,13 @@ properties:
>>      maxItems: 1
>>  
>>    resets:
>> +    description: physical line number to hardware reset the mmc
>
> This part seems to be not needed anymore. Resets field was already added.

Yes, see below.

>
>>      maxItems: 1
>>  
>> +  reset-names:
>> +    items:
>> +      - const: hw
>
> Why did you add reset-names for one item? There is no v8 of this patch,
> so I cannot find previous discussion about it.

I found resets property was added recently when I rebased.

cb7f090171393 (Kunihiko Hayashi    2023-02-13 13:52:33 +0900  32)   resets:
cb7f090171393 (Kunihiko Hayashi    2023-02-13 13:52:33 +0900  33)     maxItems: 1

I've deleted reset-names and dropped description for resets.

>
>>    # PHY DLL input delays:
>>    # They are used to delay the data valid window, and align the window to
>>    # sampling clock. The delay starts from 5ns (for delay parameter equal to 0)
>> @@ -120,6 +124,27 @@ required:
>>    - interrupts
>>    - clocks
>>  
>> +allOf:
>> +  - $ref: mmc-controller.yaml
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: amd,pensando-elba-sd4hc
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 2
>
> Hm, we missed to mention it before, but what is the second reg for? It's
> not obvious from the binding so probably you need to describe it instead
> minItems:
>   items:
>     - description: foo
>     - description: bar
>

The second reg is byte lane enable for writes.  The following passed dtbs_check
after getting: hint: "minItems" is only needed if less than the "items" list length

    then:
      properties:
        reg:
          items:
            - description: Host controller registers
            - description: Elba byte-lane enable register for writes
      required:
        - resets
    else:
      properties:
        resets: false
        reg:
          maxItems: 1

>> +      required:
>> +        - reset-names
>> +        - resets
>> +    else:
>> +      properties:
>> +        reset-names: false
>> +        resets: false
>> +        reg:
>> +          maxItems: 1
>> +
>>  unevaluatedProperties: false

Regards,
Brad