All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Conor Dooley <conor@kernel.org>,
	linux-pci@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	Daire McNamara <daire.mcnamara@microchip.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v5 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
Date: Sat, 2 Nov 2024 20:54:11 +0900	[thread overview]
Message-ID: <20241102115411.GE2260768@rocinante> (raw)
In-Reply-To: <20241101195129.GA1318063@bhelgaas>

Hello,

> > The PCI host controller on PolarFire SoC has multiple "instances", each
> > with their own bridge and ctrl address spaces. The original binding has
> > an "apb" register region, and it is expected to be set to the base
> > address of the host controllers register space. Defines in the driver
> > were used to compute the addresses of the bridge and ctrl address ranges
> > corresponding to instance1. Some customers want to use instance0 however
> > and that requires changing the defines in the driver, which is clearly
> > not a portable solution.
> 
> The subject mentions "instance 1 or 2".
> 
> This paragraph implies adding support for "instance0" ("customers want
> to use instance0").
> 
> The DT patch suggests that we're adding support for "instance2"
> ("customers want to use instance2").
> 
> Both patches suggest that the existing support is for "instance 1".
> 
> Maybe what's being added is "instance 2", and this commit log should
> s/instance0/instance 2/ ?  And probably s/instance1/instance 1/ so the
> style is consistent?
> 
> Is this a "pick one or the other but not both" situation, or does this
> device support two independent PCIe controllers?
> 
> I first thought this driver supported a single PCIe controller, and
> you were adding support for a second independent controller.
> 
> But the fact that you say "the [singular] host controller on
> PolarFire", and you're not changing mc_host_probe() to call
> pci_host_common_probe() more than once makes me think there is only a
> single PCIe controller, and for some reason you can choose to operate
> it using either register set 1 or register set 2.

Conor, let me know if we need to clarify the commit log per Bjorn's
questions.  If so, then I will do it directly on the branch.

	Krzysztof

WARNING: multiple messages have this Message-ID (diff)
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Conor Dooley <conor@kernel.org>,
	linux-pci@vger.kernel.org,
	Conor Dooley <conor.dooley@microchip.com>,
	Daire McNamara <daire.mcnamara@microchip.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v5 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
Date: Sat, 2 Nov 2024 20:54:11 +0900	[thread overview]
Message-ID: <20241102115411.GE2260768@rocinante> (raw)
In-Reply-To: <20241101195129.GA1318063@bhelgaas>

Hello,

> > The PCI host controller on PolarFire SoC has multiple "instances", each
> > with their own bridge and ctrl address spaces. The original binding has
> > an "apb" register region, and it is expected to be set to the base
> > address of the host controllers register space. Defines in the driver
> > were used to compute the addresses of the bridge and ctrl address ranges
> > corresponding to instance1. Some customers want to use instance0 however
> > and that requires changing the defines in the driver, which is clearly
> > not a portable solution.
> 
> The subject mentions "instance 1 or 2".
> 
> This paragraph implies adding support for "instance0" ("customers want
> to use instance0").
> 
> The DT patch suggests that we're adding support for "instance2"
> ("customers want to use instance2").
> 
> Both patches suggest that the existing support is for "instance 1".
> 
> Maybe what's being added is "instance 2", and this commit log should
> s/instance0/instance 2/ ?  And probably s/instance1/instance 1/ so the
> style is consistent?
> 
> Is this a "pick one or the other but not both" situation, or does this
> device support two independent PCIe controllers?
> 
> I first thought this driver supported a single PCIe controller, and
> you were adding support for a second independent controller.
> 
> But the fact that you say "the [singular] host controller on
> PolarFire", and you're not changing mc_host_probe() to call
> pci_host_common_probe() more than once makes me think there is only a
> single PCIe controller, and for some reason you can choose to operate
> it using either register set 1 or register set 2.

Conor, let me know if we need to clarify the commit log per Bjorn's
questions.  If so, then I will do it directly on the branch.

	Krzysztof

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2024-11-02 11:54 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-14  8:08 [PATCH v5 0/2] PCI: microchip: support using either instance 1 or 2 Conor Dooley
2024-08-14  8:08 ` Conor Dooley
2024-08-14  8:08 ` [PATCH v5 1/2] dt-bindings: PCI: microchip,pcie-host: fix reg properties Conor Dooley
2024-08-14  8:08   ` Conor Dooley
2024-08-14  8:08 ` [PATCH v5 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2 Conor Dooley
2024-08-14  8:08   ` Conor Dooley
2024-11-01 19:51   ` Bjorn Helgaas
2024-11-01 19:51     ` Bjorn Helgaas
2024-11-02 11:54     ` Krzysztof Wilczyński [this message]
2024-11-02 11:54       ` Krzysztof Wilczyński
2024-11-04 11:18     ` Conor Dooley
2024-11-04 11:18       ` Conor Dooley
2024-11-05 17:18       ` Bjorn Helgaas
2024-11-05 17:18         ` Bjorn Helgaas
2024-11-06 16:26         ` Conor Dooley
2024-11-06 16:26           ` Conor Dooley
2024-10-24  9:38 ` [PATCH v5 0/2] PCI: microchip: " Conor Dooley
2024-10-24  9:38   ` Conor Dooley
2024-10-24 18:46   ` Bjorn Helgaas
2024-10-24 18:46     ` Bjorn Helgaas
2024-11-02 11:51 ` Krzysztof Wilczyński
2024-11-02 11:51   ` Krzysztof Wilczyński

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=20241102115411.GE2260768@rocinante \
    --to=kw@linux.com \
    --cc=bhelgaas@google.com \
    --cc=conor+dt@kernel.org \
    --cc=conor.dooley@microchip.com \
    --cc=conor@kernel.org \
    --cc=daire.mcnamara@microchip.com \
    --cc=devicetree@vger.kernel.org \
    --cc=helgaas@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=lpieralisi@kernel.org \
    --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 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.