* Re: [PATCH 1/3] mm: return NUMA_NO_NODE in local_memory_node if zonelists are not setup
From: Andrew Morton @ 2014-02-21 22:42 UTC (permalink / raw)
To: Nishanth Aravamudan
Cc: Michal Hocko, linux-mm, Mel Gorman, David Rientjes,
Christoph Lameter, linuxppc-dev, Joonsoo Kim, Anton Blanchard
In-Reply-To: <20140220182847.GA24745@linux.vnet.ibm.com>
On Thu, 20 Feb 2014 10:28:47 -0800 Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
> On 20.02.2014 [10:05:39 -0600], Christoph Lameter wrote:
> > On Wed, 19 Feb 2014, Nishanth Aravamudan wrote:
> >
> > > We can call local_memory_node() before the zonelists are setup. In that
> > > case, first_zones_zonelist() will not set zone and the reference to
> > > zone->node will Oops. Catch this case, and, since we presumably running
> > > very early, just return that any node will do.
> >
> > Really? Isnt there some way to avoid this call if zonelists are not setup
> > yet?
>
> How do I best determine if zonelists aren't setup yet?
>
> The call-path in question (after my series is applied) is:
>
> arch/powerpc/kernel/setup_64.c::setup_arch ->
> arch/powerpc/mm/numa.c::do_init_bootmem() ->
> cpu_numa_callback() ->
> numa_setup_cpu() ->
> map_cpu_to_node() ->
> update_numa_cpu_node() ->
> set_cpu_numa_mem()
>
> and setup_arch() is called before build_all_zonelists(NULL, NULL) in
> start_kernel(). This seemed like the most reasonable path, as it's used
> on hotplug as well.
>
But the call to local_memory_node() you added was in start_secondary(),
which isn't in that trace.
I do agree that calling local_memory_node() too early then trying to
fudge around the consequences seems rather wrong.
^ permalink raw reply
* Re: [PATCH 7/7] powerpc: Added PCI MSI support using the HSTA module
From: Arnd Bergmann @ 2014-02-21 21:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alistair Popple, linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1393015286.6771.110.camel@pasglop>
On Friday 21 February 2014, Benjamin Herrenschmidt wrote:
> On Fri, 2014-02-21 at 15:33 +0100, Arnd Bergmann wrote:
>
> > > @@ -242,8 +264,10 @@
> > > ranges = <0x02000000 0x00000000 0x80000000 0x00000110 0x80000000 0x0 0x80000000
> > > 0x01000000 0x0 0x0 0x00000140 0x0 0x0 0x00010000>;
> > >
> > > - /* Inbound starting at 0 to memsize filled in by zImage */
> > > - dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x0>;
> > > + /* Inbound starting at 0x0 to 0x40000000000. In order to use MSI
> > > + * PCI devices must be able to write to the HSTA module.
> > > + */
> > > + dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x400 0x0>;
>
> Should we (provided it's possible in HW) create two ranges instead ? One
> covering RAM and one covering MSIs ? To avoid stray DMAs whacking random
> HW registers in the chip ...
>
> > > /* This drives busses 0 to 0xf */
> > > bus-range = <0x0 0xf>;
> >
> > Ah, I first only saw the line you are removing and was about
> > to suggest what you do anyway. Great!
> >
> > > diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
> > > index 54ec1d5..7cc3acc 100644
> > > --- a/arch/powerpc/sysdev/ppc4xx_pci.c
> > > +++ b/arch/powerpc/sysdev/ppc4xx_pci.c
> > > @@ -176,8 +176,12 @@ static int __init ppc4xx_parse_dma_ranges(struct pci_controller *hose,
> > > return -ENXIO;
> > > }
> > >
> > > - /* Check that we are fully contained within 32 bits space */
> > > - if (res->end > 0xffffffff) {
> > > + /* Check that we are fully contained within 32 bits space if we are not
> > > + * running on a 460sx or 476fpe which have 64 bit bus addresses.
> > > + */
> > > + if (res->end > 0xffffffff &&
> > > + !(of_device_is_compatible(hose->dn, "ibm,plb-pciex-460sx")
> > > + || of_device_is_compatible(hose->dn, "ibm,plb-pciex-476fpe"))) {
> > > printk(KERN_ERR "%s: dma-ranges outside of 32 bits space\n",
> > > hose->dn->full_name);
> > > return -ENXIO;
> >
> > A more general question for BenH: Apparently this PCI implementation is
> > getting reused on arm64 for APM X-Gene. Do you see any value in trying to
> > share host controller drivers like this one across powerpc and arm64?
>
> I would start duplicating, and see how much common code remains... Then
> eventually merge.
Ok.
> > It's possible we are going to see the same situation with fsl_pci in the
> > future, if arm and powerpc qoriq chips use the same peripherals. My
> > plan for arm64 right now is to make PCI work without any code in arch/,
> > just using new helper functions in drivers/pci and sticking the host
> > drivers into drivers/pci/host as we started doing for arm32, but it
> > can require significant work to make those drivers compatible with
> > the powerpc pci-common.c.
>
> powerpc pci-common.c is shrinking :-) At least the address remapping is
> all in the core now, we could move more over I suppose...
Ah, good. We're currently trying to work out a generic way to parse
the DT and ioremap the I/O windows. That could probably be shared
and while I hope what we need on arm64 is compatible with what you
need on powerpc, I may always miss something. I'll make sure to add
you to the discussions.
Some parts are easier because we assume that we always scan the
entire PCI bus ourselves and don't do PCI_PROBE_DEVTREE.
Other parts are harder because for the generic case we actually
want to support loading and unloading host bridge drivers,
as well as supporting any combination of host bridges in the
same system without platform specific code.
Arnd
^ permalink raw reply
* Re: Question about EHCI on P4080
From: Scott Wood @ 2014-02-21 21:08 UTC (permalink / raw)
To: Ruchika; +Cc: linuxppc-dev
In-Reply-To: <53064FC9.8070908@servergy.com>
On Thu, 2014-02-20 at 12:56 -0600, Ruchika wrote:
> linuxppc,
> Is there another mailing list which is more appropriate ?
> If so I'd be happy to get a recommendation.
If you're asking about U-Boot support, the list is u-boot@lists.denx.de
Be sure to mention what version of U-Boot you're using, and that you're
using a recent U-Boot. Also, don't top post or post the same thing
twice.
If you're using U-Boot from the Freescale SDK rather than from upstream,
then instead get support from support@freescale.com or
https://community.freescale.com/ It might be a good idea to go that
route regardless, given that this sounds like it may be a hardware
compatibility issue.
I don't think there is a separate host controller or driver for USB 1.1
(or lower speed USB 2.0) devices on p4080 (though this is common on
PCs).
-Scott
>
> Thank you
> Regards
> ruchika
>
>
> On 2/19/2014 11:55 PM, Ruchika wrote:
> > Hi,
> > I've been trying to understand why the uboot code is unable to work with
> > USB1.1 devices.
> > On a USB analyzer:
> > I notice on an analyzer that there are no SOF or IN tokens seen on the
> > bus at all. The only after the very first setup packet is sent and ACK'd.
> >
> > The board has a hub chip on it connected to the USB controller via a
> > Ulpi interface.
> >
> > Uboot debug indicates that the transfer times out and was still "Active"
> >
> > The kernel code indicates that a companion driver handles 1.1 devices.
> > Does this mean there are 2 HCD's, one for HS and one for FS/LS in the
> > kernel ?
> > Thank you
> >
> >
> > ruchika
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
^ permalink raw reply
* Re: [PATCH 7/7] powerpc: Added PCI MSI support using the HSTA module
From: Benjamin Herrenschmidt @ 2014-02-21 20:41 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Alistair Popple, linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1574137.lzDhNaVqIe@wuerfel>
On Fri, 2014-02-21 at 15:33 +0100, Arnd Bergmann wrote:
> > @@ -242,8 +264,10 @@
> > ranges = <0x02000000 0x00000000 0x80000000 0x00000110 0x80000000 0x0 0x80000000
> > 0x01000000 0x0 0x0 0x00000140 0x0 0x0 0x00010000>;
> >
> > - /* Inbound starting at 0 to memsize filled in by zImage */
> > - dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x0>;
> > + /* Inbound starting at 0x0 to 0x40000000000. In order to use MSI
> > + * PCI devices must be able to write to the HSTA module.
> > + */
> > + dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x400 0x0>;
Should we (provided it's possible in HW) create two ranges instead ? One
covering RAM and one covering MSIs ? To avoid stray DMAs whacking random
HW registers in the chip ...
> > /* This drives busses 0 to 0xf */
> > bus-range = <0x0 0xf>;
>
> Ah, I first only saw the line you are removing and was about
> to suggest what you do anyway. Great!
>
> > diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
> > index 54ec1d5..7cc3acc 100644
> > --- a/arch/powerpc/sysdev/ppc4xx_pci.c
> > +++ b/arch/powerpc/sysdev/ppc4xx_pci.c
> > @@ -176,8 +176,12 @@ static int __init ppc4xx_parse_dma_ranges(struct pci_controller *hose,
> > return -ENXIO;
> > }
> >
> > - /* Check that we are fully contained within 32 bits space */
> > - if (res->end > 0xffffffff) {
> > + /* Check that we are fully contained within 32 bits space if we are not
> > + * running on a 460sx or 476fpe which have 64 bit bus addresses.
> > + */
> > + if (res->end > 0xffffffff &&
> > + !(of_device_is_compatible(hose->dn, "ibm,plb-pciex-460sx")
> > + || of_device_is_compatible(hose->dn, "ibm,plb-pciex-476fpe"))) {
> > printk(KERN_ERR "%s: dma-ranges outside of 32 bits space\n",
> > hose->dn->full_name);
> > return -ENXIO;
>
> A more general question for BenH: Apparently this PCI implementation is
> getting reused on arm64 for APM X-Gene. Do you see any value in trying to
> share host controller drivers like this one across powerpc and arm64?
I would start duplicating, and see how much common code remains... Then
eventually merge.
> It's possible we are going to see the same situation with fsl_pci in the
> future, if arm and powerpc qoriq chips use the same peripherals. My
> plan for arm64 right now is to make PCI work without any code in arch/,
> just using new helper functions in drivers/pci and sticking the host
> drivers into drivers/pci/host as we started doing for arm32, but it
> can require significant work to make those drivers compatible with
> the powerpc pci-common.c.
powerpc pci-common.c is shrinking :-) At least the address remapping is
all in the core now, we could move more over I suppose...
Cheers,
Ben.
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/5] powerpc/powernv: Make PHB diag-data output short
From: Benjamin Herrenschmidt @ 2014-02-21 20:05 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1392983613-10090-5-git-send-email-shangw@linux.vnet.ibm.com>
On Fri, 2014-02-21 at 19:53 +0800, Gavin Shan wrote:
> According to Ben's suggestion, the patch makes the PHB diag-data
> dump looks a bit short by printing multiple values in one line
> and outputing "-" for zero fields.
>
> After the patch applied, the PHB diag-data dump looks like:
Actually, I wouldn't do that "-" thing, I would leave zeros as
zeros but I would remove lines that have all zeros.
Additionally, we might want to consider what if we can get rid
of more fields for INF, or maybe even not dump them by default
and just count them (should we have counters in sysfs ?)
One thing I'm tempted to do is turn the full logs into actual
error logs (sent to FSP) and only display a "analyzed" version
in the kernel, something that decodes the PEST for example
and indicates if it's an DMA or MMIO error, the address, etc...
Cheers,
Ben.
> PHB3 PHB#3 Diag-data (Version: 1)
>
> brdgCtl: 00000002
> UtlSts: - - -
> RootSts: 0000000f 00400000 b0830008 00100147 00002000
> RootErrSts: - - -
> RootErrLog: - - - -
> RootErrLog1: - - -
> nFir: - 0030006e00000000 -
> PhbSts: 0000001c00000000 -
> Lem: 0000000000100000 42498e327f502eae -
> PhbErr: - - - -
> OutErr: - - - -
> InAErr: 8000000000000000 8000000000000000 0402030000000000 -
> InBErr: - - - -
> PE[ 8] A/B: 8480002b00000000 8000000000000000
>
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci.c | 238 ++++++++++++++++++++--------------
> 1 file changed, 143 insertions(+), 95 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 67b2254..a5f236a 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -124,67 +124,103 @@ static void pnv_teardown_msi_irqs(struct pci_dev *pdev)
> }
> #endif /* CONFIG_PCI_MSI */
>
> +static char *pnv_pci_diag_field(char *buf, int fmt, u64 val64)
> +{
> + u32 val32 = (u32)val64;
> +
> + memset(buf, 0, 24);
> + switch (fmt) {
> + case 8:
> + if (val32)
> + sprintf(buf, "%08x", val32);
> + else
> + sprintf(buf, "%s", "-");
> + break;
> + case 16:
> + if (val64)
> + sprintf(buf, "%016llx", val64);
> + else
> + sprintf(buf, "%s", "-");
> + break;
> + default:
> + sprintf(buf, "%s", "-");
> + }
> +
> + return buf;
> +}
> +
> static void pnv_pci_dump_p7ioc_diag_data(struct pci_controller *hose,
> struct OpalIoPhbErrorCommon *common)
> {
> struct OpalIoP7IOCPhbErrorData *data;
> + char buf[120];
> int i;
>
> data = (struct OpalIoP7IOCPhbErrorData *)common;
> pr_info("P7IOC PHB#%d Diag-data (Version: %d)\n\n",
> hose->global_number, common->version);
>
> - pr_info(" brdgCtl: %08x\n", data->brdgCtl);
> -
> - pr_info(" portStatusReg: %08x\n", data->portStatusReg);
> - pr_info(" rootCmplxStatus: %08x\n", data->rootCmplxStatus);
> - pr_info(" busAgentStatus: %08x\n", data->busAgentStatus);
> -
> - pr_info(" deviceStatus: %08x\n", data->deviceStatus);
> - pr_info(" slotStatus: %08x\n", data->slotStatus);
> - pr_info(" linkStatus: %08x\n", data->linkStatus);
> - pr_info(" devCmdStatus: %08x\n", data->devCmdStatus);
> - pr_info(" devSecStatus: %08x\n", data->devSecStatus);
> -
> - pr_info(" rootErrorStatus: %08x\n", data->rootErrorStatus);
> - pr_info(" uncorrErrorStatus: %08x\n", data->uncorrErrorStatus);
> - pr_info(" corrErrorStatus: %08x\n", data->corrErrorStatus);
> - pr_info(" tlpHdr1: %08x\n", data->tlpHdr1);
> - pr_info(" tlpHdr2: %08x\n", data->tlpHdr2);
> - pr_info(" tlpHdr3: %08x\n", data->tlpHdr3);
> - pr_info(" tlpHdr4: %08x\n", data->tlpHdr4);
> - pr_info(" sourceId: %08x\n", data->sourceId);
> - pr_info(" errorClass: %016llx\n", data->errorClass);
> - pr_info(" correlator: %016llx\n", data->correlator);
> - pr_info(" p7iocPlssr: %016llx\n", data->p7iocPlssr);
> - pr_info(" p7iocCsr: %016llx\n", data->p7iocCsr);
> - pr_info(" lemFir: %016llx\n", data->lemFir);
> - pr_info(" lemErrorMask: %016llx\n", data->lemErrorMask);
> - pr_info(" lemWOF: %016llx\n", data->lemWOF);
> - pr_info(" phbErrorStatus: %016llx\n", data->phbErrorStatus);
> - pr_info(" phbFirstErrorStatus: %016llx\n", data->phbFirstErrorStatus);
> - pr_info(" phbErrorLog0: %016llx\n", data->phbErrorLog0);
> - pr_info(" phbErrorLog1: %016llx\n", data->phbErrorLog1);
> - pr_info(" mmioErrorStatus: %016llx\n", data->mmioErrorStatus);
> - pr_info(" mmioFirstErrorStatus: %016llx\n", data->mmioFirstErrorStatus);
> - pr_info(" mmioErrorLog0: %016llx\n", data->mmioErrorLog0);
> - pr_info(" mmioErrorLog1: %016llx\n", data->mmioErrorLog1);
> - pr_info(" dma0ErrorStatus: %016llx\n", data->dma0ErrorStatus);
> - pr_info(" dma0FirstErrorStatus: %016llx\n", data->dma0FirstErrorStatus);
> - pr_info(" dma0ErrorLog0: %016llx\n", data->dma0ErrorLog0);
> - pr_info(" dma0ErrorLog1: %016llx\n", data->dma0ErrorLog1);
> - pr_info(" dma1ErrorStatus: %016llx\n", data->dma1ErrorStatus);
> - pr_info(" dma1FirstErrorStatus: %016llx\n", data->dma1FirstErrorStatus);
> - pr_info(" dma1ErrorLog0: %016llx\n", data->dma1ErrorLog0);
> - pr_info(" dma1ErrorLog1: %016llx\n", data->dma1ErrorLog1);
> + pr_info(" brdgCtl: %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->brdgCtl));
> + pr_info(" UtlSts: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->portStatusReg),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->rootCmplxStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->busAgentStatus));
> + pr_info(" RootSts: %s %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->deviceStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->slotStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->linkStatus),
> + pnv_pci_diag_field(&buf[3 * 24], 8, data->devCmdStatus),
> + pnv_pci_diag_field(&buf[4 * 24], 8, data->devSecStatus));
> + pr_info(" RootErrSts: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->rootErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->uncorrErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->corrErrorStatus));
> + pr_info(" RootErrLog: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->tlpHdr1),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->tlpHdr2),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->tlpHdr3),
> + pnv_pci_diag_field(&buf[3 * 24], 8, data->tlpHdr4));
> + pr_info(" RootErrLog1: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->sourceId),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->errorClass),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->correlator));
> + pr_info(" PhbSts: %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->p7iocPlssr),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->p7iocCsr));
> + pr_info(" Lem: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->lemFir),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->lemErrorMask),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->lemWOF));
> + pr_info(" PhbErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->phbErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->phbFirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->phbErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->phbErrorLog1));
> + pr_info(" OutErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->mmioErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->mmioFirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->mmioErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->mmioErrorLog1));
> + pr_info(" InAErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->dma0ErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->dma0FirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->dma0ErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->dma0ErrorLog1));
> + pr_info(" InBErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->dma1ErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->dma1FirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->dma1ErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->dma1ErrorLog1));
>
> for (i = 0; i < OPAL_P7IOC_NUM_PEST_REGS; i++) {
> if ((data->pestA[i] >> 63) == 0 &&
> (data->pestB[i] >> 63) == 0)
> continue;
>
> - pr_info(" PE[%3d] PESTA: %016llx\n", i, data->pestA[i]);
> - pr_info(" PESTB: %016llx\n", data->pestB[i]);
> + pr_info(" PE[%3d] A/B: %s %s\n",
> + i, pnv_pci_diag_field(&buf[0], 16, data->pestA[i]),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->pestB[i]));
> }
> }
>
> @@ -192,67 +228,79 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose,
> struct OpalIoPhbErrorCommon *common)
> {
> struct OpalIoPhb3ErrorData *data;
> - int i;
> + char buf[120];
> + int i = 0;
>
> + memset(buf, 0, 120);
> data = (struct OpalIoPhb3ErrorData*)common;
> pr_info("PHB3 PHB#%d Diag-data (Version: %d)\n\n",
> hose->global_number, common->version);
>
> - pr_info(" brdgCtl: %08x\n", data->brdgCtl);
> -
> - pr_info(" portStatusReg: %08x\n", data->portStatusReg);
> - pr_info(" rootCmplxStatus: %08x\n", data->rootCmplxStatus);
> - pr_info(" busAgentStatus: %08x\n", data->busAgentStatus);
> -
> - pr_info(" deviceStatus: %08x\n", data->deviceStatus);
> - pr_info(" slotStatus: %08x\n", data->slotStatus);
> - pr_info(" linkStatus: %08x\n", data->linkStatus);
> - pr_info(" devCmdStatus: %08x\n", data->devCmdStatus);
> - pr_info(" devSecStatus: %08x\n", data->devSecStatus);
> -
> - pr_info(" rootErrorStatus: %08x\n", data->rootErrorStatus);
> - pr_info(" uncorrErrorStatus: %08x\n", data->uncorrErrorStatus);
> - pr_info(" corrErrorStatus: %08x\n", data->corrErrorStatus);
> - pr_info(" tlpHdr1: %08x\n", data->tlpHdr1);
> - pr_info(" tlpHdr2: %08x\n", data->tlpHdr2);
> - pr_info(" tlpHdr3: %08x\n", data->tlpHdr3);
> - pr_info(" tlpHdr4: %08x\n", data->tlpHdr4);
> - pr_info(" sourceId: %08x\n", data->sourceId);
> - pr_info(" errorClass: %016llx\n", data->errorClass);
> - pr_info(" correlator: %016llx\n", data->correlator);
> -
> - pr_info(" nFir: %016llx\n", data->nFir);
> - pr_info(" nFirMask: %016llx\n", data->nFirMask);
> - pr_info(" nFirWOF: %016llx\n", data->nFirWOF);
> - pr_info(" PhbPlssr: %016llx\n", data->phbPlssr);
> - pr_info(" PhbCsr: %016llx\n", data->phbCsr);
> - pr_info(" lemFir: %016llx\n", data->lemFir);
> - pr_info(" lemErrorMask: %016llx\n", data->lemErrorMask);
> - pr_info(" lemWOF: %016llx\n", data->lemWOF);
> - pr_info(" phbErrorStatus: %016llx\n", data->phbErrorStatus);
> - pr_info(" phbFirstErrorStatus: %016llx\n", data->phbFirstErrorStatus);
> - pr_info(" phbErrorLog0: %016llx\n", data->phbErrorLog0);
> - pr_info(" phbErrorLog1: %016llx\n", data->phbErrorLog1);
> - pr_info(" mmioErrorStatus: %016llx\n", data->mmioErrorStatus);
> - pr_info(" mmioFirstErrorStatus: %016llx\n", data->mmioFirstErrorStatus);
> - pr_info(" mmioErrorLog0: %016llx\n", data->mmioErrorLog0);
> - pr_info(" mmioErrorLog1: %016llx\n", data->mmioErrorLog1);
> - pr_info(" dma0ErrorStatus: %016llx\n", data->dma0ErrorStatus);
> - pr_info(" dma0FirstErrorStatus: %016llx\n", data->dma0FirstErrorStatus);
> - pr_info(" dma0ErrorLog0: %016llx\n", data->dma0ErrorLog0);
> - pr_info(" dma0ErrorLog1: %016llx\n", data->dma0ErrorLog1);
> - pr_info(" dma1ErrorStatus: %016llx\n", data->dma1ErrorStatus);
> - pr_info(" dma1FirstErrorStatus: %016llx\n", data->dma1FirstErrorStatus);
> - pr_info(" dma1ErrorLog0: %016llx\n", data->dma1ErrorLog0);
> - pr_info(" dma1ErrorLog1: %016llx\n", data->dma1ErrorLog1);
> + pr_info(" brdgCtl: %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->brdgCtl));
> + pr_info(" UtlSts: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->portStatusReg),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->rootCmplxStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->busAgentStatus));
> + pr_info(" RootSts: %s %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->deviceStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->slotStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->linkStatus),
> + pnv_pci_diag_field(&buf[3 * 24], 8, data->devCmdStatus),
> + pnv_pci_diag_field(&buf[4 * 24], 8, data->devSecStatus));
> + pr_info(" RootErrSts: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->rootErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->uncorrErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->corrErrorStatus));
> + pr_info(" RootErrLog: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->tlpHdr1),
> + pnv_pci_diag_field(&buf[1 * 24], 8, data->tlpHdr2),
> + pnv_pci_diag_field(&buf[2 * 24], 8, data->tlpHdr3),
> + pnv_pci_diag_field(&buf[3 * 24], 8, data->tlpHdr4));
> + pr_info(" RootErrLog1: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 8, data->sourceId),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->errorClass),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->correlator));
> + pr_info(" nFir: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->nFir),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->nFirMask),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->nFirWOF));
> + pr_info(" PhbSts: %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->phbPlssr),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->phbCsr));
> + pr_info(" Lem: %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->lemFir),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->lemErrorMask),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->lemWOF));
> + pr_info(" PhbErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->phbErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->phbFirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->phbErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->phbErrorLog1));
> + pr_info(" OutErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->mmioErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->mmioFirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->mmioErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->mmioErrorLog1));
> + pr_info(" InAErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->dma0ErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->dma0FirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->dma0ErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->dma0ErrorLog1));
> + pr_info(" InBErr: %s %s %s %s\n",
> + pnv_pci_diag_field(&buf[0], 16, data->dma1ErrorStatus),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->dma1FirstErrorStatus),
> + pnv_pci_diag_field(&buf[2 * 24], 16, data->dma1ErrorLog0),
> + pnv_pci_diag_field(&buf[3 * 24], 16, data->dma1ErrorLog1));
>
> for (i = 0; i < OPAL_PHB3_NUM_PEST_REGS; i++) {
> if ((data->pestA[i] >> 63) == 0 &&
> (data->pestB[i] >> 63) == 0)
> continue;
>
> - pr_info(" PE[%3d] PESTA: %016llx\n", i, data->pestA[i]);
> - pr_info(" PESTB: %016llx\n", data->pestB[i]);
> + pr_info(" PE[%3d] A/B: %s %s\n",
> + i, pnv_pci_diag_field(&buf[0], 16, data->pestA[i]),
> + pnv_pci_diag_field(&buf[1 * 24], 16, data->pestB[i]));
> }
> }
>
^ permalink raw reply
* Re: [PATCH 4/5] powerpc/powernv: Cache PHB diag-data
From: Benjamin Herrenschmidt @ 2014-02-21 20:01 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1392983613-10090-4-git-send-email-shangw@linux.vnet.ibm.com>
On Fri, 2014-02-21 at 19:53 +0800, Gavin Shan wrote:
> EEH core tries to recover from fenced PHB or frozen PE. Unfortunately,
> the log isn't consistent with the site because enabling IO path for
> the frozen PE before collecting log would ruin the site. The patch
> solves the problem to cache the PHB diag-data in advance with the
> help of additional flag PNV_PHB_FLAG_DIAG to pnv_phb::flags.
Ok, so correct me if I'm wrong, but you are
- Collecting the diag data in get_state, as a sort of side effect
(what happens if get_state is called multiple times ?)
- Dumping it later on
Any reason why we can't instead dump it immediately ? Also do we have a
clean trigger for when we detect an error condition ? It can either be
the result of an interrupt or a driver called get_state following an
ffffffff. Are both path eventually going into the same function to
handle a "new" error condition ? That's where I would both collect and
dump the EEH state..
Cheers,
Ben.
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/eeh-ioda.c | 65 ++++++++++++++++++-----------
> arch/powerpc/platforms/powernv/pci.c | 21 ++++++----
> arch/powerpc/platforms/powernv/pci.h | 1 +
> 3 files changed, 55 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
> index 04b4710..3ed8d22 100644
> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
> @@ -114,6 +114,27 @@ DEFINE_SIMPLE_ATTRIBUTE(ioda_eeh_inbB_dbgfs_ops, ioda_eeh_inbB_dbgfs_get,
> ioda_eeh_inbB_dbgfs_set, "0x%llx\n");
> #endif /* CONFIG_DEBUG_FS */
>
> +static void ioda_eeh_phb_diag(struct pci_controller *hose)
> +{
> + struct pnv_phb *phb = hose->private_data;
> + unsigned long flags;
> + long rc;
> +
> + spin_lock_irqsave(&phb->lock, flags);
> +
> + rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
> + PNV_PCI_DIAG_BUF_SIZE);
> + if (rc == OPAL_SUCCESS) {
> + phb->flags |= PNV_PHB_FLAG_DIAG;
> + } else {
> + pr_warn("%s: Can't get diag-data for PHB#%x (%ld)\n",
> + __func__, hose->global_number, rc);
> + phb->flags &= ~PNV_PHB_FLAG_DIAG;
> + }
> +
> + spin_unlock_irqrestore(&phb->lock, flags);
> +}
> +
> /**
> * ioda_eeh_post_init - Chip dependent post initialization
> * @hose: PCI controller
> @@ -272,6 +293,8 @@ static int ioda_eeh_get_state(struct eeh_pe *pe)
> result |= EEH_STATE_DMA_ACTIVE;
> result |= EEH_STATE_MMIO_ENABLED;
> result |= EEH_STATE_DMA_ENABLED;
> + } else {
> + ioda_eeh_phb_diag(hose);
> }
>
> return result;
> @@ -541,24 +564,13 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
> static int ioda_eeh_get_log(struct eeh_pe *pe, int severity,
> char *drv_log, unsigned long len)
> {
> - s64 ret;
> + struct pnv_phb *phb = pe->phb->private_data;
> unsigned long flags;
> - struct pci_controller *hose = pe->phb;
> - struct pnv_phb *phb = hose->private_data;
>
> spin_lock_irqsave(&phb->lock, flags);
>
> - ret = opal_pci_get_phb_diag_data2(phb->opal_id,
> - phb->diag.blob, PNV_PCI_DIAG_BUF_SIZE);
> - if (ret) {
> - spin_unlock_irqrestore(&phb->lock, flags);
> - pr_warning("%s: Can't get log for PHB#%x-PE#%x (%lld)\n",
> - __func__, hose->global_number, pe->addr, ret);
> - return -EIO;
> - }
> -
> - /* The PHB diag-data is always indicative */
> - pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
> + pnv_pci_dump_phb_diag_data(pe->phb, phb->diag.blob);
> + phb->flags &= ~PNV_PHB_FLAG_DIAG;
>
> spin_unlock_irqrestore(&phb->lock, flags);
>
> @@ -646,19 +658,11 @@ static void ioda_eeh_hub_diag(struct pci_controller *hose)
> }
> }
>
> -static void ioda_eeh_phb_diag(struct pci_controller *hose)
> +static void ioda_eeh_phb_diag_dump(struct pci_controller *hose)
> {
> struct pnv_phb *phb = hose->private_data;
> - long rc;
> -
> - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
> - PNV_PCI_DIAG_BUF_SIZE);
> - if (rc != OPAL_SUCCESS) {
> - pr_warning("%s: Failed to get diag-data for PHB#%x (%ld)\n",
> - __func__, hose->global_number, rc);
> - return;
> - }
>
> + ioda_eeh_phb_diag(hose);
> pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
> }
>
> @@ -778,7 +782,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
> pr_info("EEH: PHB#%x informative error "
> "detected\n",
> hose->global_number);
> - ioda_eeh_phb_diag(hose);
> + ioda_eeh_phb_diag_dump(hose);
> ret = EEH_NEXT_ERR_NONE;
> }
>
> @@ -809,6 +813,17 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
> }
>
> /*
> + * EEH core will try recover from fenced PHB or
> + * frozen PE. In the time for frozen PE, EEH core
> + * enable IO path for that before collecting logs,
> + * but it ruins the site. So we have to cache the
> + * log in advance here.
> + */
> + if (ret == EEH_NEXT_ERR_FROZEN_PE ||
> + ret == EEH_NEXT_ERR_FENCED_PHB)
> + ioda_eeh_phb_diag(hose);
> +
> + /*
> * If we have no errors on the specific PHB or only
> * informative error there, we continue poking it.
> * Otherwise, we need actions to be taken by upper
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 437c37d..67b2254 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -259,11 +259,15 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose,
> void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
> unsigned char *log_buff)
> {
> + struct pnv_phb *phb = hose->private_data;
> struct OpalIoPhbErrorCommon *common;
>
> if (!hose || !log_buff)
> return;
>
> + if (!(phb->flags & PNV_PHB_FLAG_DIAG))
> + return;
> +
> common = (struct OpalIoPhbErrorCommon *)log_buff;
> switch (common->ioType) {
> case OPAL_PHB_ERROR_DATA_TYPE_P7IOC:
> @@ -281,13 +285,19 @@ void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
> static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no)
> {
> unsigned long flags, rc;
> - int has_diag;
> + bool has_diag = false;
>
> spin_lock_irqsave(&phb->lock, flags);
>
> - rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
> - PNV_PCI_DIAG_BUF_SIZE);
> - has_diag = (rc == OPAL_SUCCESS);
> + if (!(phb->flags & PNV_PHB_FLAG_DIAG)) {
> + rc = opal_pci_get_phb_diag_data2(phb->opal_id,
> + phb->diag.blob,
> + PNV_PCI_DIAG_BUF_SIZE);
> + if (rc == OPAL_SUCCESS) {
> + phb->flags |= PNV_PHB_FLAG_DIAG;
> + has_diag = true;
> + }
> + }
>
> rc = opal_pci_eeh_freeze_clear(phb->opal_id, pe_no,
> OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
> @@ -303,9 +313,6 @@ static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no)
> */
> if (has_diag)
> pnv_pci_dump_phb_diag_data(phb->hose, phb->diag.blob);
> - else
> - pr_warning("PCI %d: No diag data available\n",
> - phb->hose->global_number);
> }
>
> spin_unlock_irqrestore(&phb->lock, flags);
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index adeb3c4..153af9a 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -82,6 +82,7 @@ struct pnv_eeh_ops {
> #endif /* CONFIG_EEH */
>
> #define PNV_PHB_FLAG_EEH (1 << 0)
> +#define PNV_PHB_FLAG_DIAG (1 << 1)
>
> struct pnv_phb {
> struct pci_controller *hose;
^ permalink raw reply
* Re: [PATCH 3/5] powerpc/powernv: Cleanup on PNV_EEH_STATE_ENABLED
From: Benjamin Herrenschmidt @ 2014-02-21 19:56 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1392983613-10090-3-git-send-email-shangw@linux.vnet.ibm.com>
On Fri, 2014-02-21 at 19:53 +0800, Gavin Shan wrote:
> The flag PNV_EEH_STATE_ENABLED is put into pnv_phb::eeh_state, which
> is protected by CONFIG_EEH. We needn't that. Instead, we can have
> pnv_phb::flags and maintain all flags there, which is the purpose
> of the patch.
Can you explain a bit more why we want to create a new flag set
that didn't exist before ? This adds confusion so we need a very
good reason... Do we need to know about the enable state of EEH
even when CNFIG_EEH is not set ?
Cheers,
Ben.
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/eeh-ioda.c | 2 +-
> arch/powerpc/platforms/powernv/pci.c | 8 ++------
> arch/powerpc/platforms/powernv/pci.h | 7 +++----
> 3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
> index 0d1d424..04b4710 100644
> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
> @@ -153,7 +153,7 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
> }
> #endif
>
> - phb->eeh_state |= PNV_EEH_STATE_ENABLED;
> + phb->flags |= PNV_PHB_FLAG_EEH;
>
> return 0;
> }
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index b555ebc..437c37d 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -396,7 +396,7 @@ int pnv_pci_cfg_read(struct device_node *dn,
> if (phb_pe && (phb_pe->state & EEH_PE_ISOLATED))
> return PCIBIOS_SUCCESSFUL;
>
> - if (phb->eeh_state & PNV_EEH_STATE_ENABLED) {
> + if (phb->flags & PNV_PHB_FLAG_EEH) {
> if (*val == EEH_IO_ERROR_VALUE(size) &&
> eeh_dev_check_failure(of_node_to_eeh_dev(dn)))
> return PCIBIOS_DEVICE_NOT_FOUND;
> @@ -434,12 +434,8 @@ int pnv_pci_cfg_write(struct device_node *dn,
> }
>
> /* Check if the PHB got frozen due to an error (no response) */
> -#ifdef CONFIG_EEH
> - if (!(phb->eeh_state & PNV_EEH_STATE_ENABLED))
> + if (!(phb->flags & PNV_PHB_FLAG_EEH))
> pnv_pci_config_check_eeh(phb, dn);
> -#else
> - pnv_pci_config_check_eeh(phb, dn);
> -#endif
>
> return PCIBIOS_SUCCESSFUL;
> }
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index dbeba3d..adeb3c4 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -79,24 +79,23 @@ struct pnv_eeh_ops {
> int (*configure_bridge)(struct eeh_pe *pe);
> int (*next_error)(struct eeh_pe **pe);
> };
> -
> -#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */
> -
> #endif /* CONFIG_EEH */
>
> +#define PNV_PHB_FLAG_EEH (1 << 0)
> +
> struct pnv_phb {
> struct pci_controller *hose;
> enum pnv_phb_type type;
> enum pnv_phb_model model;
> u64 hub_id;
> u64 opal_id;
> + int flags;
> void __iomem *regs;
> int initialized;
> spinlock_t lock;
>
> #ifdef CONFIG_EEH
> struct pnv_eeh_ops *eeh_ops;
> - int eeh_state;
> #endif
>
> #ifdef CONFIG_DEBUG_FS
^ permalink raw reply
* Re: [PATCH] rapidio/tsi721: fix tasklet termination in dma channel release
From: Thomas Gleixner @ 2014-02-21 19:51 UTC (permalink / raw)
To: Alexandre Bounine
Cc: Mike Galbraith, linux-kernel, stable, Xiaotian Feng,
Andrew Morton, linuxppc-dev
In-Reply-To: <1393010358-6837-1-git-send-email-alexandre.bounine@idt.com>
On Fri, 21 Feb 2014, Alexandre Bounine wrote:
> This patch is a modification of the patch originally proposed
> by Xiaotian Feng <xtfeng@gmail.com>: https://lkml.org/lkml/2012/11/5/413
> This new version disables DMA channel interrupts and ensures that the tasklet
> wil not be scheduled again before calling tasklet_kill().
>
> Unfortunately the updated patch was not released at that time due to planned
> rework of Tsi721 mport driver to use threaded interrupts (which has yet to
> happen).
> Recently the issue was reported again: https://lkml.org/lkml/2014/2/19/762.
>
> Description from the original Xiaotian's patch:
>
> "Some drivers use tasklet_disable in device remove/release process,
> tasklet_disable will inc tasklet->count and return. If the tasklet is
> not handled yet under some softirq pressure, the tasklet will be
> placed on the tasklet_vec, never have a chance to be excuted. This
> might lead to a heavy loaded ksoftirqd, wakeup with pending_softirq,
> but tasklet is disabled. tasklet_kill should be used in this case."
>
> This patch is applicable to kernel versions starting from v3.5.
>
> Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
> Cc: Matt Porter <mporter@kernel.crashing.org>
> Cc: Xiaotian Feng <xtfeng@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mike Galbraith <bitbucket@online.de>
> Cc: <stable@vger.kernel.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* [PATCH] rapidio/tsi721: fix tasklet termination in dma channel release
From: Alexandre Bounine @ 2014-02-21 19:19 UTC (permalink / raw)
To: Andrew Morton, linux-kernel, linuxppc-dev
Cc: Mike Galbraith, stable, Alexandre Bounine, Xiaotian Feng,
Thomas Gleixner
This patch is a modification of the patch originally proposed
by Xiaotian Feng <xtfeng@gmail.com>: https://lkml.org/lkml/2012/11/5/413
This new version disables DMA channel interrupts and ensures that the tasklet
wil not be scheduled again before calling tasklet_kill().
Unfortunately the updated patch was not released at that time due to planned
rework of Tsi721 mport driver to use threaded interrupts (which has yet to
happen).
Recently the issue was reported again: https://lkml.org/lkml/2014/2/19/762.
Description from the original Xiaotian's patch:
"Some drivers use tasklet_disable in device remove/release process,
tasklet_disable will inc tasklet->count and return. If the tasklet is
not handled yet under some softirq pressure, the tasklet will be
placed on the tasklet_vec, never have a chance to be excuted. This
might lead to a heavy loaded ksoftirqd, wakeup with pending_softirq,
but tasklet is disabled. tasklet_kill should be used in this case."
This patch is applicable to kernel versions starting from v3.5.
Signed-off-by: Alexandre Bounine <alexandre.bounine@idt.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Xiaotian Feng <xtfeng@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mike Galbraith <bitbucket@online.de>
Cc: <stable@vger.kernel.org>
---
drivers/rapidio/devices/tsi721.h | 1 +
drivers/rapidio/devices/tsi721_dma.c | 25 ++++++++++++++++++-------
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/rapidio/devices/tsi721.h b/drivers/rapidio/devices/tsi721.h
index b4b0d83..7061ac0 100644
--- a/drivers/rapidio/devices/tsi721.h
+++ b/drivers/rapidio/devices/tsi721.h
@@ -678,6 +678,7 @@ struct tsi721_bdma_chan {
struct list_head free_list;
dma_cookie_t completed_cookie;
struct tasklet_struct tasklet;
+ bool active;
};
#endif /* CONFIG_RAPIDIO_DMA_ENGINE */
diff --git a/drivers/rapidio/devices/tsi721_dma.c b/drivers/rapidio/devices/tsi721_dma.c
index 502663f..b57f650 100644
--- a/drivers/rapidio/devices/tsi721_dma.c
+++ b/drivers/rapidio/devices/tsi721_dma.c
@@ -206,8 +206,8 @@ void tsi721_bdma_handler(struct tsi721_bdma_chan *bdma_chan)
{
/* Disable BDMA channel interrupts */
iowrite32(0, bdma_chan->regs + TSI721_DMAC_INTE);
-
- tasklet_schedule(&bdma_chan->tasklet);
+ if (bdma_chan->active)
+ tasklet_schedule(&bdma_chan->tasklet);
}
#ifdef CONFIG_PCI_MSI
@@ -562,7 +562,7 @@ static int tsi721_alloc_chan_resources(struct dma_chan *dchan)
}
#endif /* CONFIG_PCI_MSI */
- tasklet_enable(&bdma_chan->tasklet);
+ bdma_chan->active = true;
tsi721_bdma_interrupt_enable(bdma_chan, 1);
return bdma_chan->bd_num - 1;
@@ -589,14 +589,25 @@ static void tsi721_free_chan_resources(struct dma_chan *dchan)
BUG_ON(!list_empty(&bdma_chan->active_list));
BUG_ON(!list_empty(&bdma_chan->queue));
- tasklet_disable(&bdma_chan->tasklet);
+ tsi721_bdma_interrupt_enable(bdma_chan, 0);
+ bdma_chan->active = false;
+
+#ifdef CONFIG_PCI_MSI
+ if (priv->flags & TSI721_USING_MSIX) {
+ synchronize_irq(priv->msix[TSI721_VECT_DMA0_DONE +
+ bdma_chan->id].vector);
+ synchronize_irq(priv->msix[TSI721_VECT_DMA0_INT +
+ bdma_chan->id].vector);
+ } else
+#endif
+ synchronize_irq(priv->pdev->irq);
+
+ tasklet_kill(&bdma_chan->tasklet);
spin_lock_bh(&bdma_chan->lock);
list_splice_init(&bdma_chan->free_list, &list);
spin_unlock_bh(&bdma_chan->lock);
- tsi721_bdma_interrupt_enable(bdma_chan, 0);
-
#ifdef CONFIG_PCI_MSI
if (priv->flags & TSI721_USING_MSIX) {
free_irq(priv->msix[TSI721_VECT_DMA0_DONE +
@@ -790,6 +801,7 @@ int tsi721_register_dma(struct tsi721_device *priv)
bdma_chan->dchan.cookie = 1;
bdma_chan->dchan.chan_id = i;
bdma_chan->id = i;
+ bdma_chan->active = false;
spin_lock_init(&bdma_chan->lock);
@@ -799,7 +811,6 @@ int tsi721_register_dma(struct tsi721_device *priv)
tasklet_init(&bdma_chan->tasklet, tsi721_dma_tasklet,
(unsigned long)bdma_chan);
- tasklet_disable(&bdma_chan->tasklet);
list_add_tail(&bdma_chan->dchan.device_node,
&mport->dma.channels);
}
--
1.7.8.4
^ permalink raw reply related
* Re: [PATCH] PPC: KVM: Introduce hypervisor call H_GET_TCE
From: Benjamin Herrenschmidt @ 2014-02-21 19:23 UTC (permalink / raw)
To: Laurent Dufour
Cc: Alexey Kardashevskiy, kvm, Gleb Natapov, Alexander Graf, kvm-ppc,
Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20140221153110.8190.6271.stgit@nimbus>
On Fri, 2014-02-21 at 16:31 +0100, Laurent Dufour wrote:
> This fix introduces the H_GET_TCE hypervisor call which is basically the
> reverse of H_PUT_TCE, as defined in the Power Architecture Platform
> Requirements (PAPR).
>
> The hcall H_GET_TCE is required by the kdump kernel which is calling it to
> retrieve the TCE set up by the panicing kernel.
Alexey, will that work for VFIO ? Or are those patches *still* not
upstream ?
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 2 ++
> arch/powerpc/kvm/book3s_64_vio_hv.c | 28 ++++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 +-
> 3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index fcd53f0..4096f16 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -129,6 +129,8 @@ extern long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> struct kvm_create_spapr_tce *args);
> extern long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> unsigned long ioba, unsigned long tce);
> +extern long kvmppc_h_get_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> + unsigned long ioba);
> extern struct kvm_rma_info *kvm_alloc_rma(void);
> extern void kvm_release_rma(struct kvm_rma_info *ri);
> extern struct page *kvm_alloc_hpt(unsigned long nr_pages);
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index 2c25f54..89e96b3 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -75,3 +75,31 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> return H_TOO_HARD;
> }
> EXPORT_SYMBOL_GPL(kvmppc_h_put_tce);
> +
> +long kvmppc_h_get_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> + unsigned long ioba)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvmppc_spapr_tce_table *stt;
> +
> + list_for_each_entry(stt, &kvm->arch.spapr_tce_tables, list) {
> + if (stt->liobn == liobn) {
> + unsigned long idx = ioba >> SPAPR_TCE_SHIFT;
> + struct page *page;
> + u64 *tbl;
> +
> + if (ioba >= stt->window_size)
> + return H_PARAMETER;
> +
> + page = stt->pages[idx / TCES_PER_PAGE];
> + tbl = (u64 *)page_address(page);
> +
> + vcpu->arch.gpr[4] = tbl[idx % TCES_PER_PAGE];
> + return H_SUCCESS;
> + }
> + }
> +
> + /* Didn't find the liobn, punt it to userspace */
> + return H_TOO_HARD;
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_h_get_tce);
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index e66d4ec..7d4fe2a 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1758,7 +1758,7 @@ hcall_real_table:
> .long 0 /* 0x10 - H_CLEAR_MOD */
> .long 0 /* 0x14 - H_CLEAR_REF */
> .long .kvmppc_h_protect - hcall_real_table
> - .long 0 /* 0x1c - H_GET_TCE */
> + .long .kvmppc_h_get_tce - hcall_real_table
> .long .kvmppc_h_put_tce - hcall_real_table
> .long 0 /* 0x24 - H_SET_SPRG0 */
> .long .kvmppc_h_set_dabr - hcall_real_table
^ permalink raw reply
* Re: [PATCH] PPC: KVM: Introduce hypervisor call H_GET_TCE
From: Alexander Graf @ 2014-02-21 15:57 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm@vger.kernel.org mailing list, Gleb Natapov, kvm-ppc,
Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20140221153110.8190.6271.stgit@nimbus>
On 21.02.2014, at 16:31, Laurent Dufour <ldufour@linux.vnet.ibm.com> =
wrote:
> This fix introduces the H_GET_TCE hypervisor call which is basically =
the
> reverse of H_PUT_TCE, as defined in the Power Architecture Platform
> Requirements (PAPR).
>=20
> The hcall H_GET_TCE is required by the kdump kernel which is calling =
it to
> retrieve the TCE set up by the panicing kernel.
>=20
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Thanks, applied to kvm-ppc-queue. Btw, why exactly are we using struct =
page pointers and alloc_page rather than __get_free_page() and simple =
page start pointers?
Alex
^ permalink raw reply
* Re: [PATCH 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
From: Alan Stern @ 2014-02-21 15:41 UTC (permalink / raw)
To: Alistair Popple; +Cc: gregkh, linux-usb, linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1392964293-13687-5-git-send-email-alistair@popple.id.au>
On Fri, 21 Feb 2014, Alistair Popple wrote:
> Currently the ppc-of driver uses the compatibility string
> "usb-ehci". This means platforms that use device-tree and implement an
> EHCI compatible interface have to either use the ppc-of driver or add
> a compatible line to the ehci-platform driver. It would be more
> appropriate for the platform driver to be compatible with "usb-ehci"
> as non-powerpc platforms are also beginning to utilise device-tree.
>
> This patch merges the device tree property parsing from ehci-ppc-of
> into the platform driver and adds a "usb-ehci" compatibility
> string. The existing ehci-ppc-of driver is removed and the 440EPX
> specific quirks are added to the ehci-platform driver.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
This patch is also out of date. The compatibility string used by the
ehci-platform driver is "generic-ehci".
There remains the question of whether to merge ehci-ppc-of into
ehci-platform. This would be a rather invasive change, but I suppose
we could do it. With adjustments along the lines suggested by Mark
Rutland.
Alan Stern
^ permalink raw reply
* Re: [PATCH 3/7] IBM Akebono: Add support to the OHCI platform driver for PPC476GTR
From: Alan Stern @ 2014-02-21 15:34 UTC (permalink / raw)
To: Alistair Popple; +Cc: gregkh, linux-usb, linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1392964293-13687-4-git-send-email-alistair@popple.id.au>
On Fri, 21 Feb 2014, Alistair Popple wrote:
> The IBM Akebono board uses the PPC476GTR SoC which has a OHCI
> compliant USB host interface. This patch adds support for it to the
> OHCI platform driver.
>
> As we use device tree to pass platform specific data instead of
> platform data we remove the check for platform data and instead
> provide reasonable defaults if no platform data is present. This is
> similar to what is currently done in ehci-platform.c.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
As Arnd pointed out, this patch is out of date. See
https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/drivers/usb/host/ohci-platform.c?h=usb-next&id=ca52a17ba975dbf47e87c9bc63086aca0cf92806
and
https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/drivers/usb/host/ohci-platform.c?h=usb-next&id=ce149c30b9f89d0c9addd1d71ccdb57c1051553b
Alan Stern
^ permalink raw reply
* [PATCH] PPC: KVM: Introduce hypervisor call H_GET_TCE
From: Laurent Dufour @ 2014-02-21 15:31 UTC (permalink / raw)
To: Gleb Natapov, Paolo Bonzini, Paul Mackerras, Alexander Graf,
Benjamin Herrenschmidt
Cc: linuxppc-dev, kvm, kvm-ppc
This fix introduces the H_GET_TCE hypervisor call which is basically the
reverse of H_PUT_TCE, as defined in the Power Architecture Platform
Requirements (PAPR).
The hcall H_GET_TCE is required by the kdump kernel which is calling it to
retrieve the TCE set up by the panicing kernel.
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/kvm_ppc.h | 2 ++
arch/powerpc/kvm/book3s_64_vio_hv.c | 28 ++++++++++++++++++++++++++++
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 +-
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index fcd53f0..4096f16 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -129,6 +129,8 @@ extern long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
struct kvm_create_spapr_tce *args);
extern long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
unsigned long ioba, unsigned long tce);
+extern long kvmppc_h_get_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
+ unsigned long ioba);
extern struct kvm_rma_info *kvm_alloc_rma(void);
extern void kvm_release_rma(struct kvm_rma_info *ri);
extern struct page *kvm_alloc_hpt(unsigned long nr_pages);
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index 2c25f54..89e96b3 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -75,3 +75,31 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
return H_TOO_HARD;
}
EXPORT_SYMBOL_GPL(kvmppc_h_put_tce);
+
+long kvmppc_h_get_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
+ unsigned long ioba)
+{
+ struct kvm *kvm = vcpu->kvm;
+ struct kvmppc_spapr_tce_table *stt;
+
+ list_for_each_entry(stt, &kvm->arch.spapr_tce_tables, list) {
+ if (stt->liobn == liobn) {
+ unsigned long idx = ioba >> SPAPR_TCE_SHIFT;
+ struct page *page;
+ u64 *tbl;
+
+ if (ioba >= stt->window_size)
+ return H_PARAMETER;
+
+ page = stt->pages[idx / TCES_PER_PAGE];
+ tbl = (u64 *)page_address(page);
+
+ vcpu->arch.gpr[4] = tbl[idx % TCES_PER_PAGE];
+ return H_SUCCESS;
+ }
+ }
+
+ /* Didn't find the liobn, punt it to userspace */
+ return H_TOO_HARD;
+}
+EXPORT_SYMBOL_GPL(kvmppc_h_get_tce);
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index e66d4ec..7d4fe2a 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1758,7 +1758,7 @@ hcall_real_table:
.long 0 /* 0x10 - H_CLEAR_MOD */
.long 0 /* 0x14 - H_CLEAR_REF */
.long .kvmppc_h_protect - hcall_real_table
- .long 0 /* 0x1c - H_GET_TCE */
+ .long .kvmppc_h_get_tce - hcall_real_table
.long .kvmppc_h_put_tce - hcall_real_table
.long 0 /* 0x24 - H_SET_SPRG0 */
.long .kvmppc_h_set_dabr - hcall_real_table
^ permalink raw reply related
* Re: [PATCH 7/7] powerpc: Added PCI MSI support using the HSTA module
From: Arnd Bergmann @ 2014-02-21 14:33 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1392964293-13687-8-git-send-email-alistair@popple.id.au>
On Friday 21 February 2014 17:31:33 Alistair Popple wrote:
>
> + HSTA0: hsta@310000e0000 {
> + compatible = "ibm,476gtr-hsta-msi", "ibm,hsta-msi";
> + reg = <0x310 0x000e0000 0x0 0xf0>;
> + interrupt-parent = <&MPIC>;
> + interrupts = <108 0
> + 109 0
> + 110 0
> + 111 0
> + 112 0
> + 113 0
> + 114 0
> + 115 0
> + 116 0
> + 117 0
> + 118 0
> + 119 0
> + 120 0
> + 121 0
> + 122 0
> + 123 0>;
> + };
Please add a binding to Documentation/devicetree for this device.
> @@ -242,8 +264,10 @@
> ranges = <0x02000000 0x00000000 0x80000000 0x00000110 0x80000000 0x0 0x80000000
> 0x01000000 0x0 0x0 0x00000140 0x0 0x0 0x00010000>;
>
> - /* Inbound starting at 0 to memsize filled in by zImage */
> - dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x0 0x0>;
> + /* Inbound starting at 0x0 to 0x40000000000. In order to use MSI
> + * PCI devices must be able to write to the HSTA module.
> + */
> + dma-ranges = <0x42000000 0x0 0x0 0x0 0x0 0x400 0x0>;
>
> /* This drives busses 0 to 0xf */
> bus-range = <0x0 0xf>;
Ah, I first only saw the line you are removing and was about
to suggest what you do anyway. Great!
> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
> index 54ec1d5..7cc3acc 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.c
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.c
> @@ -176,8 +176,12 @@ static int __init ppc4xx_parse_dma_ranges(struct pci_controller *hose,
> return -ENXIO;
> }
>
> - /* Check that we are fully contained within 32 bits space */
> - if (res->end > 0xffffffff) {
> + /* Check that we are fully contained within 32 bits space if we are not
> + * running on a 460sx or 476fpe which have 64 bit bus addresses.
> + */
> + if (res->end > 0xffffffff &&
> + !(of_device_is_compatible(hose->dn, "ibm,plb-pciex-460sx")
> + || of_device_is_compatible(hose->dn, "ibm,plb-pciex-476fpe"))) {
> printk(KERN_ERR "%s: dma-ranges outside of 32 bits space\n",
> hose->dn->full_name);
> return -ENXIO;
A more general question for BenH: Apparently this PCI implementation is
getting reused on arm64 for APM X-Gene. Do you see any value in trying to
share host controller drivers like this one across powerpc and arm64?
It's possible we are going to see the same situation with fsl_pci in the
future, if arm and powerpc qoriq chips use the same peripherals. My
plan for arm64 right now is to make PCI work without any code in arch/,
just using new helper functions in drivers/pci and sticking the host
drivers into drivers/pci/host as we started doing for arm32, but it
can require significant work to make those drivers compatible with
the powerpc pci-common.c.
Arnd
^ permalink raw reply
* Re: [PATCH 3/7] IBM Akebono: Add support to the OHCI platform driver for PPC476GTR
From: Arnd Bergmann @ 2014-02-21 14:16 UTC (permalink / raw)
To: Alistair Popple; +Cc: gregkh, linux-usb, linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1392964293-13687-4-git-send-email-alistair@popple.id.au>
On Friday 21 February 2014 17:31:29 Alistair Popple wrote:
>
> +static const struct of_device_id ohci_of_match[] = {
> + { .compatible = "usb-ohci", },
> + {},
> +};
> +
> static const struct platform_device_id ohci_platform_table[] = {
> { "ohci-platform", 0 },
> { }
> @@ -198,6 +209,7 @@ static struct platform_driver ohci_platform_driver = {
> .owner = THIS_MODULE,
> .name = "ohci-platform",
> .pm = &ohci_platform_pm_ops,
> + .of_match_table = ohci_of_match,
> }
> };
>
>
Linux-next already has a patch to add an of_match_table in this driver,
using
static const struct of_device_id ohci_platform_ids[] = {
{ .compatible = "generic-ohci", },
{ }
};
I think you should just use that string on your platform.
Arnd
^ permalink raw reply
* Re: [PATCH 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
From: Arnd Bergmann @ 2014-02-21 14:21 UTC (permalink / raw)
To: linuxppc-dev
Cc: Mark Rutland, devicetree@vger.kernel.org, Alistair Popple,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Prisk, gregkh@linuxfoundation.org
In-Reply-To: <20140221114803.GB8783@e106331-lin.cambridge.arm.com>
On Friday 21 February 2014 11:48:03 Mark Rutland wrote:
> > +
> > + np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx");
> > + if (np != NULL) {
> > + /* claim we really affected by usb23 erratum */
> > + if (!of_address_to_resource(np, 0, &res))
> > + ehci->ohci_hcctrl_reg =
> > + devm_ioremap(&pdev->dev,
> > + res.start + OHCI_HCCTRL_OFFSET,
> > + OHCI_HCCTRL_LEN);
> > + else
> > + ehci_dbg(ehci, "%s: no ohci offset in fdt\n", __FILE__);
> > + if (!ehci->ohci_hcctrl_reg) {
> > + ehci_dbg(ehci, "%s: ioremap for ohci hcctrl failed\n",
> > + __FILE__);
> > + } else {
> > + ehci->has_amcc_usb23 = 1;
> > + }
> > + }
>
> As this is being dropped into a generic driver, it would be nice to have
> a comment explaining why we need to do this -- not everyone using this
> will know the powerpc history. It certainly seems odd to look for
> another node in the tree that this driver isn't necessarily handling,
> and that should probably be explained.
>
> As this bit of fixup is only needed for powerpc, it would be nice to not
> have to do it elsewhere. Perhaps these could be factored out into a
> ppc_platform_reset function that could be empty stub for other
> architectures.
How about using the .data field of the of_device_id array to point to
a structure of functions? That way you don't even have to call
of_device_is_compatible() here. Note that using of_find_compatible_node()
is the wrong approach anyway, you want to check the current device
for compatibility, not just any device I assume.
Arnd
^ permalink raw reply
* Re: [PATCH 1/7] IBM Akebono: Add a SDHCI platform driver
From: Arnd Bergmann @ 2014-02-21 14:14 UTC (permalink / raw)
To: Alistair Popple; +Cc: devicetree, cjb, linux-mmc, linux-kernel, linuxppc-dev
In-Reply-To: <1392964293-13687-2-git-send-email-alistair@popple.id.au>
On Friday 21 February 2014 17:31:27 Alistair Popple wrote:
>
> +config MMC_SDHCI_OF_476GTR
> + tristate "SDHCI OF support for the IBM PPC476GTR SoC"
> + depends on MMC_SDHCI_PLTFM
> + depends on PPC_OF
> + help
> + This selects the Secure Digital Host Controller Interface (SDHCI)
> + found on the PPC476GTR SoC.
> +
> + If you have a controller with this interface, say Y or M here.
> +
> + If unsure, say N.
Your driver doesn't actually do anything beyond what is in the common
sdhci-pltfm.c infrastructure. IMHO you really shoulnd't need a SoC
specific abstraction for it at all and instead add a generic
platform driver registration into sdhci-pltfm.c. I'd suggest
you use "generic-sdhci" (similar to what we do for usb-ohci and usb-ehci
now) as the compatible string and change your device tree to claim
compatibility with that and your soc-specific string.
Arnd
^ permalink raw reply
* [PATCH 3/5] powerpc/powernv: Cleanup on PNV_EEH_STATE_ENABLED
From: Gavin Shan @ 2014-02-21 11:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1392983613-10090-1-git-send-email-shangw@linux.vnet.ibm.com>
The flag PNV_EEH_STATE_ENABLED is put into pnv_phb::eeh_state, which
is protected by CONFIG_EEH. We needn't that. Instead, we can have
pnv_phb::flags and maintain all flags there, which is the purpose
of the patch.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/eeh-ioda.c | 2 +-
arch/powerpc/platforms/powernv/pci.c | 8 ++------
arch/powerpc/platforms/powernv/pci.h | 7 +++----
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 0d1d424..04b4710 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -153,7 +153,7 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
}
#endif
- phb->eeh_state |= PNV_EEH_STATE_ENABLED;
+ phb->flags |= PNV_PHB_FLAG_EEH;
return 0;
}
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b555ebc..437c37d 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -396,7 +396,7 @@ int pnv_pci_cfg_read(struct device_node *dn,
if (phb_pe && (phb_pe->state & EEH_PE_ISOLATED))
return PCIBIOS_SUCCESSFUL;
- if (phb->eeh_state & PNV_EEH_STATE_ENABLED) {
+ if (phb->flags & PNV_PHB_FLAG_EEH) {
if (*val == EEH_IO_ERROR_VALUE(size) &&
eeh_dev_check_failure(of_node_to_eeh_dev(dn)))
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -434,12 +434,8 @@ int pnv_pci_cfg_write(struct device_node *dn,
}
/* Check if the PHB got frozen due to an error (no response) */
-#ifdef CONFIG_EEH
- if (!(phb->eeh_state & PNV_EEH_STATE_ENABLED))
+ if (!(phb->flags & PNV_PHB_FLAG_EEH))
pnv_pci_config_check_eeh(phb, dn);
-#else
- pnv_pci_config_check_eeh(phb, dn);
-#endif
return PCIBIOS_SUCCESSFUL;
}
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index dbeba3d..adeb3c4 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -79,24 +79,23 @@ struct pnv_eeh_ops {
int (*configure_bridge)(struct eeh_pe *pe);
int (*next_error)(struct eeh_pe **pe);
};
-
-#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */
-
#endif /* CONFIG_EEH */
+#define PNV_PHB_FLAG_EEH (1 << 0)
+
struct pnv_phb {
struct pci_controller *hose;
enum pnv_phb_type type;
enum pnv_phb_model model;
u64 hub_id;
u64 opal_id;
+ int flags;
void __iomem *regs;
int initialized;
spinlock_t lock;
#ifdef CONFIG_EEH
struct pnv_eeh_ops *eeh_ops;
- int eeh_state;
#endif
#ifdef CONFIG_DEBUG_FS
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/5] powerpc/powernv: Cache PHB diag-data
From: Gavin Shan @ 2014-02-21 11:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1392983613-10090-1-git-send-email-shangw@linux.vnet.ibm.com>
EEH core tries to recover from fenced PHB or frozen PE. Unfortunately,
the log isn't consistent with the site because enabling IO path for
the frozen PE before collecting log would ruin the site. The patch
solves the problem to cache the PHB diag-data in advance with the
help of additional flag PNV_PHB_FLAG_DIAG to pnv_phb::flags.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/eeh-ioda.c | 65 ++++++++++++++++++-----------
arch/powerpc/platforms/powernv/pci.c | 21 ++++++----
arch/powerpc/platforms/powernv/pci.h | 1 +
3 files changed, 55 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 04b4710..3ed8d22 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -114,6 +114,27 @@ DEFINE_SIMPLE_ATTRIBUTE(ioda_eeh_inbB_dbgfs_ops, ioda_eeh_inbB_dbgfs_get,
ioda_eeh_inbB_dbgfs_set, "0x%llx\n");
#endif /* CONFIG_DEBUG_FS */
+static void ioda_eeh_phb_diag(struct pci_controller *hose)
+{
+ struct pnv_phb *phb = hose->private_data;
+ unsigned long flags;
+ long rc;
+
+ spin_lock_irqsave(&phb->lock, flags);
+
+ rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
+ PNV_PCI_DIAG_BUF_SIZE);
+ if (rc == OPAL_SUCCESS) {
+ phb->flags |= PNV_PHB_FLAG_DIAG;
+ } else {
+ pr_warn("%s: Can't get diag-data for PHB#%x (%ld)\n",
+ __func__, hose->global_number, rc);
+ phb->flags &= ~PNV_PHB_FLAG_DIAG;
+ }
+
+ spin_unlock_irqrestore(&phb->lock, flags);
+}
+
/**
* ioda_eeh_post_init - Chip dependent post initialization
* @hose: PCI controller
@@ -272,6 +293,8 @@ static int ioda_eeh_get_state(struct eeh_pe *pe)
result |= EEH_STATE_DMA_ACTIVE;
result |= EEH_STATE_MMIO_ENABLED;
result |= EEH_STATE_DMA_ENABLED;
+ } else {
+ ioda_eeh_phb_diag(hose);
}
return result;
@@ -541,24 +564,13 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
static int ioda_eeh_get_log(struct eeh_pe *pe, int severity,
char *drv_log, unsigned long len)
{
- s64 ret;
+ struct pnv_phb *phb = pe->phb->private_data;
unsigned long flags;
- struct pci_controller *hose = pe->phb;
- struct pnv_phb *phb = hose->private_data;
spin_lock_irqsave(&phb->lock, flags);
- ret = opal_pci_get_phb_diag_data2(phb->opal_id,
- phb->diag.blob, PNV_PCI_DIAG_BUF_SIZE);
- if (ret) {
- spin_unlock_irqrestore(&phb->lock, flags);
- pr_warning("%s: Can't get log for PHB#%x-PE#%x (%lld)\n",
- __func__, hose->global_number, pe->addr, ret);
- return -EIO;
- }
-
- /* The PHB diag-data is always indicative */
- pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
+ pnv_pci_dump_phb_diag_data(pe->phb, phb->diag.blob);
+ phb->flags &= ~PNV_PHB_FLAG_DIAG;
spin_unlock_irqrestore(&phb->lock, flags);
@@ -646,19 +658,11 @@ static void ioda_eeh_hub_diag(struct pci_controller *hose)
}
}
-static void ioda_eeh_phb_diag(struct pci_controller *hose)
+static void ioda_eeh_phb_diag_dump(struct pci_controller *hose)
{
struct pnv_phb *phb = hose->private_data;
- long rc;
-
- rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
- PNV_PCI_DIAG_BUF_SIZE);
- if (rc != OPAL_SUCCESS) {
- pr_warning("%s: Failed to get diag-data for PHB#%x (%ld)\n",
- __func__, hose->global_number, rc);
- return;
- }
+ ioda_eeh_phb_diag(hose);
pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
}
@@ -778,7 +782,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
pr_info("EEH: PHB#%x informative error "
"detected\n",
hose->global_number);
- ioda_eeh_phb_diag(hose);
+ ioda_eeh_phb_diag_dump(hose);
ret = EEH_NEXT_ERR_NONE;
}
@@ -809,6 +813,17 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
}
/*
+ * EEH core will try recover from fenced PHB or
+ * frozen PE. In the time for frozen PE, EEH core
+ * enable IO path for that before collecting logs,
+ * but it ruins the site. So we have to cache the
+ * log in advance here.
+ */
+ if (ret == EEH_NEXT_ERR_FROZEN_PE ||
+ ret == EEH_NEXT_ERR_FENCED_PHB)
+ ioda_eeh_phb_diag(hose);
+
+ /*
* If we have no errors on the specific PHB or only
* informative error there, we continue poking it.
* Otherwise, we need actions to be taken by upper
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 437c37d..67b2254 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -259,11 +259,15 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose,
void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
unsigned char *log_buff)
{
+ struct pnv_phb *phb = hose->private_data;
struct OpalIoPhbErrorCommon *common;
if (!hose || !log_buff)
return;
+ if (!(phb->flags & PNV_PHB_FLAG_DIAG))
+ return;
+
common = (struct OpalIoPhbErrorCommon *)log_buff;
switch (common->ioType) {
case OPAL_PHB_ERROR_DATA_TYPE_P7IOC:
@@ -281,13 +285,19 @@ void pnv_pci_dump_phb_diag_data(struct pci_controller *hose,
static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no)
{
unsigned long flags, rc;
- int has_diag;
+ bool has_diag = false;
spin_lock_irqsave(&phb->lock, flags);
- rc = opal_pci_get_phb_diag_data2(phb->opal_id, phb->diag.blob,
- PNV_PCI_DIAG_BUF_SIZE);
- has_diag = (rc == OPAL_SUCCESS);
+ if (!(phb->flags & PNV_PHB_FLAG_DIAG)) {
+ rc = opal_pci_get_phb_diag_data2(phb->opal_id,
+ phb->diag.blob,
+ PNV_PCI_DIAG_BUF_SIZE);
+ if (rc == OPAL_SUCCESS) {
+ phb->flags |= PNV_PHB_FLAG_DIAG;
+ has_diag = true;
+ }
+ }
rc = opal_pci_eeh_freeze_clear(phb->opal_id, pe_no,
OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
@@ -303,9 +313,6 @@ static void pnv_pci_handle_eeh_config(struct pnv_phb *phb, u32 pe_no)
*/
if (has_diag)
pnv_pci_dump_phb_diag_data(phb->hose, phb->diag.blob);
- else
- pr_warning("PCI %d: No diag data available\n",
- phb->hose->global_number);
}
spin_unlock_irqrestore(&phb->lock, flags);
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index adeb3c4..153af9a 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -82,6 +82,7 @@ struct pnv_eeh_ops {
#endif /* CONFIG_EEH */
#define PNV_PHB_FLAG_EEH (1 << 0)
+#define PNV_PHB_FLAG_DIAG (1 << 1)
struct pnv_phb {
struct pci_controller *hose;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/5] powerpc/powernv: Make PHB diag-data output short
From: Gavin Shan @ 2014-02-21 11:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1392983613-10090-1-git-send-email-shangw@linux.vnet.ibm.com>
According to Ben's suggestion, the patch makes the PHB diag-data
dump looks a bit short by printing multiple values in one line
and outputing "-" for zero fields.
After the patch applied, the PHB diag-data dump looks like:
PHB3 PHB#3 Diag-data (Version: 1)
brdgCtl: 00000002
UtlSts: - - -
RootSts: 0000000f 00400000 b0830008 00100147 00002000
RootErrSts: - - -
RootErrLog: - - - -
RootErrLog1: - - -
nFir: - 0030006e00000000 -
PhbSts: 0000001c00000000 -
Lem: 0000000000100000 42498e327f502eae -
PhbErr: - - - -
OutErr: - - - -
InAErr: 8000000000000000 8000000000000000 0402030000000000 -
InBErr: - - - -
PE[ 8] A/B: 8480002b00000000 8000000000000000
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/pci.c | 238 ++++++++++++++++++++--------------
1 file changed, 143 insertions(+), 95 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 67b2254..a5f236a 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -124,67 +124,103 @@ static void pnv_teardown_msi_irqs(struct pci_dev *pdev)
}
#endif /* CONFIG_PCI_MSI */
+static char *pnv_pci_diag_field(char *buf, int fmt, u64 val64)
+{
+ u32 val32 = (u32)val64;
+
+ memset(buf, 0, 24);
+ switch (fmt) {
+ case 8:
+ if (val32)
+ sprintf(buf, "%08x", val32);
+ else
+ sprintf(buf, "%s", "-");
+ break;
+ case 16:
+ if (val64)
+ sprintf(buf, "%016llx", val64);
+ else
+ sprintf(buf, "%s", "-");
+ break;
+ default:
+ sprintf(buf, "%s", "-");
+ }
+
+ return buf;
+}
+
static void pnv_pci_dump_p7ioc_diag_data(struct pci_controller *hose,
struct OpalIoPhbErrorCommon *common)
{
struct OpalIoP7IOCPhbErrorData *data;
+ char buf[120];
int i;
data = (struct OpalIoP7IOCPhbErrorData *)common;
pr_info("P7IOC PHB#%d Diag-data (Version: %d)\n\n",
hose->global_number, common->version);
- pr_info(" brdgCtl: %08x\n", data->brdgCtl);
-
- pr_info(" portStatusReg: %08x\n", data->portStatusReg);
- pr_info(" rootCmplxStatus: %08x\n", data->rootCmplxStatus);
- pr_info(" busAgentStatus: %08x\n", data->busAgentStatus);
-
- pr_info(" deviceStatus: %08x\n", data->deviceStatus);
- pr_info(" slotStatus: %08x\n", data->slotStatus);
- pr_info(" linkStatus: %08x\n", data->linkStatus);
- pr_info(" devCmdStatus: %08x\n", data->devCmdStatus);
- pr_info(" devSecStatus: %08x\n", data->devSecStatus);
-
- pr_info(" rootErrorStatus: %08x\n", data->rootErrorStatus);
- pr_info(" uncorrErrorStatus: %08x\n", data->uncorrErrorStatus);
- pr_info(" corrErrorStatus: %08x\n", data->corrErrorStatus);
- pr_info(" tlpHdr1: %08x\n", data->tlpHdr1);
- pr_info(" tlpHdr2: %08x\n", data->tlpHdr2);
- pr_info(" tlpHdr3: %08x\n", data->tlpHdr3);
- pr_info(" tlpHdr4: %08x\n", data->tlpHdr4);
- pr_info(" sourceId: %08x\n", data->sourceId);
- pr_info(" errorClass: %016llx\n", data->errorClass);
- pr_info(" correlator: %016llx\n", data->correlator);
- pr_info(" p7iocPlssr: %016llx\n", data->p7iocPlssr);
- pr_info(" p7iocCsr: %016llx\n", data->p7iocCsr);
- pr_info(" lemFir: %016llx\n", data->lemFir);
- pr_info(" lemErrorMask: %016llx\n", data->lemErrorMask);
- pr_info(" lemWOF: %016llx\n", data->lemWOF);
- pr_info(" phbErrorStatus: %016llx\n", data->phbErrorStatus);
- pr_info(" phbFirstErrorStatus: %016llx\n", data->phbFirstErrorStatus);
- pr_info(" phbErrorLog0: %016llx\n", data->phbErrorLog0);
- pr_info(" phbErrorLog1: %016llx\n", data->phbErrorLog1);
- pr_info(" mmioErrorStatus: %016llx\n", data->mmioErrorStatus);
- pr_info(" mmioFirstErrorStatus: %016llx\n", data->mmioFirstErrorStatus);
- pr_info(" mmioErrorLog0: %016llx\n", data->mmioErrorLog0);
- pr_info(" mmioErrorLog1: %016llx\n", data->mmioErrorLog1);
- pr_info(" dma0ErrorStatus: %016llx\n", data->dma0ErrorStatus);
- pr_info(" dma0FirstErrorStatus: %016llx\n", data->dma0FirstErrorStatus);
- pr_info(" dma0ErrorLog0: %016llx\n", data->dma0ErrorLog0);
- pr_info(" dma0ErrorLog1: %016llx\n", data->dma0ErrorLog1);
- pr_info(" dma1ErrorStatus: %016llx\n", data->dma1ErrorStatus);
- pr_info(" dma1FirstErrorStatus: %016llx\n", data->dma1FirstErrorStatus);
- pr_info(" dma1ErrorLog0: %016llx\n", data->dma1ErrorLog0);
- pr_info(" dma1ErrorLog1: %016llx\n", data->dma1ErrorLog1);
+ pr_info(" brdgCtl: %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->brdgCtl));
+ pr_info(" UtlSts: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->portStatusReg),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->rootCmplxStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->busAgentStatus));
+ pr_info(" RootSts: %s %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->deviceStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->slotStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->linkStatus),
+ pnv_pci_diag_field(&buf[3 * 24], 8, data->devCmdStatus),
+ pnv_pci_diag_field(&buf[4 * 24], 8, data->devSecStatus));
+ pr_info(" RootErrSts: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->rootErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->uncorrErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->corrErrorStatus));
+ pr_info(" RootErrLog: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->tlpHdr1),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->tlpHdr2),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->tlpHdr3),
+ pnv_pci_diag_field(&buf[3 * 24], 8, data->tlpHdr4));
+ pr_info(" RootErrLog1: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->sourceId),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->errorClass),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->correlator));
+ pr_info(" PhbSts: %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->p7iocPlssr),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->p7iocCsr));
+ pr_info(" Lem: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->lemFir),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->lemErrorMask),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->lemWOF));
+ pr_info(" PhbErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->phbErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->phbFirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->phbErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->phbErrorLog1));
+ pr_info(" OutErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->mmioErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->mmioFirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->mmioErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->mmioErrorLog1));
+ pr_info(" InAErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->dma0ErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->dma0FirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->dma0ErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->dma0ErrorLog1));
+ pr_info(" InBErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->dma1ErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->dma1FirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->dma1ErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->dma1ErrorLog1));
for (i = 0; i < OPAL_P7IOC_NUM_PEST_REGS; i++) {
if ((data->pestA[i] >> 63) == 0 &&
(data->pestB[i] >> 63) == 0)
continue;
- pr_info(" PE[%3d] PESTA: %016llx\n", i, data->pestA[i]);
- pr_info(" PESTB: %016llx\n", data->pestB[i]);
+ pr_info(" PE[%3d] A/B: %s %s\n",
+ i, pnv_pci_diag_field(&buf[0], 16, data->pestA[i]),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->pestB[i]));
}
}
@@ -192,67 +228,79 @@ static void pnv_pci_dump_phb3_diag_data(struct pci_controller *hose,
struct OpalIoPhbErrorCommon *common)
{
struct OpalIoPhb3ErrorData *data;
- int i;
+ char buf[120];
+ int i = 0;
+ memset(buf, 0, 120);
data = (struct OpalIoPhb3ErrorData*)common;
pr_info("PHB3 PHB#%d Diag-data (Version: %d)\n\n",
hose->global_number, common->version);
- pr_info(" brdgCtl: %08x\n", data->brdgCtl);
-
- pr_info(" portStatusReg: %08x\n", data->portStatusReg);
- pr_info(" rootCmplxStatus: %08x\n", data->rootCmplxStatus);
- pr_info(" busAgentStatus: %08x\n", data->busAgentStatus);
-
- pr_info(" deviceStatus: %08x\n", data->deviceStatus);
- pr_info(" slotStatus: %08x\n", data->slotStatus);
- pr_info(" linkStatus: %08x\n", data->linkStatus);
- pr_info(" devCmdStatus: %08x\n", data->devCmdStatus);
- pr_info(" devSecStatus: %08x\n", data->devSecStatus);
-
- pr_info(" rootErrorStatus: %08x\n", data->rootErrorStatus);
- pr_info(" uncorrErrorStatus: %08x\n", data->uncorrErrorStatus);
- pr_info(" corrErrorStatus: %08x\n", data->corrErrorStatus);
- pr_info(" tlpHdr1: %08x\n", data->tlpHdr1);
- pr_info(" tlpHdr2: %08x\n", data->tlpHdr2);
- pr_info(" tlpHdr3: %08x\n", data->tlpHdr3);
- pr_info(" tlpHdr4: %08x\n", data->tlpHdr4);
- pr_info(" sourceId: %08x\n", data->sourceId);
- pr_info(" errorClass: %016llx\n", data->errorClass);
- pr_info(" correlator: %016llx\n", data->correlator);
-
- pr_info(" nFir: %016llx\n", data->nFir);
- pr_info(" nFirMask: %016llx\n", data->nFirMask);
- pr_info(" nFirWOF: %016llx\n", data->nFirWOF);
- pr_info(" PhbPlssr: %016llx\n", data->phbPlssr);
- pr_info(" PhbCsr: %016llx\n", data->phbCsr);
- pr_info(" lemFir: %016llx\n", data->lemFir);
- pr_info(" lemErrorMask: %016llx\n", data->lemErrorMask);
- pr_info(" lemWOF: %016llx\n", data->lemWOF);
- pr_info(" phbErrorStatus: %016llx\n", data->phbErrorStatus);
- pr_info(" phbFirstErrorStatus: %016llx\n", data->phbFirstErrorStatus);
- pr_info(" phbErrorLog0: %016llx\n", data->phbErrorLog0);
- pr_info(" phbErrorLog1: %016llx\n", data->phbErrorLog1);
- pr_info(" mmioErrorStatus: %016llx\n", data->mmioErrorStatus);
- pr_info(" mmioFirstErrorStatus: %016llx\n", data->mmioFirstErrorStatus);
- pr_info(" mmioErrorLog0: %016llx\n", data->mmioErrorLog0);
- pr_info(" mmioErrorLog1: %016llx\n", data->mmioErrorLog1);
- pr_info(" dma0ErrorStatus: %016llx\n", data->dma0ErrorStatus);
- pr_info(" dma0FirstErrorStatus: %016llx\n", data->dma0FirstErrorStatus);
- pr_info(" dma0ErrorLog0: %016llx\n", data->dma0ErrorLog0);
- pr_info(" dma0ErrorLog1: %016llx\n", data->dma0ErrorLog1);
- pr_info(" dma1ErrorStatus: %016llx\n", data->dma1ErrorStatus);
- pr_info(" dma1FirstErrorStatus: %016llx\n", data->dma1FirstErrorStatus);
- pr_info(" dma1ErrorLog0: %016llx\n", data->dma1ErrorLog0);
- pr_info(" dma1ErrorLog1: %016llx\n", data->dma1ErrorLog1);
+ pr_info(" brdgCtl: %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->brdgCtl));
+ pr_info(" UtlSts: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->portStatusReg),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->rootCmplxStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->busAgentStatus));
+ pr_info(" RootSts: %s %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->deviceStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->slotStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->linkStatus),
+ pnv_pci_diag_field(&buf[3 * 24], 8, data->devCmdStatus),
+ pnv_pci_diag_field(&buf[4 * 24], 8, data->devSecStatus));
+ pr_info(" RootErrSts: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->rootErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->uncorrErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->corrErrorStatus));
+ pr_info(" RootErrLog: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->tlpHdr1),
+ pnv_pci_diag_field(&buf[1 * 24], 8, data->tlpHdr2),
+ pnv_pci_diag_field(&buf[2 * 24], 8, data->tlpHdr3),
+ pnv_pci_diag_field(&buf[3 * 24], 8, data->tlpHdr4));
+ pr_info(" RootErrLog1: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 8, data->sourceId),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->errorClass),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->correlator));
+ pr_info(" nFir: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->nFir),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->nFirMask),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->nFirWOF));
+ pr_info(" PhbSts: %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->phbPlssr),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->phbCsr));
+ pr_info(" Lem: %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->lemFir),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->lemErrorMask),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->lemWOF));
+ pr_info(" PhbErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->phbErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->phbFirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->phbErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->phbErrorLog1));
+ pr_info(" OutErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->mmioErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->mmioFirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->mmioErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->mmioErrorLog1));
+ pr_info(" InAErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->dma0ErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->dma0FirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->dma0ErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->dma0ErrorLog1));
+ pr_info(" InBErr: %s %s %s %s\n",
+ pnv_pci_diag_field(&buf[0], 16, data->dma1ErrorStatus),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->dma1FirstErrorStatus),
+ pnv_pci_diag_field(&buf[2 * 24], 16, data->dma1ErrorLog0),
+ pnv_pci_diag_field(&buf[3 * 24], 16, data->dma1ErrorLog1));
for (i = 0; i < OPAL_PHB3_NUM_PEST_REGS; i++) {
if ((data->pestA[i] >> 63) == 0 &&
(data->pestB[i] >> 63) == 0)
continue;
- pr_info(" PE[%3d] PESTA: %016llx\n", i, data->pestA[i]);
- pr_info(" PESTB: %016llx\n", data->pestB[i]);
+ pr_info(" PE[%3d] A/B: %s %s\n",
+ i, pnv_pci_diag_field(&buf[0], 16, data->pestA[i]),
+ pnv_pci_diag_field(&buf[1 * 24], 16, data->pestB[i]));
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/5] powerpc/powernv: Remove PNV_EEH_STATE_REMOVED
From: Gavin Shan @ 2014-02-21 11:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1392983613-10090-1-git-send-email-shangw@linux.vnet.ibm.com>
The PHB state PNV_EEH_STATE_REMOVED maintained in pnv_phb isn't
so useful any more and it's duplicated to EEH_PE_ISOLATED. The
patch replaces PNV_EEH_STATE_REMOVED with EEH_PE_ISOLATED.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/eeh-ioda.c | 56 ++++++++---------------------
arch/powerpc/platforms/powernv/pci.h | 1 -
2 files changed, 15 insertions(+), 42 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index f514743..0d1d424 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -662,22 +662,6 @@ static void ioda_eeh_phb_diag(struct pci_controller *hose)
pnv_pci_dump_phb_diag_data(hose, phb->diag.blob);
}
-static int ioda_eeh_get_phb_pe(struct pci_controller *hose,
- struct eeh_pe **pe)
-{
- struct eeh_pe *phb_pe;
-
- phb_pe = eeh_phb_pe_get(hose);
- if (!phb_pe) {
- pr_warning("%s Can't find PE for PHB#%d\n",
- __func__, hose->global_number);
- return -EEXIST;
- }
-
- *pe = phb_pe;
- return 0;
-}
-
static int ioda_eeh_get_pe(struct pci_controller *hose,
u16 pe_no, struct eeh_pe **pe)
{
@@ -685,7 +669,8 @@ static int ioda_eeh_get_pe(struct pci_controller *hose,
struct eeh_dev dev;
/* Find the PHB PE */
- if (ioda_eeh_get_phb_pe(hose, &phb_pe))
+ phb_pe = eeh_phb_pe_get(hose);
+ if (!phb_pe)
return -EEXIST;
/* Find the PE according to PE# */
@@ -713,6 +698,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
{
struct pci_controller *hose;
struct pnv_phb *phb;
+ struct eeh_pe *phb_pe;
u64 frozen_pe_no;
u16 err_type, severity;
long rc;
@@ -729,10 +715,12 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
list_for_each_entry(hose, &hose_list, list_node) {
/*
* If the subordinate PCI buses of the PHB has been
- * removed, we needn't take care of it any more.
+ * removed or is exactly under error recovery, we
+ * needn't take care of it any more.
*/
phb = hose->private_data;
- if (phb->eeh_state & PNV_EEH_STATE_REMOVED)
+ phb_pe = eeh_phb_pe_get(hose);
+ if (!phb_pe || (phb_pe->state & EEH_PE_ISOLATED))
continue;
rc = opal_pci_next_error(phb->opal_id,
@@ -765,12 +753,6 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
switch (err_type) {
case OPAL_EEH_IOC_ERROR:
if (severity == OPAL_EEH_SEV_IOC_DEAD) {
- list_for_each_entry(hose, &hose_list,
- list_node) {
- phb = hose->private_data;
- phb->eeh_state |= PNV_EEH_STATE_REMOVED;
- }
-
pr_err("EEH: dead IOC detected\n");
ret = EEH_NEXT_ERR_DEAD_IOC;
} else if (severity == OPAL_EEH_SEV_INF) {
@@ -783,17 +765,12 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
break;
case OPAL_EEH_PHB_ERROR:
if (severity == OPAL_EEH_SEV_PHB_DEAD) {
- if (ioda_eeh_get_phb_pe(hose, pe))
- break;
-
+ *pe = phb_pe;
pr_err("EEH: dead PHB#%x detected\n",
hose->global_number);
- phb->eeh_state |= PNV_EEH_STATE_REMOVED;
ret = EEH_NEXT_ERR_DEAD_PHB;
} else if (severity == OPAL_EEH_SEV_PHB_FENCED) {
- if (ioda_eeh_get_phb_pe(hose, pe))
- break;
-
+ *pe = phb_pe;
pr_err("EEH: fenced PHB#%x detected\n",
hose->global_number);
ret = EEH_NEXT_ERR_FENCED_PHB;
@@ -813,15 +790,12 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
* fenced PHB so that it can be recovered.
*/
if (ioda_eeh_get_pe(hose, frozen_pe_no, pe)) {
- if (!ioda_eeh_get_phb_pe(hose, pe)) {
- pr_err("EEH: Escalated fenced PHB#%x "
- "detected for PE#%llx\n",
- hose->global_number,
- frozen_pe_no);
- ret = EEH_NEXT_ERR_FENCED_PHB;
- } else {
- ret = EEH_NEXT_ERR_NONE;
- }
+ *pe = phb_pe;
+ pr_err("EEH: Escalated fenced PHB#%x "
+ "detected for PE#%llx\n",
+ hose->global_number,
+ frozen_pe_no);
+ ret = EEH_NEXT_ERR_FENCED_PHB;
} else {
pr_err("EEH: Frozen PE#%x on PHB#%x detected\n",
(*pe)->addr, (*pe)->phb->global_number);
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 13f1942..dbeba3d 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -81,7 +81,6 @@ struct pnv_eeh_ops {
};
#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */
-#define PNV_EEH_STATE_REMOVED (1 << 1) /* PHB removed */
#endif /* CONFIG_EEH */
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/5] powerpc/eeh: Remove EEH_PE_PHB_DEAD
From: Gavin Shan @ 2014-02-21 11:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
The PE state (for eeh_pe instance) EEH_PE_PHB_DEAD is duplicate to
EEH_PE_ISOLATED. Originally, those PHB (PHB PE) with EEH_PE_PHB_DEAD
would be removed from the system. However, it's safe to replace
that with EEH_PE_ISOLATED.
The patch also clear EEH_PE_RECOVERING after fenced PHB has been handled,
either failure or success. It makes the PHB PE state consistent with:
PHB functions normally NONE
PHB has been removed EEH_PE_ISOLATED
PHB fenced, recovery in progress EEH_PE_ISOLATED | RECOVERING
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/kernel/eeh.c | 10 ++--------
arch/powerpc/kernel/eeh_driver.c | 10 +++++-----
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index d4dd41f..a61b06f 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -53,7 +53,6 @@ struct device_node;
#define EEH_PE_ISOLATED (1 << 0) /* Isolated PE */
#define EEH_PE_RECOVERING (1 << 1) /* Recovering PE */
-#define EEH_PE_PHB_DEAD (1 << 2) /* Dead PHB */
#define EEH_PE_KEEP (1 << 8) /* Keep PE on hotplug */
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index e7b76a6..f167676 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -232,7 +232,6 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
{
size_t loglen = 0;
struct eeh_dev *edev, *tmp;
- bool valid_cfg_log = true;
/*
* When the PHB is fenced or dead, it's pointless to collect
@@ -240,12 +239,7 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
* 0xFF's. For ER, we still retrieve the data from the PCI
* config space.
*/
- if (eeh_probe_mode_dev() &&
- (pe->type & EEH_PE_PHB) &&
- (pe->state & (EEH_PE_ISOLATED | EEH_PE_PHB_DEAD)))
- valid_cfg_log = false;
-
- if (valid_cfg_log) {
+ if (!(pe->type & EEH_PE_PHB)) {
eeh_pci_enable(pe, EEH_OPT_THAW_MMIO);
eeh_ops->configure_bridge(pe);
eeh_pe_restore_bars(pe);
@@ -309,7 +303,7 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
/* If the PHB has been in problematic state */
eeh_serialize_lock(&flags);
- if (phb_pe->state & (EEH_PE_ISOLATED | EEH_PE_PHB_DEAD)) {
+ if (phb_pe->state & EEH_PE_ISOLATED) {
ret = 0;
goto out;
}
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 7bb30dc..5f50f9c 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -661,8 +661,7 @@ static void eeh_handle_special_event(void)
phb_pe = eeh_phb_pe_get(hose);
if (!phb_pe) continue;
- eeh_pe_state_mark(phb_pe,
- EEH_PE_ISOLATED | EEH_PE_PHB_DEAD);
+ eeh_pe_state_mark(phb_pe, EEH_PE_ISOLATED);
}
eeh_serialize_unlock(flags);
@@ -678,8 +677,7 @@ static void eeh_handle_special_event(void)
eeh_remove_event(pe);
if (rc == EEH_NEXT_ERR_DEAD_PHB)
- eeh_pe_state_mark(pe,
- EEH_PE_ISOLATED | EEH_PE_PHB_DEAD);
+ eeh_pe_state_mark(pe, EEH_PE_ISOLATED);
else
eeh_pe_state_mark(pe,
EEH_PE_ISOLATED | EEH_PE_RECOVERING);
@@ -703,12 +701,14 @@ static void eeh_handle_special_event(void)
if (rc == EEH_NEXT_ERR_FROZEN_PE ||
rc == EEH_NEXT_ERR_FENCED_PHB) {
eeh_handle_normal_event(pe);
+ eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
} else {
pci_lock_rescan_remove();
list_for_each_entry(hose, &hose_list, list_node) {
phb_pe = eeh_phb_pe_get(hose);
if (!phb_pe ||
- !(phb_pe->state & EEH_PE_PHB_DEAD))
+ !(phb_pe->state & EEH_PE_ISOLATED) ||
+ (phb_pe->state & EEH_PE_RECOVERING))
continue;
/* Notify all devices to be down */
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 4/7] ECHI Platform: Merge ppc-of EHCI driver into the ehci-platform driver
From: Mark Rutland @ 2014-02-21 11:48 UTC (permalink / raw)
To: Alistair Popple
Cc: devicetree@vger.kernel.org, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Prisk, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1392964293-13687-5-git-send-email-alistair@popple.id.au>
[Adding Tony Prisk to Cc]
On Fri, Feb 21, 2014 at 06:31:30AM +0000, Alistair Popple wrote:
> Currently the ppc-of driver uses the compatibility string
> "usb-ehci". This means platforms that use device-tree and implement an
> EHCI compatible interface have to either use the ppc-of driver or add
> a compatible line to the ehci-platform driver. It would be more
> appropriate for the platform driver to be compatible with "usb-ehci"
> as non-powerpc platforms are also beginning to utilise device-tree.
>
> This patch merges the device tree property parsing from ehci-ppc-of
> into the platform driver and adds a "usb-ehci" compatibility
> string. The existing ehci-ppc-of driver is removed and the 440EPX
> specific quirks are added to the ehci-platform driver.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
> drivers/usb/host/Kconfig | 7 +-
> drivers/usb/host/ehci-hcd.c | 5 -
> drivers/usb/host/ehci-platform.c | 87 +++++++++++++-
> drivers/usb/host/ehci-ppc-of.c | 238 --------------------------------------
> 4 files changed, 89 insertions(+), 248 deletions(-)
> delete mode 100644 drivers/usb/host/ehci-ppc-of.c
[...]
> + /* Device tree properties if available will override platform data. */
> + dn = hcd_to_bus(hcd)->controller->of_node;
> + if (dn) {
> + if (of_get_property(dn, "big-endian", NULL)) {
> + ehci->big_endian_mmio = 1;
> + ehci->big_endian_desc = 1;
> + }
> + if (of_get_property(dn, "big-endian-regs", NULL))
> + ehci->big_endian_mmio = 1;
> + if (of_get_property(dn, "big-endian-desc", NULL))
> + ehci->big_endian_desc = 1;
> + }
Please use of_property_read_bool for these.
This driver already handles "via,vt8500-ehci" and "wm,prizm-ehci" which
aren't documented to handle these properties, but now gain support for
them. It might be worth unifying the binding documents if there's
nothing special about those two host controllers.
We seem to have two binding documents for "via,vt8500-ehci", so some
cleanup is definitely in order.
Tony, you seem to have written both documents judging by 95e9fd10f06c
and 8ad551d150e3. Do you have any issue with merging both of these into
a common usb-ehci document?
> +
> + np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx");
> + if (np != NULL) {
> + /* claim we really affected by usb23 erratum */
> + if (!of_address_to_resource(np, 0, &res))
> + ehci->ohci_hcctrl_reg =
> + devm_ioremap(&pdev->dev,
> + res.start + OHCI_HCCTRL_OFFSET,
> + OHCI_HCCTRL_LEN);
> + else
> + ehci_dbg(ehci, "%s: no ohci offset in fdt\n", __FILE__);
> + if (!ehci->ohci_hcctrl_reg) {
> + ehci_dbg(ehci, "%s: ioremap for ohci hcctrl failed\n",
> + __FILE__);
> + } else {
> + ehci->has_amcc_usb23 = 1;
> + }
> + }
As this is being dropped into a generic driver, it would be nice to have
a comment explaining why we need to do this -- not everyone using this
will know the powerpc history. It certainly seems odd to look for
another node in the tree that this driver isn't necessarily handling,
and that should probably be explained.
As this bit of fixup is only needed for powerpc, it would be nice to not
have to do it elsewhere. Perhaps these could be factored out into a
ppc_platform_reset function that could be empty stub for other
architectures.
> +
> + if (of_device_is_compatible(dn, "ibm,usb-ehci-440epx")) {
> + retval = ppc44x_enable_bmt(dn);
> + ehci_dbg(ehci, "Break Memory Transfer (BMT) is %senabled!\n",
> + retval ? "NOT " : "");
> + }
> +
Likewise.
[...]
> + /* use request_mem_region to test if the ohci driver is loaded. if so
> + * ensure the ohci core is operational.
> + */
Nit: comment code style (should have a newline after the /* according to
Documentation/CodingStyle).
> + if (ehci->has_amcc_usb23) {
> + np = of_find_compatible_node(NULL, NULL, "ibm,usb-ohci-440epx");
> + if (np != NULL) {
> + if (!of_address_to_resource(np, 0, &res))
> + if (!request_mem_region(res.start,
> + 0x4, hcd_name))
> + set_ohci_hcfs(ehci, 1);
> + else
> + release_mem_region(res.start, 0x4);
> + else
> + ehci_dbg(ehci, "%s: no ohci offset in fdt\n",
> + __FILE__);
> + of_node_put(np);
> + }
> + }
As with the earlier comment, this looks a bit odd. A comment explaining
why we're looking for another node would help. Also, we should only need
this for powerpc.
Cheers,
Mark.
^ permalink raw reply
* Re: [PATCH 2/7] IBM Akebono: Add support for a new PHY interface to the IBM emac driver
From: Mark Rutland @ 2014-02-21 11:18 UTC (permalink / raw)
To: Alistair Popple
Cc: netdev@vger.kernel.org, David S. Miller,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <1392964293-13687-3-git-send-email-alistair@popple.id.au>
On Fri, Feb 21, 2014 at 06:31:28AM +0000, Alistair Popple wrote:
> The IBM PPC476GTR SoC that is used on the Akebono board uses a
> different ethernet PHY interface that has wake on lan (WOL) support
> with the IBM emac. This patch adds support to the IBM emac driver for
> this new PHY interface.
>
> At this stage the wake on lan functionality has not been implemented.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
[...]
> + /* Check for RGMII flags */
> + if (of_get_property(ofdev->dev.of_node, "has-mdio", NULL))
> + dev->flags |= EMAC_RGMII_FLAG_HAS_MDIO;
You can use of_property_read_bool here.
Cheers,
Mark.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox