From: Michal Simek <michal.simek@xilinx.com>
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>
Subject: Re: [PATCH] dt-bindings: PCI: xilinx-cpm: Change reg property order
Date: Tue, 10 May 2022 09:29:24 +0200 [thread overview]
Message-ID: <eea7db0b-b483-e61c-8f2d-174f2032fd51@xilinx.com> (raw)
In-Reply-To: <CAL_JsqJkHRbXoHdgDYgeF5JhdPgDhjCg=W7YUmCRdBR8xSKz6A@mail.gmail.com>
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
next prev parent reply other threads:[~2022-05-10 7:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-29 13:02 [PATCH] dt-bindings: PCI: xilinx-cpm: Change reg property order Bharat Kumar Gogada
2022-04-29 13:27 ` Michal Simek
2022-05-04 15:43 ` Rob Herring
2022-05-10 7:29 ` Michal Simek [this message]
2022-05-10 16:00 ` Rob Herring
2022-05-10 17:18 ` Michal Simek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eea7db0b-b483-e61c-8f2d-174f2032fd51@xilinx.com \
--to=michal.simek@xilinx.com \
--cc=bharat.kumar.gogada@xilinx.com \
--cc=bhelgaas@google.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).