All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Cc: Lian Minghuan-B31939 <B31939@freescale.com>,
	"Minghuan.Lian@freescale.com" <Minghuan.Lian@freescale.com>,
	"Mingkai.Hu@freescale.com" <Mingkai.Hu@freescale.com>,
	Roy Zang <tie-fei.zang@freescale.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: 答复: [PATCH 2/2] PCI: Layerscape: Add Layerscape PCIe driver
Date: Tue, 09 Sep 2014 13:58:17 +0200	[thread overview]
Message-ID: <10318509.zOTEUpfc9B@wuerfel> (raw)
In-Reply-To: <540F51F1.1060909@freescale.com>

On Tuesday 09 September 2014 19:16:01 Lian Minghuan-B31939 wrote:
> On 2014年09月09日 10:50, Arnd Bergmann wrote:
> > On Tuesday 09 September 2014 18:46:59 Lian Minghuan-B31939 wrote:
> >> On 2014年09月09日 09:56, Arnd Bergmann wrote:
> >>> On Tuesday 09 September 2014 17:25:57 Lian Minghuan-B31939 wrote:
> >>>> [Minghuan] I discussed with my colleague. They worry about performance
> >>>> degradation if using regmap API,
> >>>> because there are some fast device use scfg. We tend to use a simple way
> >>>> to map andread/write scfg directly.
> >>> I see. In this case, I would probably create a separate msi controller
> >>> driver that owns the "fsl,ls1021a-scfg" device, and is referenced
> >>> through the "msi-parent" property in the pcie controller.
> >>>
> >>> You can use of_pci_find_msi_chip_by_node() to get the msi_chip
> >>> instance and then connect that to your pci host. This will also
> >>> take care of the case where you may want to use the main GICv3
> >>> on a future SoC.
> >>
> >> [Minghuan] There is something wrong with LS1021A MSI hardware that it
> >> only supports one interrupt not 32 interrupts.  Now, I do not want to
> >> create a separate msi controller driver just for incorrect hardware.
> >> I may provide complete MSI driver for the new hardware when it is ready.
> >
> > Would you just leave out MSI support for the LS1021A PCIe variant?
> > I guess that's fine because all device drivers should also support
> > legacy interrupts and there is no performance gain in MSI in this
> > case.
>
> [Minghuan] I have added MSI support for LS1021A PCIe just reserved 31 
> interrupts as used.

I don't understand your logic then. If LS1021A has an incorrect MSI
implementation, and you may want to reuse the PCIe driver with a
future chip that either includes a correct MSI implementation, or
with one that uses the GICv3 instead, isn't that even more reason
to split out the MSI support into a separate driver?

That way you can at least separate the normal code path from the
broken one and don't need any special run-time or compile-conditionals
beyond calling of_pci_find_msi_chip_by_node().

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: 答复: [PATCH 2/2] PCI: Layerscape: Add Layerscape PCIe driver
Date: Tue, 09 Sep 2014 13:58:17 +0200	[thread overview]
Message-ID: <10318509.zOTEUpfc9B@wuerfel> (raw)
In-Reply-To: <540F51F1.1060909@freescale.com>

On Tuesday 09 September 2014 19:16:01 Lian Minghuan-B31939 wrote:
> On 2014?09?09? 10:50, Arnd Bergmann wrote:
> > On Tuesday 09 September 2014 18:46:59 Lian Minghuan-B31939 wrote:
> >> On 2014?09?09? 09:56, Arnd Bergmann wrote:
> >>> On Tuesday 09 September 2014 17:25:57 Lian Minghuan-B31939 wrote:
> >>>> [Minghuan] I discussed with my colleague. They worry about performance
> >>>> degradation if using regmap API,
> >>>> because there are some fast device use scfg. We tend to use a simple way
> >>>> to map andread/write scfg directly.
> >>> I see. In this case, I would probably create a separate msi controller
> >>> driver that owns the "fsl,ls1021a-scfg" device, and is referenced
> >>> through the "msi-parent" property in the pcie controller.
> >>>
> >>> You can use of_pci_find_msi_chip_by_node() to get the msi_chip
> >>> instance and then connect that to your pci host. This will also
> >>> take care of the case where you may want to use the main GICv3
> >>> on a future SoC.
> >>
> >> [Minghuan] There is something wrong with LS1021A MSI hardware that it
> >> only supports one interrupt not 32 interrupts.  Now, I do not want to
> >> create a separate msi controller driver just for incorrect hardware.
> >> I may provide complete MSI driver for the new hardware when it is ready.
> >
> > Would you just leave out MSI support for the LS1021A PCIe variant?
> > I guess that's fine because all device drivers should also support
> > legacy interrupts and there is no performance gain in MSI in this
> > case.
>
> [Minghuan] I have added MSI support for LS1021A PCIe just reserved 31 
> interrupts as used.

