public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale•com>
To: Lian Minghuan-b31939 <b31939@freescale•com>
Cc: Bjorn Helgaas <bhelgaas@google•com>,
	Minghuan Lian <Minghuan.Lian@freescale•com>,
	linuxppc-dev@lists•ozlabs.org,
	Zang Roy-R61911 <r61911@freescale•com>,
	linux-pci@vger•kernel.org
Subject: Re: [02/12,v3] pci: fsl: add structure fsl_pci
Date: Tue, 7 Jan 2014 02:33:43 -0600	[thread overview]
Message-ID: <1389083623.11795.162.camel@snotra.buserror.net> (raw)
In-Reply-To: <52CA48C0.2020307@freescale.com>

On Mon, 2014-01-06 at 14:10 +0800, Lian Minghuan-b31939 wrote:
> On 01/04/2014 06:19 AM, Scott Wood wrote:
> > I don't like the extent to which this duplicates (not moves) PPC's struct
> > pci_controller.  Also this leaves some fields like "indirect_type"
> > unexplained (PPC_INDIRECT_TYPE_xxx is only in the PPC header).
> >
> > Does the arch-independent part of the driver really need all this?  Given
> > how closely this tracks the PPC code, how would this work on ARM?
> [Minghuan] I added the duplicate fields because PPC's struct 
> pci_controller need them.

I think a better approach would be to create a cleanly architected
arch-independent driver.  Share what you reasonably can with the current
fsl_pci.c, but not to the extent of propagating PPCisms that don't match
up with what we ultimately want to see in generic code, or copying
things that ought to be controller-independent infrastructure into
controller-specific code.

See these threads:
http://www.spinics.net/lists/linux-pci/msg25769.html
https://lkml.org/lkml/2013/5/4/103

> The following is for ARM, I will submit them after verification:
[snip]
> +static int fsl_pcie_register(struct fsl_pcie *pcie)
> +{
> +    pcie->controller = fsl_hw_pcie.nr_controllers;
> +    fsl_hw_pcie.nr_controllers = 1;
> +    fsl_hw_pcie.private_data = (void **)&pcie;

I believe this should be:
	fsl_hw_pcie.private_data = pcie;

> +    pci_common_init(&fsl_hw_pcie);
> +    pci_assign_unassigned_resources();
> +#ifdef CONFIG_PCI_DOMAINS
> +    fsl_hw_pcie.domain++;
> +#endif

What serializes that non-atomic increment?

-Scott

  reply	other threads:[~2014-01-07  8:33 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-23 10:41 [PATCH 01/12][v3] pci: fsl: derive the common PCI driver to drivers/pci/host Minghuan Lian
2013-10-23 10:41 ` [PATCH 02/12][v3] pci: fsl: add structure fsl_pci Minghuan Lian
2013-10-24  4:11   ` Kumar Gala
2013-10-24  4:15     ` Zang Roy-R61911
2013-10-25  5:58     ` Lian Minghuan-b31939
2013-10-28 18:22       ` Scott Wood
2014-01-03 22:19   ` [02/12,v3] " Scott Wood
2014-01-06  6:10     ` Lian Minghuan-b31939
2014-01-07  8:33       ` Scott Wood [this message]
2014-01-22 23:38     ` Roy Zang
2013-10-23 10:41 ` [PATCH 03/12][v3] pci: fsl: add PCI indirect access support Minghuan Lian
2014-01-03 22:33   ` [03/12,v3] " Scott Wood
2014-01-06  5:36     ` Lian Minghuan-b31939
2014-01-07  7:13       ` Scott Wood
2013-10-23 10:41 ` [PATCH 04/12][v3] pci: fsl: add early " Minghuan Lian
2013-10-23 10:41 ` [PATCH 05/12][v3] pci: fsl: port PCI ATMU related code Minghuan Lian
2013-10-23 10:41 ` [PATCH 06/12][v3] pci: fsl: port PCI controller setup code Minghuan Lian
2013-10-23 10:41 ` [PATCH 07/12][v3] pci: fsl: port PCI platform driver Minghuan Lian
2013-10-23 10:41 ` [PATCH 08/12][v3] pci: fsl: add PowerPC PCI driver Minghuan Lian
2013-10-23 10:41 ` [PATCH 09/12][v3] pci: fsl: update PCI PM driver Minghuan Lian
2013-10-23 10:41 ` [PATCH 10/12][v3] pci: fsl: support function fsl_pci_assign_primary Minghuan Lian
2013-10-23 10:41 ` [PATCH 11/12][v3] pci: fsl: update PCI EDAC driver Minghuan Lian
2014-01-03 22:16   ` [11/12,v3] " Scott Wood
2014-01-06  3:57     ` Lian Minghuan-b31939
2013-10-23 10:41 ` [PATCH 12/12][v3] pci: fsl: fix function check_pci_ctl_endpt_part Minghuan Lian
2013-11-25 23:01 ` [PATCH 01/12][v3] pci: fsl: derive the common PCI driver to drivers/pci/host Bjorn Helgaas
2014-01-03 22:37   ` Scott Wood

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=1389083623.11795.162.camel@snotra.buserror.net \
    --to=scottwood@freescale$(echo .)com \
    --cc=Minghuan.Lian@freescale$(echo .)com \
    --cc=b31939@freescale$(echo .)com \
    --cc=bhelgaas@google$(echo .)com \
    --cc=linux-pci@vger$(echo .)kernel.org \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=r61911@freescale$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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