Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.62.198 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.62.198; helo=xsj-pvapexch02.xlnx.xilinx.com;
Envelope-to: robh@kernel.org,
 linux-pci@vger.kernel.org,
 linux-kernel@vger.kernel.org,
 devicetree@vger.kernel.org,
 lorenzo.pieralisi@arm.com,
 bhelgaas@google.com
Message-ID: <eea7db0b-b483-e61c-8f2d-174f2032fd51@xilinx.com>
Date:   Tue, 10 May 2022 09:29:24 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Subject: Re: [PATCH] dt-bindings: PCI: xilinx-cpm: Change reg property order
Content-Language: en-US
To:     Rob Herring <robh@kernel.org>,
        Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
CC:     PCI <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>
References: <20220429130221.32113-1-bharat.kumar.gogada@xilinx.com>
 <CAL_JsqJkHRbXoHdgDYgeF5JhdPgDhjCg=W7YUmCRdBR8xSKz6A@mail.gmail.com>
From:   Michal Simek <michal.simek@xilinx.com>
In-Reply-To: <CAL_JsqJkHRbXoHdgDYgeF5JhdPgDhjCg=W7YUmCRdBR8xSKz6A@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2022 07:29:30.5288
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: a6546202-c8ec-4a27-0602-08da3256d279
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.62.198];Helo=[xsj-pvapexch02.xlnx.xilinx.com]
X-MS-Exchange-CrossTenant-AuthSource: SN1NAM02FT0018.eop-nam02.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR02MB7928
X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 5/4/22 17:43, Rob Herring wrote:
> On Fri, Apr 29, 2022 at 8:02 AM Bharat Kumar Gogada
> <bharat.kumar.gogada@xilinx.com> wrote:
>>
>> Describe cpm reg property before cfg reg property to align with
>> node name.
> 
> The order is an ABI. If breaking it is okay, explain why here.

I didn't push any description for versal to upstream u-boot or linux yet but 
xilinx is using this order for years. DT binding order wasn't aligned to it.

For example: (Xilinx Linux is in sync with this).
https://github.com/Xilinx/u-boot-xlnx/blob/master/arch/arm/dts/versal.dtsi

Driver itself is working with reg-names and order of regs doesn't matter. It 
means changed order doesn't break any functionality.
Right now reg order really matter in binding doc but would be good in these 
examples to record that both ways are fine.
Would it be better to describe that both ways are fine?

   reg-names:
     oneOf:
     - items:
       - const: cfg
       - const: cpm_slcr
     - items:
       - const: cpm_slcr
       - const: cfg


Another small reason is that all hard IPs in Versal are normally placed below 
4GB address range. And there are some others which also have mapping above. This 
is one of that example and we normally aligned with 32bit address.

And the biggest reason is that current node name is pcie@fca10000 which should 
be aligned with the first register base which is before this patch 0x600000000 
but name suggest that the first reg should be cpm_slcr instead of cfg. That's 
why I consider this patch as a fix and the patch should contain fixed tag.

Thanks,
Michal