* Deprecating reserve-map in favor of properties
From: Benjamin Herrenschmidt @ 2012-09-20 22:10 UTC (permalink / raw)
To: devicetree-discuss; +Cc: linuxppc-dev
Hi folks !
The reserve map is, imho, my biggest mistake when coming up with the FDT
format.
The main problem is that it doesn't survive the transition via a real
Open Firmware interface. There is no practical way to indicate reserved
regions of memory accross in that case, unless you have an OS that is
nice enough to try to keep OF alive and accomodate its advertised
"available" properties, but that's typically not the case of Linux on
ppc.
So I would like to propose that we add a new "better" way to convey
reserved memory information, via properties in the tree.
I originally though of having these in the "memory" nodes themselves but
this can be tricky on machines that have multiple nodes (ie, NUMA
generally means a memory node per NUMA node) since the reserved regions
can spawn accross nodes and I don't want to complicate FW life too much
by requiring breaking them up in that case.
So what about something at the root of the tree:
reserved-ranges: An array of ranges of reserved memory
reserved-names: A list of zero terminated strings, one for each entry in
the reserved-ranges array, providing optional "names" for the reserved
ranges.
The idea here is that we could have well defined names (using a similar
prefix we use for properties) such as linux,initrd, which indicates
clearly to the kernel that the only reason that range is reserved is
because it contains an initrd (ie, it can be freed once that's been
extracted).
It would also be generally handy in case a reserved area is meant to be
used by a specific driver, such as an in-memory framebuffer
pre-initialized by the firmware. The generic memory management code
doesn't need to know, but later on, the gfx driver can pick it up easily
provided the name is part of the binding for that device.
Any objection ? If none, I'll cook up a patch to add support for it (at
least on powerpc :-)
Cheers,
Ben.
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Stephen Rothwell @ 2012-09-20 22:15 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
Cc: netdev, linux-next, mika.westerberg, David Miller, linux-kernel
In-Reply-To: <20120920.164558.2281417114805373564.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
[Just bring this to the attention of the PowerPC folks ...]
On Thu, 20 Sep 2012 16:45:58 -0400 (EDT) David Miller <davem@davemloft.net> wrote:
>
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> Date: Thu, 20 Sep 2012 12:10:14 +0300
>
> > On Thu, Sep 20, 2012 at 05:36:22PM +1000, Stephen Rothwell wrote:
> >> Hi all,
> >>
> >> After merging the final tree, today's linux-next build (powerpc
> >> allyesconfig) failed like this:
> >>
> >> drivers/net/ethernet/i825xx/znet.c: In function 'hardware_init':
> >> drivers/net/ethernet/i825xx/znet.c:868:2: error: implicit declaration of function 'isa_virt_to_bus' [-Werror=implicit-function-declaration]
> >>
> >> Caused by commit 1d3ff76759b7 ("i825xx: znet: fix compiler warnings when
> >> building a 64-bit kernel"). Is there some Kconfig dependency missing (CONFIG_ISA)?
> >
> > If we make it dependent on CONFIG_ISA then the driver cannot be built with
> > 64-bit kernel. Then again is there someone running 64-bit kernel on Zenith
> > Z-note notebook? From the pictures it looks like very ancient "laptop".
> >
> > An alternative is to make it depend on X86 like this:
>
> I think the powerpc port is at fault here.
>
> Part of being able to advertise ISA_DMA_API is providing isa_virt_to_bus().
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Benjamin Herrenschmidt @ 2012-09-20 22:22 UTC (permalink / raw)
To: Stephen Rothwell
Cc: mika.westerberg, netdev, linux-kernel, linux-next, Paul Mackerras,
linuxppc-dev, David Miller
In-Reply-To: <20120921081531.67db8a7d0ac1b35426b22c45@canb.auug.org.au>
> > I think the powerpc port is at fault here.
> >
> > Part of being able to advertise ISA_DMA_API is providing isa_virt_to_bus().
Hrm, that's ancient gunk, I'll have to dig. We potentially can support
ISA devices DMA'ing from an ISA bridge... but via the iommu, which means
isa_virt_to_bus is a non-starter.
But then, do we really care ? IE. Is there single device that actually
requires ISA_DMA_API and that is expected to work on any currently
supported powerpc hw ? :-)
We don't even support PReP anymore, so that leaves us with what ?
Anybody has an objection to turning ISA_DMA_API off ?
Cheers,
Ben.
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: Stephen Rothwell @ 2012-09-20 22:28 UTC (permalink / raw)
To: David Miller
Cc: netdev, linuxppc-dev, linux-kernel, linux-next, Paul Mackerras,
mika.westerberg
In-Reply-To: <20120920.164558.2281417114805373564.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 563 bytes --]
Hi Dave,
On Thu, 20 Sep 2012 16:45:58 -0400 (EDT) David Miller <davem@davemloft.net> wrote:
>
> I think the powerpc port is at fault here.
>
> Part of being able to advertise ISA_DMA_API is providing isa_virt_to_bus().
Not disagreeing, but it would be nice if this was documented somewhere
(maybe in Documentation/DMA-ISA-LPC.txt). We have not had this problem
before because all the other uses of isa_virt_to_bus() are in drivers
that depend on X86 or ARM or ISA or EISA.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (net-next tree related)
From: David Miller @ 2012-09-20 22:53 UTC (permalink / raw)
To: benh
Cc: sfr, mika.westerberg, netdev, linux-kernel, linux-next, paulus,
linuxppc-dev
In-Reply-To: <1348179764.1132.35.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Fri, 21 Sep 2012 08:22:44 +1000
> Hrm, that's ancient gunk, I'll have to dig. We potentially can support
> ISA devices DMA'ing from an ISA bridge... but via the iommu, which means
> isa_virt_to_bus is a non-starter.
>
> But then, do we really care ? IE. Is there single device that actually
> requires ISA_DMA_API and that is expected to work on any currently
> supported powerpc hw ? :-)
>
> We don't even support PReP anymore, so that leaves us with what ?
ISA_DMA_API implies a fixed window of addresses which are <= 32-bits
on the bus, which is a hardware requirement of these devices.
isa_virt_to_bus() goes to that physical address, and the expection is
that you use GFP_DMA and thus the physical addresses fit inside of
an unsigned int.
isa_virt_to_bus() basically amounts to a virt-->phys plus a cast.
> Anybody has an objection to turning ISA_DMA_API off ?
Then you can remove all of the DMA api stuff in powerpc's asm/dma.h
but some of it looks like it might be in use.
^ permalink raw reply
* Re: [PATCH] Use pmc_overflow() to detect rolled back events
From: Sukadev Bhattiprolu @ 2012-09-21 0:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Maynard Johnson, Anton Blanchard, linuxppc-dev
In-Reply-To: <20120808010719.GA32730@us.ibm.com>
Sukadev Bhattiprolu [sukadev@linux.vnet.ibm.com] wrote:
|
| >From 21e9d1775f0c6f37a39e5d682ff74693fa9a4004 Mon Sep 17 00:00:00 2001
| From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| Date: Tue, 7 Aug 2012 17:53:24 -0700
| Subject: [PATCH] Use pmc_overflow to detect rolled back events.
|
| For certain speculative events on Power7, 'perf stat' reports far higher
| event count than 'perf record' for the same event.
|
| As described in following commit, a performance monitor exception is raised
| even when the the performance events are rolled back.
|
| commit 0837e3242c73566fc1c0196b4ec61779c25ffc93
| Author: Anton Blanchard <anton@samba.org>
| Date: Wed Mar 9 14:38:42 2011 +1100
|
| perf_event_interrupt() records an event only when an overflow occurs. But
| this check for overflow is a simple 'if (val < 0)'.
|
| Because the events are rolled back, this check for overflow fails and the
| event is not recorded. perf_event_interrupt() later uses pmc_overflow() to
| detect the overflow and resets the counters and the events are lost completely.
|
| To properly detect the overflow of rolled back events, use pmc_overflow()
| even when recording events.
Ben,
Sorry for the noise, but please revert this patch (following commit):
commit 813312110bede27bffd082c25cd31730bd567beb
Author: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Tue Aug 7 15:07:19 2012 +0000
While it does fix the problem described above and works for the limit-pmc
events, it seems to break on Power7 for other events and when the sample
period is low.
I am still investigating the problem and will follow up with a separate
mail.
Sukadev
^ permalink raw reply
* Re: Deprecating reserve-map in favor of properties
From: Kumar Gala @ 2012-09-21 1:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <1348179051.1132.32.camel@pasglop>
On Sep 20, 2012, at 5:10 PM, Benjamin Herrenschmidt wrote:
> Hi folks !
>=20
> The reserve map is, imho, my biggest mistake when coming up with the =
FDT
> format.
>=20
> The main problem is that it doesn't survive the transition via a real
> Open Firmware interface. There is no practical way to indicate =
reserved
> regions of memory accross in that case, unless you have an OS that is
> nice enough to try to keep OF alive and accomodate its advertised
> "available" properties, but that's typically not the case of Linux on
> ppc.
>=20
> So I would like to propose that we add a new "better" way to convey
> reserved memory information, via properties in the tree.
>=20
> I originally though of having these in the "memory" nodes themselves =
but
> this can be tricky on machines that have multiple nodes (ie, NUMA
> generally means a memory node per NUMA node) since the reserved =
regions
> can spawn accross nodes and I don't want to complicate FW life too =
much
> by requiring breaking them up in that case.
>=20
> So what about something at the root of the tree:
>=20
> reserved-ranges: An array of ranges of reserved memory
>=20
> reserved-names: A list of zero terminated strings, one for each entry =
in
> the reserved-ranges array, providing optional "names" for the reserved
> ranges.
>=20
> The idea here is that we could have well defined names (using a =
similar
> prefix we use for properties) such as linux,initrd, which indicates
> clearly to the kernel that the only reason that range is reserved is
> because it contains an initrd (ie, it can be freed once that's been
> extracted).
>=20
> It would also be generally handy in case a reserved area is meant to =
be
> used by a specific driver, such as an in-memory framebuffer
> pre-initialized by the firmware. The generic memory management code
> doesn't need to know, but later on, the gfx driver can pick it up =
easily
> provided the name is part of the binding for that device.
>=20
> Any objection ? If none, I'll cook up a patch to add support for it =
(at
> least on powerpc :-)
>=20
> Cheers,
> Ben.
If you do this, please update the code in dtc/libfdt to construct the =
new nodes. We use this in u-boot to reserve kernel, dtb, initrd, etc =
regions. So would be nice to have drop in replacement code that could =
use same APIs if possible.
- k=
^ permalink raw reply
* Re: Deprecating reserve-map in favor of properties
From: Benjamin Herrenschmidt @ 2012-09-21 2:13 UTC (permalink / raw)
To: Kumar Gala; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <8C5A5EB8-6CD7-4E35-AACF-A2CC04CDBE48@kernel.crashing.org>
On Thu, 2012-09-20 at 20:35 -0500, Kumar Gala wrote:
> If you do this, please update the code in dtc/libfdt to construct the
> new nodes. We use this in u-boot to reserve kernel, dtb, initrd, etc
> regions. So would be nice to have drop in replacement code that could
> use same APIs if possible.
The kernel would of course still understand the reserve map and I don't
intend to remove it from the header immediately, so I think that can
stay.... unless we make it a function of the version.
It's non-trivial to make the same (stateless) API in libfdt deal with
setting properties. I'd rather avoid that problem initially by not
changing the blob format, keeping the reserve map around for "legacy"
purposes and simply making the kernel capable of understanding the
properties.
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM support
From: Jia Hongtao-B38951 @ 2012-09-21 3:13 UTC (permalink / raw)
To: Kumar Gala
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <8086D08B-62B7-4838-8DB6-0F6158DE581A@kernel.crashing.org>
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Wednesday, September 19, 2012 11:49 PM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> Subject: Re: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
>=20
> >>> On Sep 17, 2012, at 9:10 PM, Jia Hongtao wrote:
> >>>
> >>>> Power supply for PCI inbound/outbound window registers is off when
> >>>> system go to deep-sleep state. We save the values of registers
> before
> >>>> suspend and restore to registers after resume.
> >>>>
> >>>> Signed-off-by: Jiang Yutang <b14898@freescale.com>
> >>>> Signed-off-by: Jia Hongtao <B38951@freescale.com>
> >>>> Signed-off-by: Li Yang <leoli@freescale.com>
> >>>> ---
> >>>> Changes for V4:
> >>>> We just rebase the patch upon following patch:
> >>>> powerpc/fsl-pci: Unify pci/pcie initialization code
> >>>>
> >>>> arch/powerpc/include/asm/pci-bridge.h | 2 +-
> >>>> arch/powerpc/sysdev/fsl_pci.c | 121
> >>> +++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 122 insertions(+), 1 deletions(-)
> >>>
> >>> Did you ever compare this to just re-parsing device tree method?
> >>>
> >>> - k
> >>
> >> I tested the re-parsing way by using setup_pci_atmu() when resume.
> >> And I found out that re-parsing will *change* outbound IO translation
> >> address regitster.
> >>
> >> It seems that in the first bootup, after setup_atmu()
> >> pcibios_setup_phb_resources() may update hose->io_resource, but atmu
> >> is not updated according to the new hose->io_resource value.
> >> In resume from sleep setup_atmu() will reset atmu according to the
> >> new hose->io_resource value. So the setup_atmu() will cause different
> >> result on outbound IO register between first bootup and resume from
> >> sleep.
> >>
> >> So... There's a possibility that in the first bootup atmu is not setup
> >> properly.
> >
> > [Are you seeing this happen in your testing? If so its a bug we need
> to look at fixing.]
> >
> > Yes, I see this in my testing.
> > Also PCIe ethernet card works well after resuming from sleep in both
> save/restore
> > and re-parsing way. (Maybe PCIe ethernet card don't need outbound IO
> resource)
> > So, I guess the result of re-parsing (actually it's re-setup) is right
> and ATMU is not setup
> > properly at the first bootup.
>=20
> Are you seeing the following message - "PCI: I/O resource not set for
> host bridge" ?
No.
>=20
> Trying to understand why you'd hit the reassignment of io_resource.
>=20
> - k
>=20
I did some investigations and the conclusion is:
io_resource.flags & IORESOURCE_IO are both positive but io_resource.start
is 0 before pcibios_setup_phb_io_space() is done.
The sequence of related process listed below:
fsl_add_bridge() -> setup_pci_atmu()
pcibios_init() -> pcibios_scan_phb() -> pcibios_setup_phb_io_space()
Because fsl_add_bridge() must be finished before pcibios_init() so ATMU
is set when io_resource.start is 0. That means outbound IO regs are not
set.
If system re-setup ATMU the io_resource.start has already updated so
outbound IO regs are set.
My question is:
Is there any problem if outbound IO regs are not set in first bootup?
- Hongtao.
^ permalink raw reply
* Re: [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode
From: Lian Minghaun-b31939 @ 2012-09-21 3:37 UTC (permalink / raw)
To: Kumar Gala; +Cc: Minghuan Lian, linuxppc-dev
In-Reply-To: <4FA47D63-B0ED-42BF-A075-174B6338A0E8@kernel.crashing.org>
Hi Kumar,
please see my comments inline.
On 09/19/2012 10:22 PM, Kumar Gala wrote:
> On Sep 19, 2012, at 2:23 AM, Minghuan Lian wrote:
>
>> The original code uses 'Programming Interface' field to judge if PCIE is
>> EP or RC mode, however, some latest silicons do not support this functionality.
>> According to PCIE specification, 'Header Type' offset 0x0e is used to
>> indicate header type, so change code to use 'Header Type' field to
>> judge PCIE mode. Because FSL PCI controller does not support 'Header Type',
>> patch still uses 'Programming Interface' to identify PCI mode.
>>
>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
>> ---
>> arch/powerpc/sysdev/fsl_pci.c | 38 +++++++++++++++++++++++---------------
>> 1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
>> index c37f461..43d30df 100644
>> --- a/arch/powerpc/sysdev/fsl_pci.c
>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>> @@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
>>
>> static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
>> {
>> - u8 progif;
>> + u8 hdr_type;
>>
>> /* if we aren't a PCIe don't bother */
>> if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
>> return;
>>
>> /* if we aren't in host mode don't bother */
>> - pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
>> - if (progif & 0x1)
>> + pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
>> + if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
>> return;
>>
>> dev->class = PCI_CLASS_BRIDGE_PCI << 8;
>> @@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>> struct pci_controller *hose;
>> struct resource rsrc;
>> const int *bus_range;
>> - u8 progif;
>> + u8 hdr_type, progif;
>>
>> if (!of_device_is_available(dev)) {
>> pr_warning("%s: disabled\n", dev->full_name);
>> @@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>> setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
>> PPC_INDIRECT_TYPE_BIG_ENDIAN);
>>
>> - early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>> - if ((progif & 1) == 1) {
>> - /* unmap cfg_data & cfg_addr separately if not on same page */
>> - if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
>> - ((unsigned long)hose->cfg_addr & PAGE_MASK))
>> - iounmap(hose->cfg_data);
>> - iounmap(hose->cfg_addr);
>> - pcibios_free_controller(hose);
>> - return -ENODEV;
>> - }
>> -
>> setup_pci_cmd(hose);
> I think we should be doing the check before we call setup_pci_cmd(). The old code didn't touch the controller registers if we where and end-point. We should maintain that.
[Minghuan] Thanks for you pointing this.
I want to move setup_pci_cmd like this:
pr_debug(" ->Hose at 0x%p, cfg_addr=0x%p,cfg_data=0x%p\n",
hose, hose->cfg_addr, hose->cfg_data);
+ setup_pci_cmd(hose);
/* Interpret the "ranges" property */
/* This also maps the I/O region and sets isa_io/mem_base */
pci_process_bridge_OF_ranges(hose, dev, is_primary);
This movement will cause fsl_pcie_check_link() calling before
setup_pci_cmd().
Is this ok?
If not, I will call setup_pci_cmd() for PCI and PCIE respectively.
>> /* check PCI express link status */
>> if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
>> + /* For PCIE read HEADER_TYPE to identify controler mode */
>> + early_read_config_byte(hose, 0, 0, PCI_HEADER_TYPE, &hdr_type);
>> + if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
>> + goto no_bridge;
>> +
>> hose->indirect_type |= PPC_INDIRECT_TYPE_EXT_REG |
>> PPC_INDIRECT_TYPE_SURPRESS_PRIMARY_BUS;
>> if (fsl_pcie_check_link(hose))
>> hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
>> + } else {
>> + /* For PCI read PROG to identify controller mode */
>> + early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>> + if ((progif & 1) == 1)
>> + goto no_bridge;
>> }
>>
>> printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
>> @@ -494,6 +493,15 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>> setup_pci_atmu(hose, &rsrc);
>>
>> return 0;
>> +
>> +no_bridge:
>> + /* unmap cfg_data & cfg_addr separately if not on same page */
>> + if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
>> + ((unsigned long)hose->cfg_addr & PAGE_MASK))
>> + iounmap(hose->cfg_data);
>> + iounmap(hose->cfg_addr);
>> + pcibios_free_controller(hose);
>> + return -ENODEV;
>> }
>> #endif /* CONFIG_FSL_SOC_BOOKE || CONFIG_PPC_86xx */
>>
>> --
>> 1.7.9.5
>>
>
^ permalink raw reply
* RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM support
From: Li Yang-R58472 @ 2012-09-21 3:50 UTC (permalink / raw)
To: Jia Hongtao-B38951, Kumar Gala
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01AB4A6F@039-SN1MPN1-004.039d.mgd.msft.net>
> -----Original Message-----
> From: Jia Hongtao-B38951
> Sent: Friday, September 21, 2012 11:14 AM
> To: Kumar Gala
> Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
>=20
>=20
>=20
> > -----Original Message-----
> > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > Sent: Wednesday, September 19, 2012 11:49 PM
> > To: Jia Hongtao-B38951
> > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> > Subject: Re: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> > support
> >
> > >>> On Sep 17, 2012, at 9:10 PM, Jia Hongtao wrote:
> > >>>
> > >>>> Power supply for PCI inbound/outbound window registers is off
> > >>>> when system go to deep-sleep state. We save the values of
> > >>>> registers
> > before
> > >>>> suspend and restore to registers after resume.
> > >>>>
> > >>>> Signed-off-by: Jiang Yutang <b14898@freescale.com>
> > >>>> Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > >>>> Signed-off-by: Li Yang <leoli@freescale.com>
> > >>>> ---
> > >>>> Changes for V4:
> > >>>> We just rebase the patch upon following patch:
> > >>>> powerpc/fsl-pci: Unify pci/pcie initialization code
> > >>>>
> > >>>> arch/powerpc/include/asm/pci-bridge.h | 2 +-
> > >>>> arch/powerpc/sysdev/fsl_pci.c | 121
> > >>> +++++++++++++++++++++++++++++++++
> > >>>> 2 files changed, 122 insertions(+), 1 deletions(-)
> > >>>
> > >>> Did you ever compare this to just re-parsing device tree method?
> > >>>
> > >>> - k
> > >>
> > >> I tested the re-parsing way by using setup_pci_atmu() when resume.
> > >> And I found out that re-parsing will *change* outbound IO
> > >> translation address regitster.
> > >>
> > >> It seems that in the first bootup, after setup_atmu()
> > >> pcibios_setup_phb_resources() may update hose->io_resource, but
> > >> atmu is not updated according to the new hose->io_resource value.
> > >> In resume from sleep setup_atmu() will reset atmu according to the
> > >> new hose->io_resource value. So the setup_atmu() will cause
> > >> different result on outbound IO register between first bootup and
> > >> resume from sleep.
> > >>
> > >> So... There's a possibility that in the first bootup atmu is not
> > >> setup properly.
> > >
> > > [Are you seeing this happen in your testing? If so its a bug we
> > > need
> > to look at fixing.]
> > >
> > > Yes, I see this in my testing.
> > > Also PCIe ethernet card works well after resuming from sleep in both
> > save/restore
> > > and re-parsing way. (Maybe PCIe ethernet card don't need outbound IO
> > resource)
> > > So, I guess the result of re-parsing (actually it's re-setup) is
> > > right
> > and ATMU is not setup
> > > properly at the first bootup.
> >
> > Are you seeing the following message - "PCI: I/O resource not set for
> > host bridge" ?
>=20
> No.
>=20
> >
> > Trying to understand why you'd hit the reassignment of io_resource.
> >
> > - k
> >
>=20
> I did some investigations and the conclusion is:
>=20
> io_resource.flags & IORESOURCE_IO are both positive but io_resource.start
> is 0 before pcibios_setup_phb_io_space() is done.
>=20
> The sequence of related process listed below:
> fsl_add_bridge() -> setup_pci_atmu()
> pcibios_init() -> pcibios_scan_phb() -> pcibios_setup_phb_io_space()
>=20
> Because fsl_add_bridge() must be finished before pcibios_init() so ATMU
> is set when io_resource.start is 0. That means outbound IO regs are not
> set.
>=20
> If system re-setup ATMU the io_resource.start has already updated so
> outbound IO regs are set.
>=20
> My question is:
> Is there any problem if outbound IO regs are not set in first bootup?
Please also provide the IO resource address range before and after the pci =
scan. Then we can evaluate if the range is needed to be mapped via ATMU.
Leo
^ permalink raw reply
* RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM support
From: Jia Hongtao-B38951 @ 2012-09-21 5:15 UTC (permalink / raw)
To: Li Yang-R58472, Kumar Gala
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F09432A5F@039-SN2MPN1-021.039d.mgd.msft.net>
> -----Original Message-----
> From: Li Yang-R58472
> Sent: Friday, September 21, 2012 11:51 AM
> To: Jia Hongtao-B38951; Kumar Gala
> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421
> Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> support
>=20
>=20
>=20
> > -----Original Message-----
> > From: Jia Hongtao-B38951
> > Sent: Friday, September 21, 2012 11:14 AM
> > To: Kumar Gala
> > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> > Subject: RE: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM
> > support
> >
> >
> >
> > > -----Original Message-----
> > > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > > Sent: Wednesday, September 19, 2012 11:49 PM
> > > To: Jia Hongtao-B38951
> > > Cc: linuxppc-dev@lists.ozlabs.org; Li Yang-R58472; Wood Scott-B07421
> > > Subject: Re: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound
> > > PM support
> > >
> > > >>> On Sep 17, 2012, at 9:10 PM, Jia Hongtao wrote:
> > > >>>
> > > >>>> Power supply for PCI inbound/outbound window registers is off
> > > >>>> when system go to deep-sleep state. We save the values of
> > > >>>> registers
> > > before
> > > >>>> suspend and restore to registers after resume.
> > > >>>>
> > > >>>> Signed-off-by: Jiang Yutang <b14898@freescale.com>
> > > >>>> Signed-off-by: Jia Hongtao <B38951@freescale.com>
> > > >>>> Signed-off-by: Li Yang <leoli@freescale.com>
> > > >>>> ---
> > > >>>> Changes for V4:
> > > >>>> We just rebase the patch upon following patch:
> > > >>>> powerpc/fsl-pci: Unify pci/pcie initialization code
> > > >>>>
> > > >>>> arch/powerpc/include/asm/pci-bridge.h | 2 +-
> > > >>>> arch/powerpc/sysdev/fsl_pci.c | 121
> > > >>> +++++++++++++++++++++++++++++++++
> > > >>>> 2 files changed, 122 insertions(+), 1 deletions(-)
> > > >>>
> > > >>> Did you ever compare this to just re-parsing device tree method?
> > > >>>
> > > >>> - k
> > > >>
> > > >> I tested the re-parsing way by using setup_pci_atmu() when resume.
> > > >> And I found out that re-parsing will *change* outbound IO
> > > >> translation address regitster.
> > > >>
> > > >> It seems that in the first bootup, after setup_atmu()
> > > >> pcibios_setup_phb_resources() may update hose->io_resource, but
> > > >> atmu is not updated according to the new hose->io_resource value.
> > > >> In resume from sleep setup_atmu() will reset atmu according to
> > > >> the new hose->io_resource value. So the setup_atmu() will cause
> > > >> different result on outbound IO register between first bootup and
> > > >> resume from sleep.
> > > >>
> > > >> So... There's a possibility that in the first bootup atmu is not
> > > >> setup properly.
> > > >
> > > > [Are you seeing this happen in your testing? If so its a bug we
> > > > need
> > > to look at fixing.]
> > > >
> > > > Yes, I see this in my testing.
> > > > Also PCIe ethernet card works well after resuming from sleep in
> > > > both
> > > save/restore
> > > > and re-parsing way. (Maybe PCIe ethernet card don't need outbound
> > > > IO
> > > resource)
> > > > So, I guess the result of re-parsing (actually it's re-setup) is
> > > > right
> > > and ATMU is not setup
> > > > properly at the first bootup.
> > >
> > > Are you seeing the following message - "PCI: I/O resource not set
> > > for host bridge" ?
> >
> > No.
> >
> > >
> > > Trying to understand why you'd hit the reassignment of io_resource.
> > >
> > > - k
> > >
> >
> > I did some investigations and the conclusion is:
> >
> > io_resource.flags & IORESOURCE_IO are both positive but
> > io_resource.start is 0 before pcibios_setup_phb_io_space() is done.
> >
> > The sequence of related process listed below:
> > fsl_add_bridge() -> setup_pci_atmu()
> > pcibios_init() -> pcibios_scan_phb() -> pcibios_setup_phb_io_space()
> >
> > Because fsl_add_bridge() must be finished before pcibios_init() so
> > ATMU is set when io_resource.start is 0. That means outbound IO regs
> > are not set.
> >
> > If system re-setup ATMU the io_resource.start has already updated so
> > outbound IO regs are set.
> >
> > My question is:
> > Is there any problem if outbound IO regs are not set in first bootup?
>=20
> Please also provide the IO resource address range before and after the
> pci scan. Then we can evaluate if the range is needed to be mapped via
> ATMU.
>=20
> Leo
Since potar is set by out_be32(&pci->pow[j].potar, (hose->io_resource.start=
>> 12);
I provide the result of hose->io_resource.start >> 12 as follows:
pcie@ffe09000:
before pci scan: io_resource.start >> 12: 0
after pci scan : io_resource.start >> 12: ff7ed
pcie@ffe0a000:
before pci scan: io_resource.start >> 12: 0
after pci scan : io_resource.start >> 12: ff7db
pcie@ffe0b000:
before pci scan: io_resource.start >> 12: 0
after pci scan : io_resource.start >> 12: ff7c9
Note that I tested on P1022DS.
- Hongtao.
^ permalink raw reply
* [PATCH] powerpc: Set paca->data_offset = 0 for boot cpu
From: Michael Ellerman @ 2012-09-21 8:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In commit 407821a we assigned a poison value to the paca->data_offset.
Unfortunately with CONFIG_LOCK_STAT=y lockdep will read & write to percpu
data very early in boot, prior to us initialising the percpu areas,
leading to a crash.
We have been getting away with this because the data_offset was previously
set to zero. This causes lockdep to read & write to the initial copy of
the percpu variables, which are discarded later in boot.
Although that is "fishy", it does work, and for lock statistics it is no
big deal to discard the counts from early boot.
So set the paca->data_offset = 0 for the boot cpu paca only.
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/kernel/setup_64.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 389bd4f..efb6a41 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -208,6 +208,8 @@ void __init early_setup(unsigned long dt_ptr)
/* Fix up paca fields required for the boot cpu */
get_paca()->cpu_start = 1;
+ /* Allow percpu accesses to "work" until we setup percpu data */
+ get_paca()->data_offset = 0;
/* Probe the machine type */
probe_machine();
--
1.7.9.5
^ permalink raw reply related
* [PATCH] powerpc: Remove tlb batching hack for nighthawk
From: Michael Ellerman @ 2012-09-21 8:08 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard
In hpte_init_native() we call tlb_batching_enabled() to decide if we
should setup ppc_md.flush_hash_range.
tlb_batching_enabled() checks the _unflattened_ device tree, to see
if we are running on a nighthawk.
Since commit a223535 ("dont allow pSeries_probe to succeed without
initialising MMU", Dec 2006), hpte_init_native() has been called from
pSeries_probe() - at which point we have not yet unflattened the
device tree.
This means tlb_batching_enabled() will always return true, so the hack
has effectively been disabled since Dec 2006. Ergo, I think we can
drop it.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/mm/hash_native_64.c | 26 +-------------------------
1 file changed, 1 insertion(+), 25 deletions(-)
diff --git a/arch/powerpc/mm/hash_native_64.c b/arch/powerpc/mm/hash_native_64.c
index f21e8ce..60c21a8 100644
--- a/arch/powerpc/mm/hash_native_64.c
+++ b/arch/powerpc/mm/hash_native_64.c
@@ -539,29 +539,6 @@ static void native_flush_hash_range(unsigned long number, int local)
local_irq_restore(flags);
}
-#ifdef CONFIG_PPC_PSERIES
-/* Disable TLB batching on nighthawk */
-static inline int tlb_batching_enabled(void)
-{
- struct device_node *root = of_find_node_by_path("/");
- int enabled = 1;
-
- if (root) {
- const char *model = of_get_property(root, "model", NULL);
- if (model && !strcmp(model, "IBM,9076-N81"))
- enabled = 0;
- of_node_put(root);
- }
-
- return enabled;
-}
-#else
-static inline int tlb_batching_enabled(void)
-{
- return 1;
-}
-#endif
-
void __init hpte_init_native(void)
{
ppc_md.hpte_invalidate = native_hpte_invalidate;
@@ -570,6 +547,5 @@ void __init hpte_init_native(void)
ppc_md.hpte_insert = native_hpte_insert;
ppc_md.hpte_remove = native_hpte_remove;
ppc_md.hpte_clear_all = native_hpte_clear;
- if (tlb_batching_enabled())
- ppc_md.flush_hash_range = native_flush_hash_range;
+ ppc_md.flush_hash_range = native_flush_hash_range;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] powerpc: Set paca->data_offset = 0 for boot cpu
From: Aneesh Kumar K.V @ 2012-09-21 8:37 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <1348214878-32319-1-git-send-email-michael@ellerman.id.au>
Michael Ellerman <michael@ellerman.id.au> writes:
> In commit 407821a we assigned a poison value to the paca->data_offset.
>
> Unfortunately with CONFIG_LOCK_STAT=y lockdep will read & write to percpu
> data very early in boot, prior to us initialising the percpu areas,
> leading to a crash.
>
> We have been getting away with this because the data_offset was previously
> set to zero. This causes lockdep to read & write to the initial copy of
> the percpu variables, which are discarded later in boot.
>
> Although that is "fishy", it does work, and for lock statistics it is no
> big deal to discard the counts from early boot.
>
> So set the paca->data_offset = 0 for the boot cpu paca only.
>
> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/setup_64.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 389bd4f..efb6a41 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -208,6 +208,8 @@ void __init early_setup(unsigned long dt_ptr)
>
> /* Fix up paca fields required for the boot cpu */
> get_paca()->cpu_start = 1;
> + /* Allow percpu accesses to "work" until we setup percpu data */
> + get_paca()->data_offset = 0;
>
> /* Probe the machine type */
> probe_machine();
> --
> 1.7.9.5
^ permalink raw reply
* Re: [PATCH v3 1/2] mmc: Move mmc_delay() to include/linux/mmc/core.h
From: Chunhe Lan @ 2012-09-21 20:52 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: kumar.gala, linux-mmc, cjb, linuxppc-dev, Chunhe Lan
In-Reply-To: <201208101327.47789.arnd@arndb.de>
On 08/10/2012 09:27 AM, Arnd Bergmann wrote:
> On Friday 10 August 2012, Chunhe Lan wrote:
>
>> +static inline void mmc_delay(unsigned int ms)
>> +{
>> + if (ms < 1000 / HZ) {
>> + cond_resched();
>> + mdelay(ms);
>> + } else {
>> + msleep(ms);
>> + }
>> +}
> I would actually question the point in this function to start with: The
> decision whether to call mdelay() or msleep() should only be based on
> whether you are allowed to sleep in the caller context. The idea of
>
>
> cond_resched();
> mdelay(ms);
>
> sets off alarm bells, and I would always replace that with msleep().
I think that it does not replace with msleep().
When the time of sleep is very short, program should not been scheduled
in the context. Because it expends the more time.
Thanks,
Chunhe
>
> Arnd
>
^ permalink raw reply
* [PATCH] arch/powerpc: Don't release eeh_mutex in eeh_phb_pe_get
From: Aneesh Kumar K.V @ 2012-09-21 9:29 UTC (permalink / raw)
To: benh, shangw; +Cc: linuxppc-dev, Aneesh Kumar K.V
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
=====================================
[ BUG: bad unlock balance detected! ]
3.6.0-rc5-00338-gcaa1d63-dirty #6 Not tainted
-------------------------------------
swapper/0/1 is trying to release lock (eeh_mutex) at:
[<c000000000058218>] .eeh_add_to_parent_pe+0x318/0x410
but there are no more locks to release!
other info that might help us debug this:
no locks held by swapper/0/1.
stack backtrace:
Call Trace:
[c00000003e483870] [c000000000013310] .show_stack+0x70/0x1c0 (unreliable)
[c00000003e483920] [c0000000000d8310] .print_unlock_inbalance_bug+0x110/0x120
[c00000003e4839b0] [c0000000000d9a50] .lock_release+0x1d0/0x240
[c00000003e483a60] [c000000000778064] .__mutex_unlock_slowpath+0xb4/0x250
[c00000003e483b10] [c000000000058218] .eeh_add_to_parent_pe+0x318/0x410
[c00000003e483bc0] [c00000000005a118] .pseries_eeh_of_probe+0x258/0x2f0
[c00000003e483cc0] [c000000000032528] .traverse_pci_devices+0xa8/0x150
[c00000003e483d70] [c000000000aa7288] .eeh_init+0xd4/0x140
[c00000003e483e00] [c00000000000abc4] .do_one_initcall+0x64/0x1e0
[c00000003e483ec0] [c000000000a90418] .kernel_init+0x1e8/0x2bc
[c00000003e483f90] [c00000000002048c] .kernel_thread+0x54/0x70
EEH: PCI Enhanced I/O Error Handling Enabled
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/eeh_pe.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pe.c b/arch/powerpc/platforms/pseries/eeh_pe.c
index 9d35543..797cd18 100644
--- a/arch/powerpc/platforms/pseries/eeh_pe.c
+++ b/arch/powerpc/platforms/pseries/eeh_pe.c
@@ -105,11 +105,8 @@ static struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb)
* the PE for PHB has been determined when that
* was created.
*/
- if ((pe->type & EEH_PE_PHB) &&
- pe->phb == phb) {
- eeh_unlock();
+ if ((pe->type & EEH_PE_PHB) && pe->phb == phb)
return pe;
- }
}
return NULL;
--
1.7.10
^ permalink raw reply related
* Re: [PATCH] arch/powerpc: Don't release eeh_mutex in eeh_phb_pe_get
From: Gavin Shan @ 2012-09-21 10:58 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, shangw
In-Reply-To: <1348219786-31221-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Fri, Sep 21, 2012 at 02:59:46PM +0530, Aneesh Kumar K.V wrote:
>From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
>=====================================
>[ BUG: bad unlock balance detected! ]
>3.6.0-rc5-00338-gcaa1d63-dirty #6 Not tainted
>-------------------------------------
>swapper/0/1 is trying to release lock (eeh_mutex) at:
>[<c000000000058218>] .eeh_add_to_parent_pe+0x318/0x410
>but there are no more locks to release!
>
>other info that might help us debug this:
>no locks held by swapper/0/1.
>
>stack backtrace:
>Call Trace:
>[c00000003e483870] [c000000000013310] .show_stack+0x70/0x1c0 (unreliable)
>[c00000003e483920] [c0000000000d8310] .print_unlock_inbalance_bug+0x110/0x120
>[c00000003e4839b0] [c0000000000d9a50] .lock_release+0x1d0/0x240
>[c00000003e483a60] [c000000000778064] .__mutex_unlock_slowpath+0xb4/0x250
>[c00000003e483b10] [c000000000058218] .eeh_add_to_parent_pe+0x318/0x410
>[c00000003e483bc0] [c00000000005a118] .pseries_eeh_of_probe+0x258/0x2f0
>[c00000003e483cc0] [c000000000032528] .traverse_pci_devices+0xa8/0x150
>[c00000003e483d70] [c000000000aa7288] .eeh_init+0xd4/0x140
>[c00000003e483e00] [c00000000000abc4] .do_one_initcall+0x64/0x1e0
>[c00000003e483ec0] [c000000000a90418] .kernel_init+0x1e8/0x2bc
>[c00000003e483f90] [c00000000002048c] .kernel_thread+0x54/0x70
>EEH: PCI Enhanced I/O Error Handling Enabled
>
>Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Thanks,
Gavin
>---
> arch/powerpc/platforms/pseries/eeh_pe.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
>diff --git a/arch/powerpc/platforms/pseries/eeh_pe.c b/arch/powerpc/platforms/pseries/eeh_pe.c
>index 9d35543..797cd18 100644
>--- a/arch/powerpc/platforms/pseries/eeh_pe.c
>+++ b/arch/powerpc/platforms/pseries/eeh_pe.c
>@@ -105,11 +105,8 @@ static struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb)
> * the PE for PHB has been determined when that
> * was created.
> */
>- if ((pe->type & EEH_PE_PHB) &&
>- pe->phb == phb) {
>- eeh_unlock();
>+ if ((pe->type & EEH_PE_PHB) && pe->phb == phb)
> return pe;
>- }
> }
>
> return NULL;
>--
>1.7.10
>
^ permalink raw reply
* PCI device not working
From: Davide Viti @ 2012-09-21 11:33 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]
Hi,
I'm working on a custom board based on P1020 with two (identical) PCI
devices attached;
The work is derived from another board with a single instance of that
device.
The system is based on u-boot-2009.11 and Linux 2.6.34.6
The "pci" command on u-boot, shows me both the PCI controllers and
the attached devices:
Scanning PCI devices on bus 0
BusDevFun VendorId DeviceId Device Class Sub-Class
_____________________________________________________________
00.00.00 0x1957 0x0100 Processor 0x20
Scanning PCI devices on bus 1
BusDevFun VendorId DeviceId Device Class Sub-Class
_____________________________________________________________
01.00.00 0x1b65 0xabba Network controller 0x80
Scanning PCI devices on bus 2
BusDevFun VendorId DeviceId Device Class Sub-Class
_____________________________________________________________
02.00.00 0x1957 0x0100 Processor 0x20
Scanning PCI devices on bus 3
BusDevFun VendorId DeviceId Device Class Sub-Class
_____________________________________________________________
03.00.00 0x1b65 0xabba Network controller 0x80
The kernel detects only the first instance of the device.
Didn't get very far while looking at dts file and kernel logs, so I'm
asking for some help on narrowing down the problem.
I'm wondering if I can assume that the problem is restricted to
kernel/dts and avoid concentrating on uboot.
I can provide any log (didn't want to post tons of details on the first
message)
Thanx in advance for your help,
regards
Davide
[-- Attachment #2: Type: text/html, Size: 2117 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] mmc: Move mmc_delay() to include/linux/mmc/core.h
From: Arnd Bergmann @ 2012-09-21 12:33 UTC (permalink / raw)
To: Chunhe Lan; +Cc: kumar.gala, linux-mmc, cjb, linuxppc-dev, Chunhe Lan
In-Reply-To: <505CD3A5.6080101@freescale.com>
On Friday 21 September 2012, Chunhe Lan wrote:
> On 08/10/2012 09:27 AM, Arnd Bergmann wrote:
> > On Friday 10 August 2012, Chunhe Lan wrote:
> >
> > cond_resched();
> > mdelay(ms);
> >
> > sets off alarm bells, and I would always replace that with msleep().
> I think that it does not replace with msleep().
> When the time of sleep is very short, program should not been scheduled
> in the context. Because it expends the more time.
>
A time measured in miliseconds is never "very short" for the scheduler,
a lot of things can happen during that time span. The code I quoted
also does not care too much about accuracy, otherwise it would adapt
the time in the mdelay based on whether the cond_resched() actually
schedules to another thread.
Arnd
^ permalink raw reply
* Re: [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode
From: Kumar Gala @ 2012-09-21 13:06 UTC (permalink / raw)
To: Lian Minghaun-b31939; +Cc: Minghuan Lian, linuxppc-dev
In-Reply-To: <505BE102.7080701@freescale.com>
On Sep 20, 2012, at 10:37 PM, Lian Minghaun-b31939 wrote:
> Hi Kumar,
>=20
> please see my comments inline.
>=20
>=20
> On 09/19/2012 10:22 PM, Kumar Gala wrote:
>> On Sep 19, 2012, at 2:23 AM, Minghuan Lian wrote:
>>=20
>>> The original code uses 'Programming Interface' field to judge if =
PCIE is
>>> EP or RC mode, however, some latest silicons do not support this =
functionality.
>>> According to PCIE specification, 'Header Type' offset 0x0e is used =
to
>>> indicate header type, so change code to use 'Header Type' field to
>>> judge PCIE mode. Because FSL PCI controller does not support 'Header =
Type',
>>> patch still uses 'Programming Interface' to identify PCI mode.
>>>=20
>>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>>> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
>>> ---
>>> arch/powerpc/sysdev/fsl_pci.c | 38 =
+++++++++++++++++++++++---------------
>>> 1 file changed, 23 insertions(+), 15 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/sysdev/fsl_pci.c =
b/arch/powerpc/sysdev/fsl_pci.c
>>> index c37f461..43d30df 100644
>>> --- a/arch/powerpc/sysdev/fsl_pci.c
>>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>>> @@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
>>>=20
>>> static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
>>> {
>>> - u8 progif;
>>> + u8 hdr_type;
>>>=20
>>> /* if we aren't a PCIe don't bother */
>>> if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
>>> return;
>>>=20
>>> /* if we aren't in host mode don't bother */
>>> - pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
>>> - if (progif & 0x1)
>>> + pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
>>> + if ((hdr_type & 0x7f) !=3D PCI_HEADER_TYPE_BRIDGE)
>>> return;
>>>=20
>>> dev->class =3D PCI_CLASS_BRIDGE_PCI << 8;
>>> @@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node =
*dev, int is_primary)
>>> struct pci_controller *hose;
>>> struct resource rsrc;
>>> const int *bus_range;
>>> - u8 progif;
>>> + u8 hdr_type, progif;
>>>=20
>>> if (!of_device_is_available(dev)) {
>>> pr_warning("%s: disabled\n", dev->full_name);
>>> @@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node =
*dev, int is_primary)
>>> setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
>>> PPC_INDIRECT_TYPE_BIG_ENDIAN);
>>>=20
>>> - early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>>> - if ((progif & 1) =3D=3D 1) {
>>> - /* unmap cfg_data & cfg_addr separately if not on same =
page */
>>> - if (((unsigned long)hose->cfg_data & PAGE_MASK) !=3D
>>> - ((unsigned long)hose->cfg_addr & PAGE_MASK))
>>> - iounmap(hose->cfg_data);
>>> - iounmap(hose->cfg_addr);
>>> - pcibios_free_controller(hose);
>>> - return -ENODEV;
>>> - }
>>> -
>>> setup_pci_cmd(hose);
>> I think we should be doing the check before we call setup_pci_cmd(). =
The old code didn't touch the controller registers if we where and =
end-point. We should maintain that.
> [Minghuan] Thanks for you pointing this.
> I want to move setup_pci_cmd like this:
>=20
> pr_debug(" ->Hose at 0x%p, cfg_addr=3D0x%p,cfg_data=3D0x%p\n",
> hose, hose->cfg_addr, hose->cfg_data);
>=20
> + setup_pci_cmd(hose);
>=20
> /* Interpret the "ranges" property */
> /* This also maps the I/O region and sets isa_io/mem_base */
> pci_process_bridge_OF_ranges(hose, dev, is_primary);
>=20
> This movement will cause fsl_pcie_check_link() calling before =
setup_pci_cmd().
> Is this ok?
I think so, as its how the code is today:
setup_pci_cmd()
..
if (pcie)
fsl_pcie_check_link()
> If not, I will call setup_pci_cmd() for PCI and PCIE respectively.
^ permalink raw reply
* Re: [PATCH][V4] powerpc/fsl-pci: Add pci inbound/outbound PM support
From: Kumar Gala @ 2012-09-21 13:15 UTC (permalink / raw)
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01AB6AE7@039-SN1MPN1-004.039d.mgd.msft.net>
>>>>>>>=20
>>>>>>> On Sep 17, 2012, at 9:10 PM, Jia Hongtao wrote:
>>>>>>>=20
>>>>>>>> Power supply for PCI inbound/outbound window registers is off
>>>>>>>> when system go to deep-sleep state. We save the values of
>>>>>>>> registers
>>>> before
>>>>>>>> suspend and restore to registers after resume.
>>>>>>>>=20
>>>>>>>> Signed-off-by: Jiang Yutang <b14898@freescale.com>
>>>>>>>> Signed-off-by: Jia Hongtao <B38951@freescale.com>
>>>>>>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>>>>>>> ---
>>>>>>>> Changes for V4:
>>>>>>>> We just rebase the patch upon following patch:
>>>>>>>> powerpc/fsl-pci: Unify pci/pcie initialization code
>>>>>>>>=20
>>>>>>>> arch/powerpc/include/asm/pci-bridge.h | 2 +-
>>>>>>>> arch/powerpc/sysdev/fsl_pci.c | 121
>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>> 2 files changed, 122 insertions(+), 1 deletions(-)
>>>>>>>=20
>>>>>>> Did you ever compare this to just re-parsing device tree method?
>>>>>>>=20
>>>>>>> - k
>>>>>>=20
>>>>>> I tested the re-parsing way by using setup_pci_atmu() when =
resume.
>>>>>> And I found out that re-parsing will *change* outbound IO
>>>>>> translation address regitster.
>>>>>>=20
>>>>>> It seems that in the first bootup, after setup_atmu()
>>>>>> pcibios_setup_phb_resources() may update hose->io_resource, but
>>>>>> atmu is not updated according to the new hose->io_resource value.
>>>>>> In resume from sleep setup_atmu() will reset atmu according to
>>>>>> the new hose->io_resource value. So the setup_atmu() will cause
>>>>>> different result on outbound IO register between first bootup and
>>>>>> resume from sleep.
>>>>>>=20
>>>>>> So... There's a possibility that in the first bootup atmu is not
>>>>>> setup properly.
>>>>>=20
>>>>> [Are you seeing this happen in your testing? If so its a bug we
>>>>> need
>>>> to look at fixing.]
>>>>>=20
>>>>> Yes, I see this in my testing.
>>>>> Also PCIe ethernet card works well after resuming from sleep in
>>>>> both
>>>> save/restore
>>>>> and re-parsing way. (Maybe PCIe ethernet card don't need outbound
>>>>> IO
>>>> resource)
>>>>> So, I guess the result of re-parsing (actually it's re-setup) is
>>>>> right
>>>> and ATMU is not setup
>>>>> properly at the first bootup.
>>>>=20
>>>> Are you seeing the following message - "PCI: I/O resource not set
>>>> for host bridge" ?
>>>=20
>>> No.
>>>=20
>>>>=20
>>>> Trying to understand why you'd hit the reassignment of io_resource.
>>>>=20
>>>> - k
>>>>=20
>>>=20
>>> I did some investigations and the conclusion is:
>>>=20
>>> io_resource.flags & IORESOURCE_IO are both positive but
>>> io_resource.start is 0 before pcibios_setup_phb_io_space() is done.
>>>=20
>>> The sequence of related process listed below:
>>> fsl_add_bridge() -> setup_pci_atmu()
>>> pcibios_init() -> pcibios_scan_phb() -> pcibios_setup_phb_io_space()
>>>=20
>>> Because fsl_add_bridge() must be finished before pcibios_init() so
>>> ATMU is set when io_resource.start is 0. That means outbound IO regs
>>> are not set.
>>>=20
>>> If system re-setup ATMU the io_resource.start has already updated so
>>> outbound IO regs are set.
>>>=20
>>> My question is:
>>> Is there any problem if outbound IO regs are not set in first =
bootup?
Yes, it means that IO transactions would not work.
>> Please also provide the IO resource address range before and after =
the
>> pci scan. Then we can evaluate if the range is needed to be mapped =
via
>> ATMU.
>>=20
>> Leo
>=20
> Since potar is set by out_be32(&pci->pow[j].potar, =
(hose->io_resource.start >> 12);
> I provide the result of hose->io_resource.start >> 12 as follows:
>=20
> pcie@ffe09000:
> before pci scan: io_resource.start >> 12: 0
> after pci scan : io_resource.start >> 12: ff7ed
>=20
> pcie@ffe0a000:
> before pci scan: io_resource.start >> 12: 0
> after pci scan : io_resource.start >> 12: ff7db
>=20
> pcie@ffe0b000:
> before pci scan: io_resource.start >> 12: 0
> after pci scan : io_resource.start >> 12: ff7c9
>=20
> Note that I tested on P1022DS.
>=20
> - Hongtao.
1. What's the device tree nodes for PCIe look like?
2. Can you get the pr_debug() in setup_pci_atmu() to print and report =
results (as well as full boot log)
However, I think the change of the io_resource.start is normal and =
correct behavior.
- k
^ permalink raw reply
* Re: PCI device not working
From: Kumar Gala @ 2012-09-21 13:17 UTC (permalink / raw)
To: Davide Viti; +Cc: linuxppc-dev
In-Reply-To: <CAKpAL0m2XqzuYsG4-yj3WfZN88PjQXMUxFtXpx-sjsWA_Q4EUQ@mail.gmail.com>
On Sep 21, 2012, at 6:33 AM, Davide Viti wrote:
> Hi,
> I'm working on a custom board based on P1020 with two (identical) PCI =
devices attached;
> The work is derived from another board with a single instance of that =
device.
> The system is based on u-boot-2009.11 and Linux 2.6.34.6
>=20
> The "pci" command on u-boot, shows me both the PCI controllers and
> the attached devices:
>=20
> Scanning PCI devices on bus 0
> BusDevFun VendorId DeviceId Device Class Sub-Class
> _____________________________________________________________
> 00.00.00 0x1957 0x0100 Processor 0x20
>=20
> Scanning PCI devices on bus 1
> BusDevFun VendorId DeviceId Device Class Sub-Class
> _____________________________________________________________
> 01.00.00 0x1b65 0xabba Network controller 0x80
>=20
> Scanning PCI devices on bus 2
> BusDevFun VendorId DeviceId Device Class Sub-Class
> _____________________________________________________________
> 02.00.00 0x1957 0x0100 Processor 0x20
>=20
> Scanning PCI devices on bus 3
> BusDevFun VendorId DeviceId Device Class Sub-Class
> _____________________________________________________________
> 03.00.00 0x1b65 0xabba Network controller 0x80
>=20
> The kernel detects only the first instance of the device.
What do you mean by first instance of the device ?
> Didn't get very far while looking at dts file and kernel logs, so I'm
> asking for some help on narrowing down the problem.
>=20
> I'm wondering if I can assume that the problem is restricted to
> kernel/dts and avoid concentrating on uboot.=20
> I can provide any log (didn't want to post tons of details on the =
first=20
> message)
Probably a dts issue.
What does lspci in linux say?
- k
^ permalink raw reply
* Re: PCI device not working
From: Davide Viti @ 2012-09-21 14:06 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <89227A80-31E1-4E18-9257-6B60126E5A62@kernel.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 4311 bytes --]
I mean there are two controllers and both of them have a device "subtended"
(both 0x1b65:0xabba).
u-boot can see both devices, linux detects only the device attached to the
first controller.
Here's the output of lspci and /proc/iomem :
root@(none):/# lspci -v
0000:00:00.0 Class 0604: Device 1957:0100 (rev 11)
Flags: bus master, fast devsel, latency 0
Memory at <ignored> (32-bit, non-prefetchable)
Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
I/O behind bridge: 00000000-00000fff
Memory behind bridge: a0000000-afffffff
Capabilities: [44] Power Management version 2
Capabilities: [4c] Express Root Port (Slot-), MSI 00
Capabilities: [100] Advanced Error Reporting
0000:01:00.0 Class 0280: Device 1b65:abba (rev 01)
Flags: bus master, fast devsel, latency 0, IRQ 16
Memory at a0000000 (32-bit, non-prefetchable) [size=1K]
Memory at a0010000 (32-bit, non-prefetchable) [size=64K]
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [78] Power Management version 3
Capabilities: [80] Express Endpoint, MSI 00
Capabilities: [100] Virtual Channel <?>
Capabilities: [800] Advanced Error Reporting
0001:02:00.0 Class 0604: Device 1957:0100 (rev 11)
Flags: bus master, fast devsel, latency 0
Memory at <ignored> (32-bit, non-prefetchable)
Bus: primary=00, secondary=03, subordinate=03, sec-latency=0
I/O behind bridge: 00000000-00000fff
Memory behind bridge: b0000000-bfffffff
Capabilities: [44] Power Management version 2
Capabilities: [4c] Express Root Port (Slot-), MSI 00
Capabilities: [100] Advanced Error Reporting
root@(none):/# cat /proc/iomem
a0000000-afffffff : /pcie@ffe09000
a0000000-afffffff : PCI Bus 0000:01
a0000000-a00003ff : 0000:01:00.0
a0010000-a001ffff : 0000:01:00.0
b0000000-bfffffff : /pcie@ffe0a000
b0000000-bfffffff : PCI Bus 0001:03
ef000000-efffffff : ef000000.nor
ffe04500-ffe04507 : serial
ffe04600-ffe04607 : serial
thanx for your help,
Davide
I mean that the kernel detects the first controller and the device attached
to it, plus the second controller: the device on the second controller is
not detected (same device as the one detected on the first controller)
2012/9/21 Kumar Gala <galak@kernel.crashing.org>
>
> On Sep 21, 2012, at 6:33 AM, Davide Viti wrote:
>
> > Hi,
> > I'm working on a custom board based on P1020 with two (identical) PCI
> devices attached;
> > The work is derived from another board with a single instance of that
> device.
> > The system is based on u-boot-2009.11 and Linux 2.6.34.6
> >
> > The "pci" command on u-boot, shows me both the PCI controllers and
> > the attached devices:
> >
> > Scanning PCI devices on bus 0
> > BusDevFun VendorId DeviceId Device Class Sub-Class
> > _____________________________________________________________
> > 00.00.00 0x1957 0x0100 Processor 0x20
> >
> > Scanning PCI devices on bus 1
> > BusDevFun VendorId DeviceId Device Class Sub-Class
> > _____________________________________________________________
> > 01.00.00 0x1b65 0xabba Network controller 0x80
> >
> > Scanning PCI devices on bus 2
> > BusDevFun VendorId DeviceId Device Class Sub-Class
> > _____________________________________________________________
> > 02.00.00 0x1957 0x0100 Processor 0x20
> >
> > Scanning PCI devices on bus 3
> > BusDevFun VendorId DeviceId Device Class Sub-Class
> > _____________________________________________________________
> > 03.00.00 0x1b65 0xabba Network controller 0x80
> >
> > The kernel detects only the first instance of the device.
>
> What do you mean by first instance of the device ?
>
> > Didn't get very far while looking at dts file and kernel logs, so I'm
> > asking for some help on narrowing down the problem.
> >
> > I'm wondering if I can assume that the problem is restricted to
> > kernel/dts and avoid concentrating on uboot.
> > I can provide any log (didn't want to post tons of details on the first
> > message)
>
> Probably a dts issue.
>
> What does lspci in linux say?
>
> - k
>
>
[-- Attachment #2: Type: text/html, Size: 9183 bytes --]
^ permalink raw reply
* Re: [PATCH v3] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
From: Greg KH @ 2012-09-21 16:43 UTC (permalink / raw)
To: Shengzhou Liu; +Cc: linux-usb, linuxppc-dev
In-Reply-To: <1347958359-20153-1-git-send-email-Shengzhou.Liu@freescale.com>
On Tue, Sep 18, 2012 at 04:52:39PM +0800, Shengzhou Liu wrote:
> when missing USB PHY clock, kernel booting up will hang during USB
> initialization. We should check USBGP[PHY_CLK_VALID] bit to avoid
> CPU hanging in this case.
>
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> ---
> v3 change: no check for UTMI PHY.
> v2 change: use spin_event_timeout() instead.
>
> drivers/usb/host/ehci-fsl.c | 57 +++++++++++++++++++++++++++++-------------
> drivers/usb/host/ehci-fsl.h | 1 +
> include/linux/fsl_devices.h | 1 +
> 3 files changed, 41 insertions(+), 18 deletions(-)
This is already applied, right?
greg k-h
^ 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