All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: "Musham, Sai Krishna" <sai.krishna.musham@amd.com>
Cc: "bhelgaas@google.com" <bhelgaas@google.com>,
	"lpieralisi@kernel.org" <lpieralisi@kernel.org>,
	"kw@linux.com" <kw@linux.com>,
	"mani@kernel.org" <mani@kernel.org>,
	"krzk+dt@kernel.org" <krzk+dt@kernel.org>,
	"conor+dt@kernel.org" <conor+dt@kernel.org>,
	"cassel@kernel.org" <cassel@kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Simek, Michal" <michal.simek@amd.com>,
	"Gogada, Bharat Kumar" <bharat.kumar.gogada@amd.com>,
	"Havalige, Thippeswamy" <thippeswamy.havalige@amd.com>,
	"Sanwal, Pranav" <Pranav.sanwal@amd.com>
Subject: Re: [PATCH v7 1/2] dt-bindings: PCI: amd-mdb: Add CPM6 support
Date: Tue, 6 Oct 2026 10:05:28 -0500	[thread overview]
Message-ID: <20261006150528.GB2194299-robh@kernel.org> (raw)
In-Reply-To: <IA4PR12MB9810F0345A3E7A7D3F28ADCDCD952@IA4PR12MB9810.namprd12.prod.outlook.com>

On Tue, Oct 06, 2026 at 10:01:31AM +0000, Musham, Sai Krishna wrote:
> AMD General
> 
> Hi Rob,
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Friday, September 18, 2026 12:55 AM
> > To: Musham, Sai Krishna <sai.krishna.musham@amd.com>
> > Cc: bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com;
> > mani@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > cassel@kernel.org; linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>;
> > Gogada, Bharat Kumar <bharat.kumar.gogada@amd.com>; Havalige,
> > Thippeswamy <thippeswamy.havalige@amd.com>; Sanwal, Pranav
> > <Pranav.sanwal@amd.com>
> > Subject: Re: [PATCH v7 1/2] dt-bindings: PCI: amd-mdb: Add CPM6 support
> >
> > On Sun, Aug 23, 2026 at 09:37:14AM +0530, Sai Krishna Musham wrote:
> > > The AMD CPM6 PCIe controller is based on the Synopsys DesignWare PCIe
> > > IP and integrates two PCIe host controller instances. Add the
> > > "amd,versal2-cpm6-host" and "amd,versal2-cpm6-host1" compatible strings
> > > to describe the HOST0 and HOST1 controller instances.
> >
> > That is clear from the diff. What's not clear in this patch, and I
> > probably already asked, what is the difference between host0 and host1?
> > Usually instances are not different compatibles...
> >
> 
> Thanks for reviewing, there is no functional difference between HOST0 and
> HOST1. They are two instances of the same CPM6 PCIe controller and support
> the same feature set.
> 
> The underlying hardware and driver behavior are identical between the
> two instances. The only differences are instance-specific resources,
> such as register regions and controller-specific interrupt status bits.

The only difference I see in the driver is:

+/* MERGED input bits for the MISC_EVENT sources this driver handles. */
+#define AMD_CPM6_MERGED_MISC_EVENT_HOST0	14
+#define AMD_CPM6_MERGED_MISC_EVENT_HOST1	17

If that is the only difference, I would make this a property instead.

But there's another problem here:

+		/*
+		 * CPM6 moves the per-controller MISC_EVENT registers
+		 * into a separate "intr" region. The SLCR block, which holds
+		 * the shared MERGED/PS_MISC aggregators, is shared by both CPM6
+		 * host controllers, so map it without requesting exclusive
+		 * ownership; otherwise the second controller fails to probe.
+		 */

This implies the 'slcr' region is the same address for both controllers? 
That's a problem because the DT should not contain the same address in 
multiple nodes. There should be a single 'slcr' node.

> I understand your point that instances are typically not represented by
> different compatible strings. Given that the driver needs to distinguish
> between the two controller instances for these instance-specific
> resources, could you please suggest the preferred DT representation for
> this case?

The answer depends on what the variation is, so the patch needs to 
explain that.

Rob

  reply	other threads:[~2026-10-06 15:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-23  4:07 [PATCH v7 0/2] pci: AMD: Add Versal2 CPM6 PCIe host controller support Sai Krishna Musham
2026-08-23  4:07 ` [PATCH v7 1/2] dt-bindings: PCI: amd-mdb: Add CPM6 support Sai Krishna Musham
2026-08-23  4:11   ` sashiko-bot
2026-09-17 19:25   ` Rob Herring
2026-10-06 10:01     ` Musham, Sai Krishna
2026-10-06 15:05       ` Rob Herring [this message]
2026-08-23  4:07 ` [PATCH v7 2/2] PCI: amd-mdb: Add CPM6 host controller support Sai Krishna Musham
2026-08-23  4:21   ` sashiko-bot

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=20261006150528.GB2194299-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=Pranav.sanwal@amd.com \
    --cc=bharat.kumar.gogada@amd.com \
    --cc=bhelgaas@google.com \
    --cc=cassel@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=michal.simek@amd.com \
    --cc=sai.krishna.musham@amd.com \
    --cc=thippeswamy.havalige@amd.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.