I don't understand your logic then. If LS1021A has an incorrect MSI
implementation, and you may want to reuse the PCIe driver with a
future chip that either includes a correct MSI implementation, or
with one that uses the GICv3 instead, isn't that even more reason
to split out the MSI support into a separate driver?

That way you can at least separate the normal code path from the
broken one and don't need any special run-time or compile-conditionals
beyond calling of_pci_find_msi_chip_by_node().

	Arnd

  reply	other threads:[~2014-09-09 11:58 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-04 18:45 [PATCH 1/2] PCI: designware: change MSI-related pcie_host_ops Minghuan Lian
2014-09-04 18:45 ` Minghuan Lian
2014-09-04 13:20 ` Bjorn Helgaas
2014-09-04 13:20   ` Bjorn Helgaas
2014-09-05  6:15   ` 答复: " Minghuan.Lian at freescale.com
2014-09-04 18:45 ` [PATCH 2/2] PCI: Layerscape: Add Layerscape PCIe driver Minghuan Lian
2014-09-04 18:45   ` Minghuan Lian
2014-09-04 12:14   ` Arnd Bergmann
2014-09-04 12:14     ` Arnd Bergmann
2014-09-04 13:51     ` Arnd Bergmann
2014-09-04 13:51       ` Arnd Bergmann
2014-09-04 13:57       ` Arnd Bergmann
2014-09-04 13:57         ` Arnd Bergmann
2014-09-05  7:22     ` 答复: " Minghuan.Lian at freescale.com
2014-09-05  8:44       ` Arnd Bergmann
2014-09-05  8:44         ` Arnd Bergmann
2014-09-09 17:25         ` Lian Minghuan-B31939
2014-09-09 17:25           ` Lian Minghuan-B31939
2014-09-09  9:56           ` Arnd Bergmann
2014-09-09  9:56             ` Arnd Bergmann
2014-09-09 18:46             ` Lian Minghuan-B31939
2014-09-09 18:46               ` Lian Minghuan-B31939
2014-09-09 10:50               ` Arnd Bergmann
2014-09-09 10:50                 ` Arnd Bergmann
2014-09-09 19:16                 ` Lian Minghuan-B31939
2014-09-09 19:16                   ` Lian Minghuan-B31939
2014-09-09 11:58                   ` Arnd Bergmann [this message]
2014-09-09 11:58                     ` Arnd Bergmann
2014-09-10 11:29                     ` Lian Minghuan-B31939
2014-09-10 11:29                       ` Lian Minghuan-B31939
2014-09-04 13:24   ` Bjorn Helgaas
2014-09-04 13:24     ` Bjorn Helgaas
2014-09-05  7:24     ` 答复: " Minghuan.Lian at freescale.com
2014-09-04 20:21   ` Fabio Estevam
2014-09-04 20:21     ` Fabio Estevam
2014-09-04 21:12     ` Arnd Bergmann
2014-09-04 21:12       ` Arnd Bergmann
2014-09-05  6:43       ` 答复: " Minghuan.Lian at freescale.com
2014-09-05  7:40     ` Minghuan.Lian at freescale.com

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=10318509.zOTEUpfc9B@wuerfel \
    --to=arnd@arndb.de \
    --cc=B31939@freescale.com \
    --cc=Minghuan.Lian@freescale.com \
    --cc=Mingkai.Hu@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=tie-fei.zang@freescale.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.