* Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Tabi Timur-B04825 @ 2012-02-24 4:11 UTC (permalink / raw)
To: Li Yang-R58472
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F0578D5BD@039-SN2MPN1-023.039d.mgd.msft.net>
Li Yang-R58472 wrote:
> It's a good point. Why can't we decide to use 32-bit/36-bit TLB at runti=
me even for e500v2?
That's not what PHYS_64BIT does. PHYS_64BIT determines whether=20
phys_addr_t is a u64 or a u32. This is something that must be determined=20
at compilation time.
>> Please remember that the Kconfig for the P1022DS already forced
>> PHYS_64BIT for all mpc85xx platforms. All we're doing is making it
>> possible to deselect PHYS_64BIT.
>
> I think it's a side-effect introduced by P1022DS support and need to be f=
ixed.
Exactly. That's what these patches do. And these patches have been=20
applied to the SDK. I'm just waiting for Kumar to apply them to his=20
repository.
> There was no mentioning of enforcing 36-bit for all mpc85xx platforms.
It's not enforcing, it's just the default. If you build with=20
mpc85xx_smp_defconfig, then you'll get a 36-bit kernel. But now you can=20
also use menuconfig to turn off PHYS_64BIT. Before these fixes, that was=20
not possible.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Li Yang-R58472 @ 2012-02-24 4:02 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <4F470910.9080008@freescale.com>
> Subject: Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be
> selectable
>=20
> Li Yang-R58472 wrote:
>=20
> > The mpc85xx_defconfig does include silicons with e500v1 core which
> > doesn't have the 36-bit support. Won't enabling 36-bit support by
> > default break the support for them?
>=20
> No. The kernel will detect at runtime that that it's an e500v1 core and
> it won't try to create 36-bit TLBs. (e.g. it won't write to MAS7).
It's a good point. Why can't we decide to use 32-bit/36-bit TLB at runtime=
even for e500v2?
>=20
> Please remember that the Kconfig for the P1022DS already forced
> PHYS_64BIT for all mpc85xx platforms. All we're doing is making it
> possible to deselect PHYS_64BIT.
I think it's a side-effect introduced by P1022DS support and need to be fix=
ed. There was no mentioning of enforcing 36-bit for all mpc85xx platforms.
- Leo
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Tabi Timur-B04825 @ 2012-02-24 3:50 UTC (permalink / raw)
To: Li Yang-R58472
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F0578D305@039-SN2MPN1-023.039d.mgd.msft.net>
Li Yang-R58472 wrote:
> The mpc85xx_defconfig does include silicons with e500v1 core which
> doesn't have the 36-bit support. Won't enabling 36-bit support by
> default break the support for them?
No. The kernel will detect at runtime that that it's an e500v1 core and=20
it won't try to create 36-bit TLBs. (e.g. it won't write to MAS7).
Please remember that the Kconfig for the P1022DS already forced PHYS_64BIT=
=20
for all mpc85xx platforms. All we're doing is making it possible to=20
deselect PHYS_64BIT.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Li Yang-R58472 @ 2012-02-24 3:40 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <4F46FE3E.1050605@freescale.com>
> Subject: Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be
> selectable
>=20
> Li Yang-R58472 wrote:
>=20
> > Even though the user still need to know the addressing mode that
> > u-boot is using. It won't work if the addressing mode of u-boot and
> > device tree are different.
>=20
> U-Boot will tell the user if the DT does not match. I added code to U-
> Boot to do that. So if you have a 36-bit U-Boot and a 32-bit DT, then
> you will get a warning. If you have a 36-bit U-boot and a 36-bit DT and
> a 32-bit kernel, you will get nothing. But if you have a 32-bit U-boot
> and a 32-bit DT and a 36-bit kernel, then that will work. A 36-bit
> kernel works with 32-bit *and* 36-bit DTs. This is why a 36-bit kernel
> should be the default.
The mpc85xx_defconfig does include silicons with e500v1 core which doesn't =
have the 36-bit support. Won't enabling 36-bit support by default break th=
e support for them?
- Leo
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Tabi Timur-B04825 @ 2012-02-24 3:36 UTC (permalink / raw)
To: Huang Changming-R66093
Cc: linuxppc-dev@ozlabs.org, Wood Scott-B07421, Li Yang-R58472
In-Reply-To: <110EED8CC96DFC488B7E717A2027A27C02E5C3@039-SN1MPN1-002.039d.mgd.msft.net>
Huang Changming-R66093 wrote:
> I want to know if you have the other codes for different address?
>
> The current U-boot just detect the base address of DTS and the CCSR addre=
ss.
> If they are different, u-boot will print the warning and return 0,
> so the kernel can't been booted.
I had a patch that verified some PCI addresses (which are sometimes=20
slightly mismatched), but Kumar rejected it.
I could add some more checks, but the 90% of the time, the only problem is=
=20
using the wrong size (32-bit vs 36-bit) DT.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Huang Changming-R66093 @ 2012-02-24 3:24 UTC (permalink / raw)
To: Tabi Timur-B04825, Li Yang-R58472
Cc: linuxppc-dev@ozlabs.org, Wood Scott-B07421
In-Reply-To: <4F46FE3E.1050605@freescale.com>
>=20
> Li Yang-R58472 wrote:
>=20
> > Even though the user still need to know the addressing mode that
> > u-boot is using. It won't work if the addressing mode of u-boot and
> > device tree are different.
>=20
> U-Boot will tell the user if the DT does not match. I added code to U-
> Boot to do that. So if you have a 36-bit U-Boot and a 32-bit DT, then
> you will get a warning. If you have a 36-bit U-boot and a 36-bit DT and
> a 32-bit kernel, you will get nothing. But if you have a 32-bit U-boot
> and a 32-bit DT and a 36-bit kernel, then that will work. A 36-bit
> kernel works with 32-bit *and* 36-bit DTs. This is why a 36-bit kernel
> should be the default.
>=20
> --
Hi, Timur
I want to know if you have the other codes for different address?
The current U-boot just detect the base address of DTS and the CCSR address=
.
If they are different, u-boot will print the warning and return 0,
so the kernel can't been booted.
Jerry.
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Tabi Timur-B04825 @ 2012-02-24 3:04 UTC (permalink / raw)
To: Li Yang-R58472
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F0578CFD9@039-SN2MPN1-023.039d.mgd.msft.net>
Li Yang-R58472 wrote:
> Even though the user still need to know the addressing mode that u-boot
> is using. It won't work if the addressing mode of u-boot and device
> tree are different.
U-Boot will tell the user if the DT does not match. I added code to=20
U-Boot to do that. So if you have a 36-bit U-Boot and a 32-bit DT, then=20
you will get a warning. If you have a 36-bit U-boot and a 36-bit DT and a=
=20
32-bit kernel, you will get nothing. But if you have a 32-bit U-boot and=20
a 32-bit DT and a 36-bit kernel, then that will work. A 36-bit kernel=20
works with 32-bit *and* 36-bit DTs. This is why a 36-bit kernel should be=
=20
the default.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Huang Changming-R66093 @ 2012-02-24 3:01 UTC (permalink / raw)
To: Li Yang-R58472, Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, Wood Scott-B07421
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F0578CFD9@039-SN2MPN1-023.039d.mgd.msft.net>
> > -----Original Message-----
> > From: Tabi Timur-B04825
> > Sent: Friday, February 24, 2012 10:46 AM
> > To: Li Yang-R58472
> > Cc: Huang Changming-R66093; galak@kernel.crashing.org;
> > benh@kernel.crashing.org; Wood Scott-B07421; linuxppc-dev@ozlabs.org
> > Subject: Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be
> > selectable
> >
> > Li Yang-R58472 wrote:
> >
> > > I agree with Changming that we shouldn't setting PHYS_64BIT by
> default.
> >
> > The default kernel should always be the compatible with as much as
> > possible. Disabling PHYS_64BIT by default means that the default
> > kernel will not work with a 36-bit DTS. If you attempt to boot such a
> > kernel with a 36-bit DTS, there will be no text output. Most people
> > will not know why it's not working.
> >
> > So the safest option is for PHYS_64BIT to be enabled by default. That
> > way, the kernel will always work.
>=20
> Even though the user still need to know the addressing mode that u-boot
> is using. It won't work if the addressing mode of u-boot and device tree
> are different.
>=20
Yes, u-boot must has the same address mode with DTS, otherwise, we can't bo=
ot the kernel.
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Li Yang-R58472 @ 2012-02-24 2:59 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <4F46F9CF.6040600@freescale.com>
> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Friday, February 24, 2012 10:46 AM
> To: Li Yang-R58472
> Cc: Huang Changming-R66093; galak@kernel.crashing.org;
> benh@kernel.crashing.org; Wood Scott-B07421; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be
> selectable
>=20
> Li Yang-R58472 wrote:
>=20
> > I agree with Changming that we shouldn't setting PHYS_64BIT by default.
>=20
> The default kernel should always be the compatible with as much as
> possible. Disabling PHYS_64BIT by default means that the default kernel
> will not work with a 36-bit DTS. If you attempt to boot such a kernel
> with a 36-bit DTS, there will be no text output. Most people will not
> know why it's not working.
>=20
> So the safest option is for PHYS_64BIT to be enabled by default. That
> way, the kernel will always work.
Even though the user still need to know the addressing mode that u-boot is =
using. It won't work if the addressing mode of u-boot and device tree are =
different.
- Leo
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Tabi Timur-B04825 @ 2012-02-24 2:45 UTC (permalink / raw)
To: Li Yang-R58472
Cc: linuxppc-dev@ozlabs.org, Huang Changming-R66093,
Wood Scott-B07421
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F0578CD9A@039-SN2MPN1-023.039d.mgd.msft.net>
Li Yang-R58472 wrote:
> I agree with Changming that we shouldn't setting PHYS_64BIT by default.
The default kernel should always be the compatible with as much as=20
possible. Disabling PHYS_64BIT by default means that the default kernel=20
will not work with a 36-bit DTS. If you attempt to boot such a kernel=20
with a 36-bit DTS, there will be no text output. Most people will not=20
know why it's not working.
So the safest option is for PHYS_64BIT to be enabled by default. That=20
way, the kernel will always work.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: How to handle cache when I allocate phys memory?
From: Ayman El-Khashab @ 2012-02-23 23:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1318577991.29415.514.camel@pasglop>
I never did get this to work, and now I am back to it again.
On Fri, Oct 14, 2011 at 09:39:51AM +0200, Benjamin Herrenschmidt wrote:
> On Wed, 2011-10-12 at 16:08 -0500, Ayman El-Khashab wrote:
> > I'm using the 460sx (440 core) so no snooping here. What
> > I've done is reserved the top of memory for my driver. My
> > driver can read/write the memory and I can mmap it just
> > fine. The problem is I want to enable caching on the mmap
> > for performance but I don't know / can't figure out how to
> > tell the kernel to sync the cache after it gets dma data
> > from the device or after i put data into it from user space.
> > I know how to do it from regular devices, but not when I've
> > allocated the physical memory myself. I suppose what I am
> > looking for is something akin to dma_sync_single cpu/device.
> >
> > In my device driver, I am allocating the memory like this,
> > in this case the buffer is about 512MB.
> >
> > vma->vm_flags |= VM_LOCKED | VM_RESERVED;
> >
> > /* map the physical area into one buffer */
> > rc = remap_pfn_range(vma, vma->vm_start,
> > (PHYS_MEM_ADDR)>>PAGE_SHIFT,
> > len, vma->vm_page_prot);
> >
> > Is this going to give me the best performance, or is there
> > something more I can do?
> >
> > Failing that, what is the best way to do this (i need a very
> > large contiguous buffer). it runs in batch mode, so it
> > DMAs, stops, cpu reads, cpu writes, repeat ...
>
> Did you try looking at what the dma_* functions do under the hood and
> call it directly (or reproducing it) ?
>
> Basically it boils down to using dcbf instructions to flush dirty data
> or dcbi to invalidate cache lines.
>
I've reserved (using mem=) memory at the top of my system.
In my case, its the upper 1GB of 2GB total. I've got a
small driver that I've written that maps it into user space
using mmap .. that all works fine. I've also got it caching
and that also works. The problem is that depending on how I
do things, I can get some cache-coherency issues. I know in
the user code where to poke things, but I've tried
everything I can think of and *cannot* get flush_dcache_range
to work for me.
My mapping code in the driver is:
static int my_mmap(struct file *fip, struct
vm_area_struct *vma)
{
int rc;
unsigned long len = vma->vm_end - vma->vm_start;
printk(KERN_DEBUG "mapping %ld bytes\n", len);
vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
kernel_vp = ioremap(PHYS_MEM_ADDR, 1<<20);
rc = remap_pfn_range(vma, vma->vm_start,
(PHYS_MEM_ADDR)>>PAGE_SHIFT,
len, vma->vm_page_prot);
return (rc < 0 ? rc : 0);
}
I've stripped out some comments but otherwise, this is it.
I've tried both with and without ioremap, both fail in the
same way. I've changed the vma->vm_page_prot a number of
ways. In this example, I had knocked the size of ioremap
(and the flush) to 1MB to see if it was a size issue, but
the kernel gives an error as soon as the first dcbf
instruction is executed in the flush loop.
Then I've got an ioctl to flush
case CACHE_FLUSH:
{
u_int32_t phys_start = phys_mem_addr<<20;
u_int32_t phys_stop = (phys_mem_addr +
phys_mem_size)<<20;
//flush_dcache_range(phys_start, phys_stop-1);
flush_dcache_range(kernel_vp, kernel_vp +
(1<<20));
}
break;
kernel_vp is a virtual pointer from ioremap (only 1MB in
size). The phys_stop and phys_start is the physical address
range, which I think might be wrong. It does not work in
either case anyway.
I'd really like to map 1G, make it cachable and do the flush
and invalidate on demand ... what am I missing? Here is the
kernel dump
##########Unable to handle kernel paging request for data at
address 0x40000000
##Faulting instruction address: 0xc000c398
##Oops: Kernel access of bad area, sig: 11 [#1]
PowerPC 44x Platform
last sysfs file:
/sys/devices/plb.0/opb.3/4ef600400.i2c/i2c-0/0-0022/gpio/gpio223/value
Modules linked in: tan_mpt2sas tanomem [last unloaded:
tanomem]
NIP: c000c398 LR: f4fd91d4 CTR: 02000000
REGS: ebad7dc0 TRAP: 0300 Not tainted
(2.6.37.6-tanisys-sx2-24099)
MSR: 00029000 <EE,ME,CE> CR: 48040242 XER: 00000001
DEAR: 40000000, ESR: 00800000
TASK = ebceb0c0[2720] 'testapplication' THREAD: ebad6000
GPR00: 00000400 ebad7e70 ebceb0c0 40000000 02000000 0000001f
28040244 101132b0
GPR08: 0002d000 f4fd9590 ebbda180 c000c37c 00000000 101a47c8
00000003 bfa4d229
GPR16: bfd98f84 1003a5f8 101416c0 101414b0 bfa4baa8 bfa4bb1c
bfa4bd00 00000000
GPR24: 00000000 101a1514 bfa4d69c ebbda180 fffffff7 c0045402
28040244 28040244
NIP [c000c398] invalidate_dcache_range+0x1c/0x30
LR [f4fd91d4] tanomem_ioctl+0xc8/0x200 [tanomem]
Call Trace:
[ebad7e70] [00029000] 0x29000 (unreliable)
[ebad7e90] [c00ac98c] vfs_ioctl+0x40/0x64
[ebad7ea0] [c00acb7c] do_vfs_ioctl+0x88/0x6fc
[ebad7f10] [c00ad230] sys_ioctl+0x40/0x74
[ebad7f40] [c000c7dc] ret_from_syscall+0x0/0x3c
--- Exception: c01 at 0x1011ad90
LR = 0x10021508
Instruction dump:
7c0018ac 38630020 4200fff8 7c0004ac 4e800020 38a0001f
7c632878 7c832050
7c842a14 5484d97f 4d820020 7c8903a6 <7c001bac> 38630020
4200fff8 7c0004ac
# Kernel panic - not syncing: Fatal exception
IRebooting in 5 seconds..dentifyDevice ###############
Thanks,
Ayman
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Li Yang-R58472 @ 2012-02-24 2:29 UTC (permalink / raw)
To: Huang Changming-R66093, Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, Wood Scott-B07421
In-Reply-To: <110EED8CC96DFC488B7E717A2027A27C02E4A8@039-SN1MPN1-002.039d.mgd.msft.net>
> > Subject: Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be
> > selectable
> >
> > Huang Changming-R66093 wrote:
> > > I have one similar patch to remove the "select PHYS_64BIT".
> > > http://patchwork.ozlabs.org/patch/132351/
> >
> > That one doesn't update the defconfigs, which means that the default
> > kernel will not have PHYS_64BIT enabled.
> I think it is not necessary to enable the 64BIT, if customer want to
> enable it, he can do it manually.
I agree with Changming that we shouldn't setting PHYS_64BIT by default. Fo=
r the platforms covered by the mpc85xx_defconfig, most user won't need the =
PHYS_64BIT. We shouldn't set the one with worse performance and unnecessar=
y to most people as default. Also all these platforms supports 32-bit mode=
.
- Leo
^ permalink raw reply
* RE: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Huang Changming-R66093 @ 2012-02-24 1:54 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: linuxppc-dev@ozlabs.org, Li Yang-R58472, Wood Scott-B07421
In-Reply-To: <4F463006.7060507@freescale.com>
> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Thursday, February 23, 2012 8:25 PM
> To: Huang Changming-R66093
> Cc: galak@kernel.crashing.org; benh@kernel.crashing.org; Wood Scott-
> B07421; Li Yang-R58472; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be
> selectable
>=20
> Huang Changming-R66093 wrote:
> > I have one similar patch to remove the "select PHYS_64BIT".
> > http://patchwork.ozlabs.org/patch/132351/
>=20
> That one doesn't update the defconfigs, which means that the default
> kernel will not have PHYS_64BIT enabled.
I think it is not necessary to enable the 64BIT,
if customer want to enable it, he can do it manually.=20
^ permalink raw reply
* Re: [PATCH v3 22/25] irq_domain/x86: Convert x86 (embedded) to use common irq_domain
From: Grant Likely @ 2012-02-23 21:39 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Stephen Rothwell, devicetree-discuss, linux-kernel, Rob Herring,
Milton Miller, Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <4F46AE07.2000105@linutronix.de>
On Thu, Feb 23, 2012 at 10:22:15PM +0100, Sebastian Andrzej Siewior wrote:
> On 02/23/2012 08:56 PM, Grant Likely wrote:
> >On Wed, Feb 1, 2012 at 11:06 AM, Grant Likely<grant.likely@secretlab.ca> wrote:
> >>On Wed, Feb 1, 2012 at 7:17 AM, Sebastian Andrzej Siewior
> >><bigeasy@linutronix.de> wrote:
> >>>* Grant Likely | 2012-01-30 12:58:42 [-0700]:
> >>>
> >>>>Ugh. This isn't easy. The legacy mapping really needs all the
> >>>
> >>>Feel free to merge this patch. I don't have the time to look at this now
> >>>so I take a look at the ioapic later.
> >>
> >>There's no rush here. I can leave it as-is with IRQ_DOMAIN turned off
> >>for x86 for now.
> >
> >Turns out I have to enable IRQ_DOMAIN for x86 because the TI TWL4030
> >driver needs it. I do need to apply this patch. Until something
> >better can be implemented, can I change ioapic_add_ofnode() so that it
> >allocates all irq_descs immediately. It's not ideal, but every other
> >approach I've looked at results in nasty hacks.
> >
> >Looking at the ioapic code, it appears to handle preallocated
> >irq_descs gracefully.
>
> Please merge your initial patch as-it.
Okay, will do.
> >Does adding this loop help (apologies if it is whitespace damaged, I
> >cut&paste it):
> >
> >diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
> >index 3ae2ced..89c1310 100644
> >--- a/arch/x86/kernel/devicetree.c
> >+++ b/arch/x86/kernel/devicetree.c
> >@@ -345,7 +345,7 @@ const struct irq_domain_ops ioapic_irq_domain_ops = {
> > static void __init ioapic_add_ofnode(struct device_node *np)
> > {
> > struct resource r;
> >- int i, ret;
> >+ int i, j, ret;
> >
> > ret = of_address_to_resource(np, 0,&r);
> > if (ret) {
> >@@ -361,6 +361,14 @@ static void __init ioapic_add_ofnode(struct
> >device_node *np)
> >
> > gsi_cfg = mp_ioapic_gsi_routing(i);
> >
> >+ /*
> >+ * Preallocate irq_descs so that the legacy mapping
> >+ * works, but don't set them up.
> >+ * io_apic_setup_irq_pin_once() will finish the set up.
> >+ */
> >+ for (j = 0; j< 32; j++)
>
> It is not 32. If I remember correctly the first ioapic had 24 pins so
> did the second. This is ioapic specifc.
Is this better (bigger diffstat, but mostly due to renaming a local varable):
---
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 3ae2ced..4028dc6 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -345,7 +345,7 @@ const struct irq_domain_ops ioapic_irq_domain_ops = {
static void __init ioapic_add_ofnode(struct device_node *np)
{
struct resource r;
- int i, ret;
+ int i, irq, ret;
ret = of_address_to_resource(np, 0, &r);
if (ret) {
@@ -357,13 +357,22 @@ static void __init ioapic_add_ofnode(struct device_node *np)
for (i = 0; i < nr_ioapics; i++) {
if (r.start == mpc_ioapic_addr(i)) {
struct irq_domain *id;
- struct mp_ioapic_gsi *gsi_cfg;
-
- gsi_cfg = mp_ioapic_gsi_routing(i);
-
- id = irq_domain_add_legacy(np, 32, gsi_cfg->gsi_base, 0,
- &ioapic_irq_domain_ops,
- (void*)i);
+ struct mp_ioapic_gsi *cfg;
+
+ cfg = mp_ioapic_gsi_routing(i);
+
+ /*
+ * Preallocate irq_descs so that the legacy mapping
+ * works, but don't set them up.
+ * io_apic_setup_irq_pin_once() will finish the set up.
+ */
+ for (irq = cfg->gsi_base; irq <= cfg->gsi_end; irq++)
+ irq_alloc_desc_at(irq, cpu_to_node(0));
+
+ id = irq_domain_add_legacy(np,
+ cfg->gsi_end - cfg->gsi_base + 1,
+ cfg->gsi_base, 0,
+ &ioapic_irq_domain_ops, (void*)i);
BUG_ON(!id);
return;
}
^ permalink raw reply related
* Re: [PATCH v3 22/25] irq_domain/x86: Convert x86 (embedded) to use common irq_domain
From: Sebastian Andrzej Siewior @ 2012-02-23 21:22 UTC (permalink / raw)
To: Grant Likely
Cc: Stephen Rothwell, devicetree-discuss, linux-kernel, Rob Herring,
Milton Miller, Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <CACxGe6tQvBZBMynXbknWx1o1s5TPEN7rXrNOOAjC5F_dz9B51w@mail.gmail.com>
On 02/23/2012 08:56 PM, Grant Likely wrote:
> On Wed, Feb 1, 2012 at 11:06 AM, Grant Likely<grant.likely@secretlab.ca> wrote:
>> On Wed, Feb 1, 2012 at 7:17 AM, Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de> wrote:
>>> * Grant Likely | 2012-01-30 12:58:42 [-0700]:
>>>
>>>> Ugh. This isn't easy. The legacy mapping really needs all the
>>>
>>> Feel free to merge this patch. I don't have the time to look at this now
>>> so I take a look at the ioapic later.
>>
>> There's no rush here. I can leave it as-is with IRQ_DOMAIN turned off
>> for x86 for now.
>
> Turns out I have to enable IRQ_DOMAIN for x86 because the TI TWL4030
> driver needs it. I do need to apply this patch. Until something
> better can be implemented, can I change ioapic_add_ofnode() so that it
> allocates all irq_descs immediately. It's not ideal, but every other
> approach I've looked at results in nasty hacks.
>
> Looking at the ioapic code, it appears to handle preallocated
> irq_descs gracefully.
Please merge your initial patch as-it.
> Does adding this loop help (apologies if it is whitespace damaged, I
> cut&paste it):
>
> diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
> index 3ae2ced..89c1310 100644
> --- a/arch/x86/kernel/devicetree.c
> +++ b/arch/x86/kernel/devicetree.c
> @@ -345,7 +345,7 @@ const struct irq_domain_ops ioapic_irq_domain_ops = {
> static void __init ioapic_add_ofnode(struct device_node *np)
> {
> struct resource r;
> - int i, ret;
> + int i, j, ret;
>
> ret = of_address_to_resource(np, 0,&r);
> if (ret) {
> @@ -361,6 +361,14 @@ static void __init ioapic_add_ofnode(struct
> device_node *np)
>
> gsi_cfg = mp_ioapic_gsi_routing(i);
>
> + /*
> + * Preallocate irq_descs so that the legacy mapping
> + * works, but don't set them up.
> + * io_apic_setup_irq_pin_once() will finish the set up.
> + */
> + for (j = 0; j< 32; j++)
It is not 32. If I remember correctly the first ioapic had 24 pins so
did the second. This is ioapic specifc.
>
> g.
Sebastian
^ permalink raw reply
* Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses
From: Bjorn Helgaas @ 2012-02-23 20:51 UTC (permalink / raw)
To: Jesse Barnes
Cc: linux-arch, Tony Luck, linuxppc-dev, linux-kernel,
Dominik Brodowski, Linus Torvalds, Paul Mackerras, linux-pci,
Andrew Morton, Yinghai Lu
In-Reply-To: <20120223122536.6a2a7a6b@jbarnes-desktop>
On Thu, Feb 23, 2012 at 12:25 PM, Jesse Barnes <jbarnes@virtuousgeek.org> w=
rote:
> On Fri, 10 Feb 2012 08:35:58 +1100
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> On Thu, 2012-02-09 at 11:24 -0800, Bjorn Helgaas wrote:
>> > My point is that the interface between the arch and the PCI core
>> > should be simply the arch telling the core "this is the range of bus
>> > numbers you can use." =A0If the firmware doesn't give you the HW limit=
s,
>> > that's the arch's problem. =A0If you want to assume 0..255 are
>> > available, again, that's the arch's decision.
>> >
>> > But the answer to the question "what bus numbers are available to me"
>> > depends only on the host bridge HW configuration. =A0It does not depen=
d
>> > on what pci_scan_child_bus() found. =A0Therefore, I think we can come =
up
>> > with a design where pci_bus_update_busn_res_end() is unnecessary.
>>
>> In an ideal world yes. In a world where there are reverse engineered
>> platforms on which we aren't 100% sure how thing actually work under the
>> hood and have the code just adapt on "what's there" (and try to fix it
>> up -sometimes-), thinks can get a bit murky :-)
>>
>> But yes, I see your point. As for what is the "correct" setting that
>> needs to be done so that the patch doesn't end up a regression for us,
>> I'll have to dig into some ancient HW to dbl check a few things. I hope
>> 0...255 will just work but I can't guarantee it.
>>
>> What I'll probably do is constraint the core to the values in
>> hose->min/max, and update selected platforms to put 0..255 in there when
>> I know for sure they can cope.
>
> But I think the point is, can't we intiialize the busn resource after
> the first & last bus numbers have been determined? =A0E.g. rather than
> Yinghai's current:
> + =A0 =A0 =A0 pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_=
busno);
> +
> =A0 =A0 =A0 =A0/* Get probe mode and perform scan */
> =A0 =A0 =A0 =A0mode =3D PCI_PROBE_NORMAL;
> =A0 =A0 =A0 =A0if (node && ppc_md.pci_probe_mode)
> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_control=
ler *hose)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0of_scan_bus(node, bus);
> =A0 =A0 =A0 =A0}
>
> - =A0 =A0 =A0 if (mode =3D=3D PCI_PROBE_NORMAL)
> + =A0 =A0 =A0 if (mode =3D=3D PCI_PROBE_NORMAL) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pci_bus_update_busn_res_end(bus, 255);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hose->last_busno =3D bus->subordinate =3D =
pci_scan_child_bus(bus);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pci_bus_update_busn_res_end(bus, bus->subor=
dinate);
> + =A0 =A0 =A0 }
>
> we'd have something more like:
>
> =A0 =A0 =A0 =A0/* Get probe mode and perform scan */
> =A0 =A0 =A0 =A0mode =3D PCI_PROBE_NORMAL;
> =A0 =A0 =A0 =A0if (node && ppc_md.pci_probe_mode)
> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_control=
ler *hose)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0of_scan_bus(node, bus);
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0if (mode =3D=3D PCI_PROBE_NORMAL)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hose->last_busno =3D bus->subordinate =3D =
pci_scan_child_bus(bus);
>
> + =A0 =A0 =A0 pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_=
busno);
>
> since we should have the final bus range by then? =A0Setting the end to
> 255 and then changing it again doesn't make sense; and definitely makes
> the code hard to follow.
I have two issues here:
1) hose->last_busno is currently the highest bus number found by
pci_scan_child_bus(). If I understand correctly,
pci_bus_insert_busn_res() is supposed to update the core's idea of the
host bridge's bus number aperture. (Actually, I guess it just updates
the *end* of the aperture, since we supply the start directly to
pci_scan_root_bus()). The aperture and the highest bus number we
found are not related, except that we should have:
hose->first_busno <=3D bus->subordinate <=3D hose->last_busno
If we set the aperture to [first_busno - last_busno], we artificially
prevent some hotplug.
2) We already have a way to add resources to a root bus: the
pci_add_resource() used to add I/O port and MMIO apertures. I think
it'd be a lot simpler to just use that same interface for the bus
number aperture, e.g.,
pci_add_resource(&resources, hose->io_space);
pci_add_resource(&resources, hose->mem_space);
pci_add_resource(&resources, hose->busnr_space);
bus =3D pci_scan_root_bus(dev, next_busno, pci_ops, sysdata, &resources=
);
This is actually a bit redundant, since "next_busno" should be the
same as hose->busnr_space->start. So if we adopted this approach, we
might want to eventually drop the "next_busno" argument.
Bjorn
^ permalink raw reply
* Re: in_be32() etc
From: Grant Likely @ 2012-02-23 20:27 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <20120223202534.GE22562@n2100.arm.linux.org.uk>
On Thu, Feb 23, 2012 at 1:25 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Feb 24, 2012 at 07:18:59AM +1100, Benjamin Herrenschmidt wrote:
>> On Thu, 2012-02-23 at 11:29 +0000, Russell King - ARM Linux wrote:
>> > What's this stuff doing in generic drivers?
>>
>> Well, I suppose that's because the xilinx stuff used to be ppc
>> only ? :-)
>
> Note that's just the first one grep turned up. =A0I've no idea which spec=
ific
> drivers the ST SPEAr people are wanting these accessors for (they didn't
> include that information in their submission adding them to ARM.)
>
>> > See drivers/gpio/gpio-xilinx.c:
>> > static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
>> > {
>> > =A0 =A0 =A0 =A0 struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(g=
c);
>> >
>> > =A0 =A0 =A0 =A0 return (in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) >> gp=
io) & 1;
>> > }
>> >
>> > include/linux/of_gpio.h:
>> > struct of_mm_gpio_chip {
>> > =A0 =A0 =A0 =A0 struct gpio_chip gc;
>> > =A0 =A0 =A0 =A0 void (*save_regs)(struct of_mm_gpio_chip *mm_gc);
>> > =A0 =A0 =A0 =A0 void __iomem *regs;
>> > };
>> >
>> > Why am I being asked to add in_be32() etc to ARMs io.h ? =A0Why do we =
need
>> > yet another set of IO accessors? =A0Is there something wrong with
>> > ioread*()/ioread*be() etc?
>>
>> Nope, nothing wrong with them, the driver should be fixed. in_be* is
>> historical ppc stuff.
>>
>> > My guess is this stems from a lack of proper review
>>
>> That or history. Our readX/writeX used to be more PCI specific (have
>> infrastructure to work around PCI bridge bugs) which some drivers
>> avoided using the in_/out_ variants, in some case it's just pure
>> history, etc... Some of these things are ancient.
>
> So, if I tell the SPEAr people that any driver they come across using
> these old in_XX should be converted to use ioread*() you'll be happy
> for that to happen?
yes
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: in_be32() etc
From: Russell King - ARM Linux @ 2012-02-23 20:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1330028339.20389.28.camel@pasglop>
On Fri, Feb 24, 2012 at 07:18:59AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2012-02-23 at 11:29 +0000, Russell King - ARM Linux wrote:
> > What's this stuff doing in generic drivers?
>
> Well, I suppose that's because the xilinx stuff used to be ppc
> only ? :-)
Note that's just the first one grep turned up. I've no idea which specific
drivers the ST SPEAr people are wanting these accessors for (they didn't
include that information in their submission adding them to ARM.)
> > See drivers/gpio/gpio-xilinx.c:
> > static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
> > {
> > struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> >
> > return (in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) >> gpio) & 1;
> > }
> >
> > include/linux/of_gpio.h:
> > struct of_mm_gpio_chip {
> > struct gpio_chip gc;
> > void (*save_regs)(struct of_mm_gpio_chip *mm_gc);
> > void __iomem *regs;
> > };
> >
> > Why am I being asked to add in_be32() etc to ARMs io.h ? Why do we need
> > yet another set of IO accessors? Is there something wrong with
> > ioread*()/ioread*be() etc?
>
> Nope, nothing wrong with them, the driver should be fixed. in_be* is
> historical ppc stuff.
>
> > My guess is this stems from a lack of proper review
>
> That or history. Our readX/writeX used to be more PCI specific (have
> infrastructure to work around PCI bridge bugs) which some drivers
> avoided using the in_/out_ variants, in some case it's just pure
> history, etc... Some of these things are ancient.
So, if I tell the SPEAr people that any driver they come across using
these old in_XX should be converted to use ioread*() you'll be happy
for that to happen?
^ permalink raw reply
* Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses
From: Jesse Barnes @ 2012-02-23 20:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-arch, Tony Luck, Yinghai Lu, linuxppc-dev, linux-kernel,
Dominik Brodowski, Paul Mackerras, linux-pci, Bjorn Helgaas,
Andrew Morton, Linus Torvalds
In-Reply-To: <1328823358.2903.77.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]
On Fri, 10 Feb 2012 08:35:58 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Thu, 2012-02-09 at 11:24 -0800, Bjorn Helgaas wrote:
> > My point is that the interface between the arch and the PCI core
> > should be simply the arch telling the core "this is the range of bus
> > numbers you can use." If the firmware doesn't give you the HW limits,
> > that's the arch's problem. If you want to assume 0..255 are
> > available, again, that's the arch's decision.
> >
> > But the answer to the question "what bus numbers are available to me"
> > depends only on the host bridge HW configuration. It does not depend
> > on what pci_scan_child_bus() found. Therefore, I think we can come up
> > with a design where pci_bus_update_busn_res_end() is unnecessary.
>
> In an ideal world yes. In a world where there are reverse engineered
> platforms on which we aren't 100% sure how thing actually work under the
> hood and have the code just adapt on "what's there" (and try to fix it
> up -sometimes-), thinks can get a bit murky :-)
>
> But yes, I see your point. As for what is the "correct" setting that
> needs to be done so that the patch doesn't end up a regression for us,
> I'll have to dig into some ancient HW to dbl check a few things. I hope
> 0...255 will just work but I can't guarantee it.
>
> What I'll probably do is constraint the core to the values in
> hose->min/max, and update selected platforms to put 0..255 in there when
> I know for sure they can cope.
But I think the point is, can't we intiialize the busn resource after
the first & last bus numbers have been determined? E.g. rather than
Yinghai's current:
+ pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
+
/* Get probe mode and perform scan */
mode = PCI_PROBE_NORMAL;
if (node && ppc_md.pci_probe_mode)
@@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
of_scan_bus(node, bus);
}
- if (mode == PCI_PROBE_NORMAL)
+ if (mode == PCI_PROBE_NORMAL) {
+ pci_bus_update_busn_res_end(bus, 255);
hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+ pci_bus_update_busn_res_end(bus, bus->subordinate);
+ }
we'd have something more like:
/* Get probe mode and perform scan */
mode = PCI_PROBE_NORMAL;
if (node && ppc_md.pci_probe_mode)
@@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
of_scan_bus(node, bus);
}
if (mode == PCI_PROBE_NORMAL)
hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+ pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
since we should have the final bus range by then? Setting the end to
255 and then changing it again doesn't make sense; and definitely makes
the code hard to follow.
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: in_be32() etc
From: Benjamin Herrenschmidt @ 2012-02-23 20:18 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20120223112915.GT22562@n2100.arm.linux.org.uk>
On Thu, 2012-02-23 at 11:29 +0000, Russell King - ARM Linux wrote:
> What's this stuff doing in generic drivers?
Well, I suppose that's because the xilinx stuff used to be ppc
only ? :-)
> See drivers/gpio/gpio-xilinx.c:
> static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
> {
> struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>
> return (in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) >> gpio) & 1;
> }
>
> include/linux/of_gpio.h:
> struct of_mm_gpio_chip {
> struct gpio_chip gc;
> void (*save_regs)(struct of_mm_gpio_chip *mm_gc);
> void __iomem *regs;
> };
>
> Why am I being asked to add in_be32() etc to ARMs io.h ? Why do we need
> yet another set of IO accessors? Is there something wrong with
> ioread*()/ioread*be() etc?
Nope, nothing wrong with them, the driver should be fixed. in_be* is
historical ppc stuff.
> My guess is this stems from a lack of proper review
That or history. Our readX/writeX used to be more PCI specific (have
infrastructure to work around PCI bridge bugs) which some drivers
avoided using the in_/out_ variants, in some case it's just pure
history, etc... Some of these things are ancient.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v3 22/25] irq_domain/x86: Convert x86 (embedded) to use common irq_domain
From: Grant Likely @ 2012-02-23 19:56 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Stephen Rothwell, devicetree-discuss, linux-kernel, Rob Herring,
Milton Miller, Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <CACxGe6vZxWXUG-WJFWDaoH3+PYBGWOXb291_jn9TdjrSF=p_JQ@mail.gmail.com>
On Wed, Feb 1, 2012 at 11:06 AM, Grant Likely <grant.likely@secretlab.ca> w=
rote:
> On Wed, Feb 1, 2012 at 7:17 AM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
>> * Grant Likely | 2012-01-30 12:58:42 [-0700]:
>>
>>>Ugh. =A0This isn't easy. =A0The legacy mapping really needs all the
>>
>> Feel free to merge this patch. I don't have the time to look at this now
>> so I take a look at the ioapic later.
>
> There's no rush here. =A0I can leave it as-is with IRQ_DOMAIN turned off
> for x86 for now.
Turns out I have to enable IRQ_DOMAIN for x86 because the TI TWL4030
driver needs it. I do need to apply this patch. Until something
better can be implemented, can I change ioapic_add_ofnode() so that it
allocates all irq_descs immediately. It's not ideal, but every other
approach I've looked at results in nasty hacks.
Looking at the ioapic code, it appears to handle preallocated
irq_descs gracefully.
Does adding this loop help (apologies if it is whitespace damaged, I
cut&paste it):
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 3ae2ced..89c1310 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -345,7 +345,7 @@ const struct irq_domain_ops ioapic_irq_domain_ops =3D {
static void __init ioapic_add_ofnode(struct device_node *np)
{
struct resource r;
- int i, ret;
+ int i, j, ret;
ret =3D of_address_to_resource(np, 0, &r);
if (ret) {
@@ -361,6 +361,14 @@ static void __init ioapic_add_ofnode(struct
device_node *np)
gsi_cfg =3D mp_ioapic_gsi_routing(i);
+ /*
+ * Preallocate irq_descs so that the legacy mapping
+ * works, but don't set them up.
+ * io_apic_setup_irq_pin_once() will finish the set up.
+ */
+ for (j =3D 0; j < 32; j++)
+ irq_alloc_desc_at(gsi_cfg->gsi_base + j,
+ cpu_to_node(0));
id =3D irq_domain_add_legacy(np, 32, gsi_cfg->gsi_base, 0,
&ioapic_irq_domain_ops,
(void*)i);
g.
^ permalink raw reply related
* [PATCH v2 10/12] powerpc/PCI: replace pci_probe_only with pci_flags
From: Bjorn Helgaas @ 2012-02-23 19:43 UTC (permalink / raw)
To: linux-pci; +Cc: linux-arch, linuxppc-dev
In-Reply-To: <20120223194209.20708.54480.stgit@bhelgaas.mtv.corp.google.com>
We already use pci_flags, so this just sets pci_flags directly and removes
the intermediate step of figuring out pci_probe_only, then using it to set
pci_flags.
The PCI core provides a pci_flags definition (currently __weak), so drop
the powerpc definitions in favor of that.
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
arch/powerpc/include/asm/ppc-pci.h | 2 --
arch/powerpc/kernel/pci-common.c | 3 ---
arch/powerpc/kernel/pci_64.c | 5 -----
arch/powerpc/kernel/rtas_pci.c | 10 +++++++---
arch/powerpc/platforms/iseries/pci.c | 2 +-
arch/powerpc/platforms/maple/pci.c | 2 +-
arch/powerpc/platforms/pasemi/pci.c | 2 +-
arch/powerpc/platforms/powernv/pci-ioda.c | 5 ++---
arch/powerpc/platforms/powernv/pci.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 4 ++--
10 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index 6d42297..a96d012 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -45,8 +45,6 @@ extern void init_pci_config_tokens (void);
extern unsigned long get_phb_buid (struct device_node *);
extern int rtas_setup_phb(struct pci_controller *phb);
-extern unsigned long pci_probe_only;
-
/* ---- EEH internal-use-only related routines ---- */
#ifdef CONFIG_EEH
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index cce98d7..6d03da4 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -50,9 +50,6 @@ static int global_phb_number; /* Global phb counter */
/* ISA Memory physical address */
resource_size_t isa_mem_base;
-/* Default PCI flags is 0 on ppc32, modified at boot on ppc64 */
-unsigned int pci_flags = 0;
-
static struct dma_map_ops *pci_dma_ops = &dma_direct_ops;
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index f627eb7..75417fd 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -33,8 +33,6 @@
#include <asm/machdep.h>
#include <asm/ppc-pci.h>
-unsigned long pci_probe_only = 0;
-
/* pci_io_base -- the base address from which io bars are offsets.
* This is the lowest I/O base address (so bar values are always positive),
* and it *must* be the start of ISA space if an ISA bus exists because
@@ -55,9 +53,6 @@ static int __init pcibios_init(void)
*/
ppc_md.phys_mem_access_prot = pci_phys_mem_access_prot;
- if (pci_probe_only)
- pci_add_flags(PCI_PROBE_ONLY);
-
/* On ppc64, we always enable PCI domains and we keep domain 0
* backward compatible in /proc for video cards
*/
diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index 6cd8f01..140735c 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -276,7 +276,7 @@ void __init find_and_init_phbs(void)
pci_devs_phb_init();
/*
- * pci_probe_only and pci_assign_all_buses can be set via properties
+ * PCI_PROBE_ONLY and PCI_REASSIGN_ALL_BUS can be set via properties
* in chosen.
*/
if (of_chosen) {
@@ -284,8 +284,12 @@ void __init find_and_init_phbs(void)
prop = of_get_property(of_chosen,
"linux,pci-probe-only", NULL);
- if (prop)
- pci_probe_only = *prop;
+ if (prop) {
+ if (*prop)
+ pci_add_flags(PCI_PROBE_ONLY);
+ else
+ pci_clear_flags(PCI_PROBE_ONLY);
+ }
#ifdef CONFIG_PPC32 /* Will be made generic soon */
prop = of_get_property(of_chosen,
diff --git a/arch/powerpc/platforms/iseries/pci.c b/arch/powerpc/platforms/iseries/pci.c
index c754128..171b2f3 100644
--- a/arch/powerpc/platforms/iseries/pci.c
+++ b/arch/powerpc/platforms/iseries/pci.c
@@ -868,7 +868,7 @@ void __init iSeries_pcibios_init(void)
/* Install IO hooks */
ppc_pci_io = iseries_pci_io;
- pci_probe_only = 1;
+ pci_add_flags(PCI_PROBE_ONLY);
/* iSeries has no IO space in the common sense, it needs to set
* the IO base to 0
diff --git a/arch/powerpc/platforms/maple/pci.c b/arch/powerpc/platforms/maple/pci.c
index 401e3f3..465ee8f 100644
--- a/arch/powerpc/platforms/maple/pci.c
+++ b/arch/powerpc/platforms/maple/pci.c
@@ -620,7 +620,7 @@ void __init maple_pci_init(void)
}
/* Tell pci.c to not change any resource allocations. */
- pci_probe_only = 1;
+ pci_add_flags(PCI_PROBE_ONLY);
}
int maple_pci_get_legacy_ide_irq(struct pci_dev *pdev, int channel)
diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
index b6a0ec4..b27d886 100644
--- a/arch/powerpc/platforms/pasemi/pci.c
+++ b/arch/powerpc/platforms/pasemi/pci.c
@@ -231,7 +231,7 @@ void __init pas_pci_init(void)
pci_devs_phb_init();
/* Use the common resource allocation mechanism */
- pci_probe_only = 1;
+ pci_add_flags(PCI_PROBE_ONLY);
}
void __iomem *pasemi_pci_getcfgaddr(struct pci_dev *dev, int offset)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 5e155df..fbdd74d 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1299,15 +1299,14 @@ void __init pnv_pci_init_ioda1_phb(struct device_node *np)
/* Setup MSI support */
pnv_pci_init_ioda_msis(phb);
- /* We set both probe_only and PCI_REASSIGN_ALL_RSRC. This is an
+ /* We set both PCI_PROBE_ONLY and PCI_REASSIGN_ALL_RSRC. This is an
* odd combination which essentially means that we skip all resource
* fixups and assignments in the generic code, and do it all
* ourselves here
*/
- pci_probe_only = 1;
ppc_md.pcibios_fixup_phb = pnv_pci_ioda_fixup_phb;
ppc_md.pcibios_enable_device_hook = pnv_pci_enable_device_hook;
- pci_add_flags(PCI_REASSIGN_ALL_RSRC);
+ pci_add_flags(PCI_PROBE_ONLY | PCI_REASSIGN_ALL_RSRC);
/* Reset IODA tables to a clean state */
rc = opal_pci_reset(phb_id, OPAL_PCI_IODA_TABLE_RESET, OPAL_ASSERT_RESET);
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 94ed056..15f4e73 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -561,7 +561,7 @@ void __init pnv_pci_init(void)
{
struct device_node *np;
- pci_set_flags(PCI_CAN_SKIP_ISA_ALIGN);
+ pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN);
/* OPAL absent, try POPAL first then RTAS detection of PHBs */
if (!firmware_has_feature(FW_FEATURE_OPAL)) {
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 386e265..fc2a6f6 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -380,8 +380,8 @@ static void __init pSeries_setup_arch(void)
fwnmi_init();
- /* By default, only probe PCI (can be overriden by rtas_pci */
- pci_probe_only = 1;
+ /* By default, only probe PCI (can be overriden by rtas_pci) */
+ pci_add_flags(PCI_PROBE_ONLY);
/* Find and initialize PCI host bridges */
init_pci_config_tokens();
^ permalink raw reply related
* Re: [PATCH v1 09/11] powerpc/PCI: replace pci_probe_only with pci_flags
From: Bjorn Helgaas @ 2012-02-23 18:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-arch, linux-pci, Olof Johansson, linuxppc-dev
In-Reply-To: <1329946899.20389.9.camel@pasglop>
On Wed, Feb 22, 2012 at 1:41 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2012-02-22 at 11:19 -0700, Bjorn Helgaas wrote:
>> =A0int maple_pci_get_legacy_ide_irq(struct pci_dev *pdev, int channel)
>> diff --git a/arch/powerpc/platforms/pasemi/pci.c
>> b/arch/powerpc/platforms/pasemi/pci.c
>> index b6a0ec4..b27d886 100644
>> --- a/arch/powerpc/platforms/pasemi/pci.c
>> +++ b/arch/powerpc/platforms/pasemi/pci.c
>> @@ -231,7 +231,7 @@ void __init pas_pci_init(void)
>> =A0 =A0 =A0 =A0 pci_devs_phb_init();
>>
>> =A0 =A0 =A0 =A0 /* Use the common resource allocation mechanism */
>> - =A0 =A0 =A0 pci_probe_only =3D 1;
>> + =A0 =A0 =A0 pci_add_flags(PCI_PROBE_ONLY);
>> =A0}
>
> Olof, do you remember why you used to set that on pasemi ?
>
> I would have expected it to be clear, so the kernel can re-assign things
> if needed. We really only want it set for pseries because of the
> hypervisor being a PITA :-)
Speaking of the hypervisor, powerpc has this of_scan_bus() thing which
seems to discover PCI devices using the OF device tree rather than
using PCI config accesses. This seems related to the "pci_probe_only"
idea.
I wonder whether it would be possible/feasible/desirable to implement
this by using config space accessors that would internally look at the
OF device tree. Then we possibly could use the standard PCI scan bus
path. Maybe the core could detect non-changeable BARs somehow and
mark them, e.g., with IORESOURCE_PCI_FIXED.
Just a pie-in-the sky thought for now; I have no concrete plans to do
anything like this, but it seems like it might allow more unification
if it were possible.
Bjorn
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: allow CONFIG_PHYS_64BIT to be selectable
From: Tabi Timur-B04825 @ 2012-02-23 12:24 UTC (permalink / raw)
To: Huang Changming-R66093
Cc: linuxppc-dev@ozlabs.org, Li Yang-R58472, Wood Scott-B07421
In-Reply-To: <110EED8CC96DFC488B7E717A2027A27C02C5E8@039-SN1MPN1-002.039d.mgd.msft.net>
Huang Changming-R66093 wrote:
> I have one similar patch to remove the "select PHYS_64BIT".
> http://patchwork.ozlabs.org/patch/132351/
That one doesn't update the defconfigs, which means that the default=20
kernel will not have PHYS_64BIT enabled.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* in_be32() etc
From: Russell King - ARM Linux @ 2012-02-23 11:29 UTC (permalink / raw)
To: Grant Likely, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev
What's this stuff doing in generic drivers?
See drivers/gpio/gpio-xilinx.c:
static int xgpio_get(struct gpio_chip *gc, unsigned int gpio)
{
struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
return (in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) >> gpio) & 1;
}
include/linux/of_gpio.h:
struct of_mm_gpio_chip {
struct gpio_chip gc;
void (*save_regs)(struct of_mm_gpio_chip *mm_gc);
void __iomem *regs;
};
Why am I being asked to add in_be32() etc to ARMs io.h ? Why do we need
yet another set of IO accessors? Is there something wrong with
ioread*()/ioread*be() etc?
My guess is this stems from a lack of proper review:
/*
* Low level MMIO accessors
*
* This provides the non-bus specific accessors to MMIO. Those are PowerPC
* specific and thus shouldn't be used in generic code. The accessors
* provided here are:
*
* in_8, in_le16, in_be16, in_le32, in_be32, in_le64, in_be64
* out_8, out_le16, out_be16, out_le32, out_be32, out_le64, out_be64
* _insb, _insw_ns, _insl_ns, _outsb, _outsw_ns, _outsl_ns
$ grep '\(out\|in\)_be32' drivers/ -rl
drivers/media/video/fsl-viu.c
drivers/macintosh/via-pmu.c
drivers/gpio/gpio-mpc8xxx.c
drivers/gpio/gpio-mpc5200.c
drivers/gpio/gpio-xilinx.c
drivers/edac/mpc85xx_edac.c
drivers/tty/serial/mpc52xx_uart.c
drivers/tty/serial/ucc_uart.c
drivers/tty/serial/cpm_uart/cpm_uart_core.c
drivers/scsi/mvme16x_scsi.c
drivers/char/xilinx_hwicap/buffer_icap.c
drivers/char/xilinx_hwicap/fifo_icap.c
drivers/rtc/rtc-mpc5121.c
drivers/pcmcia/m8xx_pcmcia.c
drivers/ata/pata_scc.c
drivers/ata/pata_mpc52xx.c
drivers/ide/scc_pata.c
drivers/net/ethernet/xilinx/xilinx_emaclite.c
drivers/net/ethernet/xilinx/ll_temac_main.c
drivers/net/ethernet/freescale/ucc_geth.c
drivers/net/ethernet/freescale/fsl_pq_mdio.c
drivers/net/ethernet/freescale/fec_mpc52xx.c
drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
drivers/net/ethernet/freescale/fs_enet/mii-fec.c
drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
drivers/net/ethernet/freescale/fs_enet/mac-fec.c
drivers/net/ethernet/freescale/fs_enet/mac-scc.c
drivers/net/ethernet/freescale/fs_enet/fs_enet.h
drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
drivers/net/ethernet/freescale/gianfar.h
drivers/net/ethernet/freescale/ucc_geth_ethtool.c
drivers/net/ethernet/tundra/tsi108_eth.h
drivers/net/ethernet/ibm/emac/tah.c
drivers/net/ethernet/ibm/emac/zmii.c
drivers/net/ethernet/ibm/emac/rgmii.c
drivers/net/ethernet/ibm/emac/core.c
drivers/net/ethernet/ibm/emac/debug.c
drivers/net/ethernet/toshiba/spider_net.c
drivers/net/can/flexcan.c
drivers/net/can/mscan/mpc5xxx_can.c
drivers/video/xilinxfb.c
drivers/video/fsl-diu-fb.c
drivers/video/platinumfb.c
drivers/spi/spi-fsl-spi.c
drivers/spi/spi-fsl-lib.h
drivers/spi/spi-mpc52xx-psc.c
drivers/spi/spi-mpc512x-psc.c
drivers/watchdog/mpc8xxx_wdt.c
drivers/watchdog/pika_wdt.c
drivers/mmc/host/sdhci-pltfm.h
drivers/mmc/host/sdhci-of-esdhc.c
drivers/crypto/caam/regs.h
drivers/crypto/talitos.c
drivers/usb/gadget/fsl_qe_udc.c
drivers/usb/gadget/fsl_udc_core.c
drivers/usb/host/fhci-hcd.c
drivers/usb/host/ehci-fsl.c
drivers/usb/host/ehci-ppc-of.c
drivers/usb/host/fhci-tds.c
drivers/usb/host/fsl-mph-dr-of.c
drivers/usb/otg/fsl_otg.c
drivers/dma/fsldma.c
drivers/dma/fsldma.h
drivers/dma/mpc512x_dma.c
drivers/i2c/busses/i2c-mpc.c
drivers/i2c/busses/i2c-cpm.c
drivers/input/serio/xilinx_ps2.c
drivers/isdn/hisax/avm_pci.c
drivers/mtd/nand/ppchameleonevb.c
drivers/mtd/nand/fsl_elbc_nand.c
drivers/mtd/nand/mpc5121_nfc.c
drivers/mtd/nand/ndfc.c
drivers/mtd/nand/socrates_nand.c
^ 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