* Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
From: Sukadev Bhattiprolu @ 2011-12-02 3:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, sbest, paulus, anton
In-Reply-To: <1322789207.3729.45.camel@pasglop>
Benjamin Herrenschmidt [benh@kernel.crashing.org] wrote:
| And an additional comment regarding the rtas bit:
| > +int devmem_is_allowed(unsigned long pfn)
| > +{
| > + if (iomem_is_exclusive(pfn << PAGE_SHIFT))
| > + return 0;
| > + if (!page_is_ram(pfn))
| > + return 1;
| > + if (page_is_rtas(pfn))
| > + return 1;
| > + return 0;
| > +}
|
| This calls it unconditionally... you just broke the build of all !rtas
| platforms. Additionally, putting an extern definition like that in a .c
| file is gross at best....
Oh, Sorry.
|
| Please instead, put in a header something like
|
| #ifdef CONFIG_PPC_RTAS
| extern int page_is_rtas(unsigned long pfn);
| #else
| static inline int page_is_rtas(unsigned long pfn) { }
| #endif
|
| And while at it, call it page_is_rtas_user_buf(); to make it clear what
| we are talking about, ie, not RTAS core per-se but specifically the RMO
| buffer.
Ok. I will rename, move the declaration to <asm/rtas.h> and resend the
incremental patch.
Sukadev
^ permalink raw reply
* Re: [PATCH 10/10] powerpc/mpic: Add in-core support for cascaded MPICs
From: Benjamin Herrenschmidt @ 2011-12-02 3:04 UTC (permalink / raw)
To: Moffett, Kyle D; +Cc: linuxppc-dev, Paul Mackerras, michaele@au1.ibm.com
In-Reply-To: <CDCF68E8-7EF4-4ED7-A1FA-2A2CE74287CB@boeing.com>
On Thu, 2011-12-01 at 19:48 -0600, Moffett, Kyle D wrote:
> Ben,
>
> I fixed the 3 issues that Paul and Michael reported and I can provide them
> to you two different ways, however you would prefer. I can also send the
> patches via email if that's more convenient.
>
> Option 1: Squashed into the the original patches for bisectability:
> git://opensource.exmeritus.com/hww-1u-1a/linux.git mpc85xx/mpic-and-stuff
This option, please just re-send to the list.
Cheers,
Ben.
^ permalink raw reply
* [powerpc] boot up problem
From: Jia Hongtao-B38951 @ 2011-12-02 2:21 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <3EC6F9E0-C847-4DA8-AD8C-313976982A10@kernel.crashing.org>
Hi
I just found that the 'next' branch you mentioned have problem to boot up.
I test it in p1022ds and p1010rdb boards and the result are both the same.
Note that for p1022ds I use "make p1022ds.dtb" to make the dtb file(36bit) =
with 36bit-uboot.
And for p1010rdb I use all 32bit image.
The problem list below:
scsi0 : sata_fsl
ata1: SATA max UDMA/133 irq 74
fsl-sata fffe19000.sata: Sata FSL Platform/CSB Driver init
scsi1 : sata_fsl
ata2: SATA max UDMA/133 irq 41
Fixed MDIO Bus: probed
Unable to handle kernel paging request for data at address 0x00000000
Faulting instruction address: 0xc0451630
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=3D2 P1022 DS
Modules linked in:
NIP: c0451630 LR: c0451618 CTR: 00000007
REGS: ef03fce0 TRAP: 0300 Not tainted (3.2.0-rc3-00099-g883381d)
MSR: 00029000 <EE,ME,CE> CR: 24042022 XER: 00000000
DEAR: 00000000, ESR: 00800000
TASK =3D ef040000[1] 'swapper' THREAD: ef03e000 CPU: 0
GPR00: ef03fd98 ef03fd90 ef040000 ef1ab22c 00000000 00000002 ffeb0000 0000f=
ffe
GPR08: b0541215 00000000 00000000 00000000 24042028 23c406c2 00000000 00000=
000
GPR16: c0000a00 00000014 3fffffff 03ff9000 00000015 7ff3a760 f1044030 fffff=
ff4
GPR24: c053e128 ef1ab230 c056a3a8 ef03e000 ef040000 ef1ab22c 00000000 ef1ab=
228
NIP [c0451630] __mutex_lock_slowpath+0xb4/0x190
LR [c0451618] __mutex_lock_slowpath+0x9c/0x190
Call Trace:
[ef03fdd0] [c0451758] mutex_lock+0x4c/0x50
[ef03fde0] [c02b5124] mdiobus_read+0x38/0x74
[ef03fe00] [c02b41f4] get_phy_id+0x24/0x80
[ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580
[ef03feb0] [c0266120] platform_drv_probe+0x20/0x30
[ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4
[ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0
[ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c
[ef03ff30] [c02647f4] driver_attach+0x24/0x34
[ef03ff40] [c0264444] bus_add_driver+0x1ac/0x274
[ef03ff70] [c02651b0] driver_register+0x88/0x154
[ef03ff90] [c0266450] platform_driver_register+0x68/0x78
[ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28
[ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8
[ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c
[ef03fff0] [c000d878] kernel_thread+0x4c/0x68
Instruction dump:
801c0020 2f800063 419dffe8 3bbf0004 7fa3eb78 48001aad 813f000c 38010008
3b3f0008 901f000c 93210008 9121000c
3800ffff 93810010 7c0004ac
---[ end trace 1643a9a9c5097f8f ]---
Kernel panic - not syncing: Attempted to kill init!
Call Trace:
[ef03fbc0] [c0008044] show_stack+0x44/0x154 (unreliable)
[ef03fc00] [c04532c8] panic+0xa4/0x1d8
[ef03fc50] [c0049a00] do_exit+0x5dc/0x684
[ef03fca0] [c000a6f0] die+0xdc/0x1b4
[ef03fcc0] [c00128d0] bad_page_fault+0xb4/0xfc
[ef03fcd0] [c000ebe4] handle_page_fault+0x7c/0x80
--- Exception: 300 at __mutex_lock_slowpath+0xb4/0x190
LR =3D __mutex_lock_slowpath+0x9c/0x190
[ef03fd90] [00000000] (null) (unreliable)
[ef03fdd0] [c0451758] mutex_lock+0x4c/0x50
[ef03fde0] [c02b5124] mdiobus_read+0x38/0x74
[ef03fe00] [c02b41f4] get_phy_id+0x24/0x80
[ef03fe20] [c02b9de4] fsl_pq_mdio_probe+0x3b4/0x580
[ef03feb0] [c0266120] platform_drv_probe+0x20/0x30
[ef03fec0] [c0264bbc] driver_probe_device+0xa4/0x1d4
[ef03fee0] [c0264da8] __driver_attach+0xbc/0xc0
[ef03ff00] [c0263ac0] bus_for_each_dev+0x60/0x9c
[ef03ff30] [c02647f4] driver_attach+0x24/0x34
[ef03ff40] [c0264444] bus_add_driver+0x1ac/0x274
[ef03ff70] [c02651b0] driver_register+0x88/0x154
[ef03ff90] [c0266450] platform_driver_register+0x68/0x78
[ef03ffa0] [c05d93b8] fsl_pq_mdio_init+0x18/0x28
[ef03ffb0] [c0001eb8] do_one_initcall+0x34/0x1a8
[ef03ffe0] [c05bb82c] kernel_init+0xa0/0x13c
[ef03fff0] [c000d878] kernel_thread+0x4c/0x68
Rebooting in 180 seconds..
Do you or anyone else have any idea about this?
Thanks.
-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]=20
Sent: Thursday, November 24, 2011 3:10 PM
To: Jia Hongtao-B38951
Cc: Li Yang-R58472
Subject: Re: [PATCH 1/2] Unify pci/pcie initialization code
When you do this please do it against my latest upstream 'next' branch on:
http://git.kernel.org/?p=3Dlinux/kernel/git/galak/powerpc.git
git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git
- k
On Nov 24, 2011, at 12:27 AM, Jia Hongtao-B38951 wrote:
> Ok, I got it.
>=20
> - Hongtao
>=20
> -----Original Message-----
> From: Li Yang-R58472
> Sent: Thursday, November 24, 2011 2:06 PM
> To: Jia Hongtao-B38951; Kumar Gala
> Subject: RE: [PATCH 1/2] Unify pci/pcie initialization code
>=20
> Hongtao,
>=20
> Please update all the boards currently using fsl_add_bridge().
>=20
> - Leo
>=20
>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Thursday, November 24, 2011 12:51 PM
>> To: Jia Hongtao-B38951
>> Cc: linuxppc-dev@lists.ozlabs.org; Gala Kumar-B11780; Li Yang-R58472
>> Subject: Re: [PATCH 1/2] Unify pci/pcie initialization code
>>=20
>>=20
>> On Nov 22, 2011, at 12:20 AM, Jia Hongtao-B38951 wrote:
>>=20
>>> Hi Kumar,
>>> We want more comments on this series of patches ([1/2] & [2/2]) to=20
>>> speed
>> up the pushing-to-kernel progress.
>>> Thanks.
>>=20
>> I think the code is fine, but you need to update it for all the=20
>> boards include the 86xx ones.
>>=20
>> - k
>>=20
>>>=20
>>> -----Original Message-----
>>> From: Jia Hongtao-B38951
>>> Sent: Monday, October 31, 2011 1:55 PM
>>> To: linuxppc-dev@lists.ozlabs.org
>>> Cc: Li Yang-R58472; Gala Kumar-B11780; Jia Hongtao-B38951
>>> Subject: [PATCH 1/2] Unify pci/pcie initialization code
>>>=20
>>> In previous version pci/pcie initialization is in platform code=20
>>> which
>> Initialize PCI bridge base on EP/RC or host/agent settings.
>>> We unified pci/pcie initialization as common APIs named=20
>>> fsl_pci_setup
>> which can be called by platform code.
>>>=20
>>> Signed-off-by: Jia Hongtao <B38951@freescale.com>
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> ---
>>> arch/powerpc/platforms/85xx/mpc85xx_ds.c | 30 ++-----------------
>>> arch/powerpc/sysdev/fsl_pci.c | 48
>> ++++++++++++++++++++++++++++++
>>> arch/powerpc/sysdev/fsl_pci.h | 5 +++
>>> 3 files changed, 56 insertions(+), 27 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>> b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> index 10e7db0..7188c0b 100644
>>> --- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
>>> @@ -157,33 +157,12 @@ extern void __init mpc85xx_smp_init(void);=20
>>> #endif
>> static void __init mpc85xx_ds_setup_arch(void) { -#ifdef CONFIG_PCI
>>> - struct device_node *np;
>>> - struct pci_controller *hose;
>>> -#endif
>>> - dma_addr_t max =3D 0xffffffff;
>>> -
>>> if (ppc_md.progress)
>>> ppc_md.progress("mpc85xx_ds_setup_arch()", 0);
>>>=20
>>> -#ifdef CONFIG_PCI
>>> - for_each_node_by_type(np, "pci") {
>>> - if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
>>> - of_device_is_compatible(np, "fsl,mpc8548-pcie") ||
>>> - of_device_is_compatible(np, "fsl,p2020-pcie")) {
>>> - struct resource rsrc;
>>> - of_address_to_resource(np, 0, &rsrc);
>>> - if ((rsrc.start & 0xfffff) =3D=3D primary_phb_addr)
>>> - fsl_add_bridge(np, 1);
>>> - else
>>> - fsl_add_bridge(np, 0);
>>> -
>>> - hose =3D pci_find_hose_for_OF_device(np);
>>> - max =3D min(max, hose->dma_window_base_cur +
>>> - hose->dma_window_size);
>>> - }
>>> - }
>>> + fsl_pci_setup(primary_phb_addr);
>>>=20
>>> +#ifdef CONFIG_PCI
>>> ppc_md.pci_exclude_device =3D mpc85xx_exclude_device; #endif
>>>=20
>>> @@ -192,11 +171,8 @@ static void __init mpc85xx_ds_setup_arch(void)
>> #endif
>>>=20
>>> #ifdef CONFIG_SWIOTLB
>>> - if (memblock_end_of_DRAM() > max) {
>>> + if (memblock_end_of_DRAM() > 0xffffffff)
>>> ppc_swiotlb_enable =3D 1;
>>> - set_pci_dma_ops(&swiotlb_dma_ops);
>>> - ppc_md.pci_dma_dev_setup =3D pci_dma_dev_setup_swiotlb;
>>> - }
>>> #endif
>>>=20
>>> printk("MPC85xx DS board from Freescale Semiconductor\n"); diff --
>> git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c=20
>> index 80b8b7a..4d4536f 100644
>>> --- a/arch/powerpc/sysdev/fsl_pci.c
>>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>>> @@ -402,6 +402,54 @@ int __init fsl_add_bridge(struct device_node=20
>>> *dev,
>> int is_primary) } #endif /* CONFIG_FSL_SOC_BOOKE || CONFIG_PPC_86xx=20
>> */
>>>=20
>>> +static struct of_device_id pci_ids[] =3D {
>>> + { .compatible =3D "fsl,mpc8540-pci", },
>>> + { .compatible =3D "fsl,mpc8548-pcie", },
>>> + {},
>>> +};
>>> +
>>> +/**
>>> + * fsl_pci_setup - Initialization for PCI
>>> + * @primary_phb_addr: primary bus address
>>> + *
>>> + * Add bridge if pci controller is a host */ void=20
>>> +fsl_pci_setup(int
>>> +primary_phb_addr) {
>>> + struct device_node *np;
>>> + struct pci_controller *hose;
>>> + dma_addr_t min_dma_addr =3D 0xffffffff;
>>> +
>>> + for_each_node_by_type(np, "pci") {
>>> + if (of_match_node(pci_ids, np)) {
>>> + struct resource rsrc;
>>> + of_address_to_resource(np, 0, &rsrc);
>>> + if ((rsrc.start & 0xfffff) =3D=3D primary_phb_addr)
>>> + fsl_add_bridge(np, 1);
>>> + else
>>> + fsl_add_bridge(np, 0);
>>> +
>>> + hose =3D pci_find_hose_for_OF_device(np);
>>> + min_dma_addr =3D min(min_dma_addr,
>>> + hose->dma_window_base_cur
>>> + + hose->dma_window_size);
>>> +
>>> + }
>>> + }
>>> +
>>> +#ifdef CONFIG_SWIOTLB
>>> + /*
>>> + * if we couldn't map all of DRAM via the dma windows we need
>> SWIOTLB
>>> + * to handle buffers located outside of dma capable memory region
>>> + */
>>> + if (memblock_end_of_DRAM() > min_dma_addr) {
>>> + ppc_swiotlb_enable =3D 1;
>>> + set_pci_dma_ops(&swiotlb_dma_ops);
>>> + ppc_md.pci_dma_dev_setup =3D pci_dma_dev_setup_swiotlb;
>>> + }
>>> +#endif
>>> +}
>>> +
>>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
>> quirk_fsl_pcie_header);
>>>=20
>>> #if defined(CONFIG_PPC_83xx) || defined(CONFIG_PPC_MPC512x) diff=20
>>> --git
>> a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h index
>> a39ed5c..775ea21 100644
>>> --- a/arch/powerpc/sysdev/fsl_pci.h
>>> +++ b/arch/powerpc/sysdev/fsl_pci.h
>>> @@ -89,6 +89,11 @@ struct ccsr_pci { };
>>>=20
>>> extern int fsl_add_bridge(struct device_node *dev, int is_primary);
>>> +#ifndef CONFIG_PCI
>>> +#define fsl_pci_setup(p)
>>> +#else
>>> +extern void fsl_pci_setup(int primary_phb_addr); #endif
>>> extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int
>> mpc83xx_add_bridge(struct device_node *dev);
>>> u64 fsl_pci_immrbar_base(struct pci_controller *hose);
>>> --
>>> 1.7.5.1
>>>=20
>>>=20
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@lists.ozlabs.org
>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>>=20
>=20
^ permalink raw reply
* Re: [PATCH 10/10] powerpc/mpic: Add in-core support for cascaded MPICs
From: Moffett, Kyle D @ 2011-12-02 1:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, michaele@au1.ibm.com, Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <1322722788.5215.3.camel@concordia>
Ben,
I fixed the 3 issues that Paul and Michael reported and I can provide them
to you two different ways, however you would prefer. I can also send the
patches via email if that's more convenient.
Option 1: Squashed into the the original patches for bisectability:
git://opensource.exmeritus.com/hww-1u-1a/linux.git mpc85xx/mpic-and-stuff
Option 2: As a fixup patch on the end:
git://opensource.exmeritus.com/hww-1u-1a/linux.git mpc85xx/mpic-and-stuff=
-fixup
You can also use HTTP if you would prefer:
http://opensource.exmeritus.com/git/hww-1u-1a/linux.git
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
From: Paul Gortmaker @ 2011-12-02 1:42 UTC (permalink / raw)
To: Alan Cox; +Cc: gregkh, linux-kernel, linux-serial, scottwood, linuxppc-dev
In-Reply-To: <20111202005752.10ce5eea@bob.linux.org.uk>
On Thu, Dec 1, 2011 at 7:57 PM, Alan Cox <alan@linux.intel.com> wrote:
>
>> @@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct
>> uart_8250_port *up)
>> =A0 =A0 =A0 spin_lock_irqsave(&up->port.lock, flags);
>>
>> - =A0 =A0 status =3D serial_inp(up, UART_LSR);
>> + =A0 =A0 /* Workaround for IRQ storm errata on break with Freescale
>> 16550 */
>> + =A0 =A0 if (UART_BUG_FSLBK & up->port.bugs && up->lsr_last &
>> UART_LSR_BI) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 up->lsr_last &=3D ~UART_LSR_BI;
>> + =A0 =A0 =A0 =A0 =A0 =A0 serial_inp(up, UART_RX);
>> + =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&up->port.lock, flags);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 status =3D up->lsr_last =3D serial_inp(up, UART_LSR);
>
> We've now had a recent pile of IRQ function changes adding more quirk
> bits and special casing. This doesn't scale. We either need to make
> handle_port a method the specific drivers can override or you could
> hide the mess in your serial_inp implementation and not touch any core
> code.
To be fair, this one is zero cost for !PPC, but I understand your point,
and the idea of hiding it somehow in serial_inp was something that
never crossed my mind. I'll look into seeing if I can abuse that.
>
> I really don't mind which but I suspect dealing with it in your
> serial_inp handler so that when you read UART_LSR you do the fixup
> might be simplest providing you can just do that.
>
> Sorting out a ->handle_port override is probably ultimately the right
> thing to do and then we can push some of the other funnies out further.
>
> At this point it's becoming increasingly clear that 8250 UART cloners
> are both very bad at cloning the things accurately, and very busy adding
> extra useful features so we need to start to treat 8250.c as a library
> you can wrap.
Sad but true. It is like a time warp back to lib8390.c --- whee.
P.
>
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" i=
n
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] powerpc/nvram: Add spinlock to oops_to_nvram to prevent oops in compression code.
From: Jim Keniston @ 2011-12-02 1:37 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev
In-Reply-To: <20111201124645.24c6e54f@kryten>
On Thu, 2011-12-01 at 12:46 +1100, Anton Blanchard wrote:
> When issuing a system reset we almost always oops in the oops_to_nvram
> code because multiple CPUs are using the deflate work area. Add a
> spinlock to protect it.
>
> To play it safe I'm using trylock to avoid locking up if the NVRAM
> code oopses. This means we might miss multiple CPUs oopsing at exactly
> the same time but I think it's best to play it safe for now. Once we
> are happy with the reliability we can change it to a full spinlock.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
Acked-by: Jim Keniston <jkenisto@us.ibm.com>
> ---
>
> Index: linux-build/arch/powerpc/platforms/pseries/nvram.c
> ===================================================================
> --- linux-build.orig/arch/powerpc/platforms/pseries/nvram.c 2011-12-01 09:44:27.205568463 +1100
> +++ linux-build/arch/powerpc/platforms/pseries/nvram.c 2011-12-01 12:36:49.334478156 +1100
> @@ -634,6 +634,8 @@ static void oops_to_nvram(struct kmsg_du
> {
> static unsigned int oops_count = 0;
> static bool panicking = false;
> + static DEFINE_SPINLOCK(lock);
> + unsigned long flags;
> size_t text_len;
> unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ;
> int rc = -1;
> @@ -664,6 +666,9 @@ static void oops_to_nvram(struct kmsg_du
> if (clobbering_unread_rtas_event())
> return;
>
> + if (!spin_trylock_irqsave(&lock, flags))
> + return;
> +
> if (big_oops_buf) {
> text_len = capture_last_msgs(old_msgs, old_len,
> new_msgs, new_len, big_oops_buf, big_oops_buf_sz);
> @@ -679,4 +684,6 @@ static void oops_to_nvram(struct kmsg_du
>
> (void) nvram_write_os_partition(&oops_log_partition, oops_buf,
> (int) (sizeof(*oops_len) + *oops_len), err_type, ++oops_count);
> +
> + spin_unlock_irqrestore(&lock, flags);
> }
>
^ permalink raw reply
* Re: [PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
From: Paul Gortmaker @ 2011-12-02 1:32 UTC (permalink / raw)
To: Alan Cox; +Cc: gregkh, linux-kernel, linux-serial, scottwood, linuxppc-dev
In-Reply-To: <20111202005150.2e9fde43@bob.linux.org.uk>
On Thu, Dec 1, 2011 at 7:51 PM, Alan Cox <alan@linux.intel.com> wrote:
>> Make the bugs field part of the globally visible struct
>> uart_port and remove the 8250 specific one.
>
> Except all the bits in it are 8250 specific things or names that are
> meaningless in generic form - no. I also don't want to encourage flags
> and bug bits. We already have too many and its making the code a mess.
> So right now we want less not more.
The bits in "bugs" are only passed through -- the idea is that there is
no "interpretation" of them by any generic layer -- only that they prop
from the arch down to the driver which knows what they mean.
I can understand the "want less not more" mentality -- I was thinking
the same thing when I was looking at the replication of fields between
uart_port and uart_8250_port, and toying with the idea of helping clean
that up.... (separate topic, to be sure.)
If you have an idea in mind how arch/platform code should cleanly
pass data about known uart bugs to the uart driver, then let me know
what you have in mind. I've no real attachment to what I proposed
here -- it just happened to be the solution I thought would be the least
offensive. If there is a better idea floating around, I'll go ahead and
try to implement it.
Thanks,
Paul.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" i=
n
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
From: Benjamin Herrenschmidt @ 2011-12-02 1:26 UTC (permalink / raw)
To: Sukadev Bhattiprolu; +Cc: linuxppc-dev, sbest, paulus, anton
In-Reply-To: <20111202001141.GA14860@us.ibm.com>
And an additional comment regarding the rtas bit:
> #ifdef CONFIG_PPC_SMLPAR
> void arch_free_page(struct page *page, int order);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index d5ca823..07cf2cf 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
> return 0;
> }
>
> +int page_is_rtas(unsigned long pfn)
> +{
> + unsigned long paddr = (pfn << PAGE_SHIFT);
> +
> + if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
> + return 1;
> +
> + return 0;
> +}
So this only exist whenever rtas.c is compiled... but ...
> +
> /*
> * Call early during boot, before mem init or bootmem, to retrieve the RTAS
> * informations from the device-tree and allocate the RMO buffer for userland
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index c781bbc..d21dbc6 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
> hash_preload(vma->vm_mm, address, access, trap);
> #endif /* CONFIG_PPC_STD_MMU */
> }
> +
> +extern int page_is_rtas(unsigned long pfn);
> +
> +/*
> + * devmem_is_allowed(): check to see if /dev/mem access to a certain address
> + * is valid. The argument is a physical page number.
> + *
> + * Access has to be given to non-kernel-ram areas as well, these contain the
> + * PCI mmio resources as well as potential bios/acpi data regions.
> + */
> +int devmem_is_allowed(unsigned long pfn)
> +{
> + if (iomem_is_exclusive(pfn << PAGE_SHIFT))
> + return 0;
> + if (!page_is_ram(pfn))
> + return 1;
> + if (page_is_rtas(pfn))
> + return 1;
> + return 0;
> +}
This calls it unconditionally... you just broke the build of all !rtas
platforms. Additionally, putting an extern definition like that in a .c
file is gross at best....
Please instead, put in a header something like
#ifdef CONFIG_PPC_RTAS
extern int page_is_rtas(unsigned long pfn);
#else
static inline int page_is_rtas(unsigned long pfn) { }
#endif
And while at it, call it page_is_rtas_user_buf(); to make it clear what
we are talking about, ie, not RTAS core per-se but specifically the RMO
buffer.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
From: Benjamin Herrenschmidt @ 2011-12-02 1:23 UTC (permalink / raw)
To: Sukadev Bhattiprolu; +Cc: linuxppc-dev, sbest, paulus, anton
In-Reply-To: <20111202001141.GA14860@us.ibm.com>
On Thu, 2011-12-01 at 16:11 -0800, Sukadev Bhattiprolu wrote:
> >From aebed2e9fb9fba7dd0a34595780b828d7a545ba2 Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Date: Mon, 29 Aug 2011 14:12:08 -0700
> Subject: [PATCH 1/1][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc.
>
> As described in the help text in the patch, this token restricts general
> access to /dev/mem as a way of increasing security. Specifically, access
> to exclusive IOMEM and kernel RAM is denied unless CONFIG_STRICT_DEVMEM is
> set to 'n'.
>
> Implement the 'devmem_is_allowed()' interface for POWER. It will be
> called from range_is_allowed() when userpsace attempts to access /dev/mem.
>
> This patch is based on an earlier patch from Steve Best and with input from
> Paul Mackerras.
A slightly different version of that patch is already in my -next branch
since earlier this week. Please send a followup patch adding the missing
rtas bit.
Note that I've dropped the generic printk change which has nothing to do
in that patch and can/should be submitted separately if it's really
needed.
Cheers,
Ben.
> A test for this patch:
>
> # cat /proc/rtas/rmo_buffer
> 000000000f190000 10000
>
> # python -c "print 0x000000000f190000 / 0x10000"
> 3865
>
> # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865
> 1+0 records in
> 1+0 records out
> 65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s
>
> # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3864
> dd: reading `/dev/mem': Operation not permitted
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.000226844 s, 0.0 kB/s
>
> # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3866
> dd: reading `/dev/mem': Operation not permitted
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.00023542 s, 0.0 kB/s
>
> # dd if=/dev/mem of=/tmp/foo
> dd: reading `/dev/mem': Operation not permitted
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s
>
> Changelog[v2]:
> - [Anton Blanchard] Punch a hole in devmem to allow librtas/dlpar
> tools access to /dev/mem.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> arch/powerpc/Kconfig.debug | 12 ++++++++++++
> arch/powerpc/include/asm/page.h | 1 +
> arch/powerpc/kernel/rtas.c | 10 ++++++++++
> arch/powerpc/mm/mem.c | 20 ++++++++++++++++++++
> drivers/char/mem.c | 5 +++--
> 5 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index 067cb84..1cf1b00 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -298,4 +298,16 @@ config PPC_EARLY_DEBUG_CPM_ADDR
> platform probing is done, all platforms selected must
> share the same address.
>
> +config STRICT_DEVMEM
> + def_bool y
> + prompt "Filter access to /dev/mem"
> + help
> + This option restricts access to /dev/mem. If this option is
> + disabled, you allow userspace access to all memory, including
> + kernel and userspace memory. Accidental memory access is likely
> + to be disastrous.
> + Memory access is required for experts who want to debug the kernel.
> +
> + If you are unsure, say Y.
> +
> endmenu
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 2cd664e..dc2ec96 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -261,6 +261,7 @@ extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg);
> extern void copy_user_page(void *to, void *from, unsigned long vaddr,
> struct page *p);
> extern int page_is_ram(unsigned long pfn);
> +extern int devmem_is_allowed(unsigned long pfn);
>
> #ifdef CONFIG_PPC_SMLPAR
> void arch_free_page(struct page *page, int order);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index d5ca823..07cf2cf 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
> return 0;
> }
>
> +int page_is_rtas(unsigned long pfn)
> +{
> + unsigned long paddr = (pfn << PAGE_SHIFT);
> +
> + if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
> + return 1;
> +
> + return 0;
> +}
> +
> /*
> * Call early during boot, before mem init or bootmem, to retrieve the RTAS
> * informations from the device-tree and allocate the RMO buffer for userland
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index c781bbc..d21dbc6 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
> hash_preload(vma->vm_mm, address, access, trap);
> #endif /* CONFIG_PPC_STD_MMU */
> }
> +
> +extern int page_is_rtas(unsigned long pfn);
> +
> +/*
> + * devmem_is_allowed(): check to see if /dev/mem access to a certain address
> + * is valid. The argument is a physical page number.
> + *
> + * Access has to be given to non-kernel-ram areas as well, these contain the
> + * PCI mmio resources as well as potential bios/acpi data regions.
> + */
> +int devmem_is_allowed(unsigned long pfn)
> +{
> + if (iomem_is_exclusive(pfn << PAGE_SHIFT))
> + return 0;
> + if (!page_is_ram(pfn))
> + return 1;
> + if (page_is_rtas(pfn))
> + return 1;
> + return 0;
> +}
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 8fc04b4..64917e8 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -29,6 +29,7 @@
>
> #include <asm/uaccess.h>
> #include <asm/io.h>
> +#include <asm/page.h>
>
> #ifdef CONFIG_IA64
> # include <linux/efi.h>
> @@ -66,8 +67,8 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
> while (cursor < to) {
> if (!devmem_is_allowed(pfn)) {
> printk(KERN_INFO
> - "Program %s tried to access /dev/mem between %Lx->%Lx.\n",
> - current->comm, from, to);
> + "Program %s tried to access /dev/mem between %Lx->%Lx and "
> + "pfn %lu.\n", current->comm, from, to, pfn);
> return 0;
> }
> cursor += PAGE_SIZE;
^ permalink raw reply
* Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
From: Alan Cox @ 2011-12-02 0:57 UTC (permalink / raw)
To: Paul Gortmaker
Cc: gregkh, linux-kernel, linux-serial, scottwood, linuxppc-dev
In-Reply-To: <1322783258-20443-4-git-send-email-paul.gortmaker@windriver.com>
> @@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct
> uart_8250_port *up)
> spin_lock_irqsave(&up->port.lock, flags);
>
> - status = serial_inp(up, UART_LSR);
> + /* Workaround for IRQ storm errata on break with Freescale
> 16550 */
> + if (UART_BUG_FSLBK & up->port.bugs && up->lsr_last &
> UART_LSR_BI) {
> + up->lsr_last &= ~UART_LSR_BI;
> + serial_inp(up, UART_RX);
> + spin_unlock_irqrestore(&up->port.lock, flags);
> + return;
> + }
> +
> + status = up->lsr_last = serial_inp(up, UART_LSR);
We've now had a recent pile of IRQ function changes adding more quirk
bits and special casing. This doesn't scale. We either need to make
handle_port a method the specific drivers can override or you could
hide the mess in your serial_inp implementation and not touch any core
code.
I really don't mind which but I suspect dealing with it in your
serial_inp handler so that when you read UART_LSR you do the fixup
might be simplest providing you can just do that.
Sorting out a ->handle_port override is probably ultimately the right
thing to do and then we can push some of the other funnies out further.
At this point it's becoming increasingly clear that 8250 UART cloners
are both very bad at cloning the things accurately, and very busy adding
extra useful features so we need to start to treat 8250.c as a library
you can wrap.
Alan
^ permalink raw reply
* Re: [PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
From: Alan Cox @ 2011-12-02 0:51 UTC (permalink / raw)
To: Paul Gortmaker
Cc: gregkh, linux-kernel, linux-serial, scottwood, linuxppc-dev
In-Reply-To: <1322783258-20443-2-git-send-email-paul.gortmaker@windriver.com>
> Make the bugs field part of the globally visible struct
> uart_port and remove the 8250 specific one.
Except all the bits in it are 8250 specific things or names that are
meaningless in generic form - no. I also don't want to encourage flags
and bug bits. We already have too many and its making the code a mess.
So right now we want less not more.
^ permalink raw reply
* Re: [RFC][PATCH] update FSL 16550 nodes to have...
From: Paul Gortmaker @ 2011-12-02 0:41 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <1322510541-30688-1-git-send-email-galak@kernel.crashing.org>
On Mon, Nov 28, 2011 at 3:02 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> * Need to fixup the commit message
I had this written when I was thinking of re-sending the dts
with the other three, that is before I realized the dts patch
would then overwhelm the other patches completely. :)
P.
----------------
ppc: update FSL 16550 nodes to have fsl,ns16550 compat entry
There is an errata relating to break handling on some of
the Freescale 16550 implementations. By marking the nodes
with a fsl prefix, we can use that to know when to enable
the errata handling.
----------------
^ permalink raw reply
* Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
From: Kumar Gala @ 2011-12-02 0:17 UTC (permalink / raw)
To: Paul Gortmaker
Cc: gregkh, linux-kernel, linux-serial, Scott Wood, linuxppc-dev,
alan
In-Reply-To: <4ED81632.3030809@windriver.com>
On Dec 1, 2011, at 6:05 PM, Paul Gortmaker wrote:
> On 11-12-01 06:51 PM, Scott Wood wrote:
>> On 12/01/2011 05:47 PM, Paul Gortmaker wrote:
>>> diff --git a/include/linux/serial_8250.h =
b/include/linux/serial_8250.h
>>> index 8c660af..b0f4042 100644
>>> --- a/include/linux/serial_8250.h
>>> +++ b/include/linux/serial_8250.h
>>> @@ -18,6 +18,11 @@
>>> #define UART_BUG_TXEN (1 << 1) /* buggy TX IIR status =
*/
>>> #define UART_BUG_NOMSR (1 << 2) /* buggy MSR status bits =
(Au1x00) */
>>> #define UART_BUG_THRE (1 << 3) /* buggy THRE =
reassertion */
>>> +#ifdef CONFIG_PPC32
>>> +#define UART_BUG_FSLBK (1 << 4) /* buggy FSL break IRQ =
storm */
>>> +#else /* help GCC optimize away IRQ handler errata code for =
ARCH !=3D PPC32 */
>>> +#define UART_BUG_FSLBK 0
>>> +#endif
>>=20
>> I believe this bug still exists on our 64-bit chips.
>=20
> OK, I'll simply change the above to CONFIG_PPC then.
It does, the bug is in the uart IP which I don't think we ever plan on =
fixing, so 32 or 64-bit parts will have it for ever and ever ;)
- k=
^ permalink raw reply
* [PATCH][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc
From: Sukadev Bhattiprolu @ 2011-12-02 0:11 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, sbest, paulus, anton
>From aebed2e9fb9fba7dd0a34595780b828d7a545ba2 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Mon, 29 Aug 2011 14:12:08 -0700
Subject: [PATCH 1/1][v2] Enable CONFIG_STRICT_DEVMEM support for Powerpc.
As described in the help text in the patch, this token restricts general
access to /dev/mem as a way of increasing security. Specifically, access
to exclusive IOMEM and kernel RAM is denied unless CONFIG_STRICT_DEVMEM is
set to 'n'.
Implement the 'devmem_is_allowed()' interface for POWER. It will be
called from range_is_allowed() when userpsace attempts to access /dev/mem.
This patch is based on an earlier patch from Steve Best and with input from
Paul Mackerras.
A test for this patch:
# cat /proc/rtas/rmo_buffer
000000000f190000 10000
# python -c "print 0x000000000f190000 / 0x10000"
3865
# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865
1+0 records in
1+0 records out
65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s
# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3864
dd: reading `/dev/mem': Operation not permitted
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.000226844 s, 0.0 kB/s
# dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3866
dd: reading `/dev/mem': Operation not permitted
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.00023542 s, 0.0 kB/s
# dd if=/dev/mem of=/tmp/foo
dd: reading `/dev/mem': Operation not permitted
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s
Changelog[v2]:
- [Anton Blanchard] Punch a hole in devmem to allow librtas/dlpar
tools access to /dev/mem.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
arch/powerpc/Kconfig.debug | 12 ++++++++++++
arch/powerpc/include/asm/page.h | 1 +
arch/powerpc/kernel/rtas.c | 10 ++++++++++
arch/powerpc/mm/mem.c | 20 ++++++++++++++++++++
drivers/char/mem.c | 5 +++--
5 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 067cb84..1cf1b00 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -298,4 +298,16 @@ config PPC_EARLY_DEBUG_CPM_ADDR
platform probing is done, all platforms selected must
share the same address.
+config STRICT_DEVMEM
+ def_bool y
+ prompt "Filter access to /dev/mem"
+ help
+ This option restricts access to /dev/mem. If this option is
+ disabled, you allow userspace access to all memory, including
+ kernel and userspace memory. Accidental memory access is likely
+ to be disastrous.
+ Memory access is required for experts who want to debug the kernel.
+
+ If you are unsure, say Y.
+
endmenu
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 2cd664e..dc2ec96 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -261,6 +261,7 @@ extern void clear_user_page(void *page, unsigned long vaddr, struct page *pg);
extern void copy_user_page(void *to, void *from, unsigned long vaddr,
struct page *p);
extern int page_is_ram(unsigned long pfn);
+extern int devmem_is_allowed(unsigned long pfn);
#ifdef CONFIG_PPC_SMLPAR
void arch_free_page(struct page *page, int order);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index d5ca823..07cf2cf 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -937,6 +937,16 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
return 0;
}
+int page_is_rtas(unsigned long pfn)
+{
+ unsigned long paddr = (pfn << PAGE_SHIFT);
+
+ if (paddr >= rtas_rmo_buf && paddr < (rtas_rmo_buf + RTAS_RMOBUF_MAX))
+ return 1;
+
+ return 0;
+}
+
/*
* Call early during boot, before mem init or bootmem, to retrieve the RTAS
* informations from the device-tree and allocate the RMO buffer for userland
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index c781bbc..d21dbc6 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -549,3 +549,23 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address,
hash_preload(vma->vm_mm, address, access, trap);
#endif /* CONFIG_PPC_STD_MMU */
}
+
+extern int page_is_rtas(unsigned long pfn);
+
+/*
+ * devmem_is_allowed(): check to see if /dev/mem access to a certain address
+ * is valid. The argument is a physical page number.
+ *
+ * Access has to be given to non-kernel-ram areas as well, these contain the
+ * PCI mmio resources as well as potential bios/acpi data regions.
+ */
+int devmem_is_allowed(unsigned long pfn)
+{
+ if (iomem_is_exclusive(pfn << PAGE_SHIFT))
+ return 0;
+ if (!page_is_ram(pfn))
+ return 1;
+ if (page_is_rtas(pfn))
+ return 1;
+ return 0;
+}
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 8fc04b4..64917e8 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -29,6 +29,7 @@
#include <asm/uaccess.h>
#include <asm/io.h>
+#include <asm/page.h>
#ifdef CONFIG_IA64
# include <linux/efi.h>
@@ -66,8 +67,8 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
while (cursor < to) {
if (!devmem_is_allowed(pfn)) {
printk(KERN_INFO
- "Program %s tried to access /dev/mem between %Lx->%Lx.\n",
- current->comm, from, to);
+ "Program %s tried to access /dev/mem between %Lx->%Lx and "
+ "pfn %lu.\n", current->comm, from, to, pfn);
return 0;
}
cursor += PAGE_SIZE;
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
From: Paul Gortmaker @ 2011-12-02 0:05 UTC (permalink / raw)
To: Scott Wood; +Cc: gregkh, linux-kernel, linux-serial, linuxppc-dev, alan
In-Reply-To: <4ED812E4.60905@freescale.com>
On 11-12-01 06:51 PM, Scott Wood wrote:
> On 12/01/2011 05:47 PM, Paul Gortmaker wrote:
>> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
>> index 8c660af..b0f4042 100644
>> --- a/include/linux/serial_8250.h
>> +++ b/include/linux/serial_8250.h
>> @@ -18,6 +18,11 @@
>> #define UART_BUG_TXEN (1 << 1) /* buggy TX IIR status */
>> #define UART_BUG_NOMSR (1 << 2) /* buggy MSR status bits (Au1x00) */
>> #define UART_BUG_THRE (1 << 3) /* buggy THRE reassertion */
>> +#ifdef CONFIG_PPC32
>> +#define UART_BUG_FSLBK (1 << 4) /* buggy FSL break IRQ storm */
>> +#else /* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */
>> +#define UART_BUG_FSLBK 0
>> +#endif
>
> I believe this bug still exists on our 64-bit chips.
OK, I'll simply change the above to CONFIG_PPC then.
Thanks,
Paul.
>
> -Scott
>
^ permalink raw reply
* Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
From: Scott Wood @ 2011-12-01 23:51 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: gregkh, linux-kernel, linux-serial, linuxppc-dev, alan
In-Reply-To: <1322783258-20443-4-git-send-email-paul.gortmaker@windriver.com>
On 12/01/2011 05:47 PM, Paul Gortmaker wrote:
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 8c660af..b0f4042 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -18,6 +18,11 @@
> #define UART_BUG_TXEN (1 << 1) /* buggy TX IIR status */
> #define UART_BUG_NOMSR (1 << 2) /* buggy MSR status bits (Au1x00) */
> #define UART_BUG_THRE (1 << 3) /* buggy THRE reassertion */
> +#ifdef CONFIG_PPC32
> +#define UART_BUG_FSLBK (1 << 4) /* buggy FSL break IRQ storm */
> +#else /* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */
> +#define UART_BUG_FSLBK 0
> +#endif
I believe this bug still exists on our 64-bit chips.
-Scott
^ permalink raw reply
* [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
In-Reply-To: <1322783258-20443-1-git-send-email-paul.gortmaker@windriver.com>
Sending a break on the SOC UARTs found in some MPC83xx/85xx/86xx
chips seems to cause a short lived IRQ storm (/proc/interrupts
typically shows somewhere between 300 and 1500 events). Unfortunately
this renders SysRQ over the serial console completely inoperable.
The suggested workaround in the errata is to read the Rx register,
wait one character period, and then read the Rx register again.
We achieve this by tracking the old LSR value, and on the subsequent
interrupt event after a break, we don't read LSR, instead we just
read the RBR again and return immediately.
The "fsl,ns16550" is used in the compatible field of the serial
device to mark UARTs known to have this issue.
Thanks to Scott Wood for providing the errata data which led to
a much cleaner fix.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
arch/powerpc/kernel/legacy_serial.c | 11 +++++++++++
drivers/tty/serial/8250.c | 11 ++++++++++-
include/linux/serial_8250.h | 5 +++++
3 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c
index c7b5afe..dd232ca 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -476,6 +476,15 @@ static void __init fixup_port_mmio(int index,
port->membase = ioremap(port->mapbase, 0x100);
}
+static void __init fixup_port_bugs(int index,
+ struct device_node *np,
+ struct plat_serial8250_port *port)
+{
+ DBG("fixup_port_bugs(%d)\n", index);
+
+ if (of_device_is_compatible(np, "fsl,ns16550"))
+ port->bugs = UART_BUG_FSLBK;
+}
/*
* This is called as an arch initcall, hopefully before the PCI bus is
* probed and/or the 8250 driver loaded since we need to register our
@@ -512,6 +521,8 @@ static int __init serial_dev_init(void)
fixup_port_pio(i, np, port);
if ((port->iotype == UPIO_MEM) || (port->iotype == UPIO_TSI))
fixup_port_mmio(i, np, port);
+
+ fixup_port_bugs(i, np, port);
}
DBG("Registering platform serial ports\n");
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index f99f27c..32e9821 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -142,6 +142,7 @@ struct uart_8250_port {
unsigned char mcr_mask; /* mask of user bits */
unsigned char mcr_force; /* mask of forced bits */
unsigned char cur_iotype; /* Running I/O type */
+ unsigned char lsr_last; /* LSR of last IRQ event */
/*
* Some bits in registers are cleared on a read, so they must
@@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct uart_8250_port *up)
spin_lock_irqsave(&up->port.lock, flags);
- status = serial_inp(up, UART_LSR);
+ /* Workaround for IRQ storm errata on break with Freescale 16550 */
+ if (UART_BUG_FSLBK & up->port.bugs && up->lsr_last & UART_LSR_BI) {
+ up->lsr_last &= ~UART_LSR_BI;
+ serial_inp(up, UART_RX);
+ spin_unlock_irqrestore(&up->port.lock, flags);
+ return;
+ }
+
+ status = up->lsr_last = serial_inp(up, UART_LSR);
DEBUG_INTR("status = %x...", status);
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 8c660af..b0f4042 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -18,6 +18,11 @@
#define UART_BUG_TXEN (1 << 1) /* buggy TX IIR status */
#define UART_BUG_NOMSR (1 << 2) /* buggy MSR status bits (Au1x00) */
#define UART_BUG_THRE (1 << 3) /* buggy THRE reassertion */
+#ifdef CONFIG_PPC32
+#define UART_BUG_FSLBK (1 << 4) /* buggy FSL break IRQ storm */
+#else /* help GCC optimize away IRQ handler errata code for ARCH != PPC32 */
+#define UART_BUG_FSLBK 0
+#endif
/*
* This is the platform device platform_data structure
--
1.7.7
^ permalink raw reply related
* [PATCH 1/3] serial: make bugs field not specific to 8250 type uarts.
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
In-Reply-To: <1322783258-20443-1-git-send-email-paul.gortmaker@windriver.com>
There is a struct uart_8250_port that is private to 8250.c
itself, and it had a bugs field. But there is no reason to
assume that hardware bugs are unique to 8250 type UARTS.
Make the bugs field part of the globally visible struct
uart_port and remove the 8250 specific one.
The value in doing so is that it helps pave the way for
allowing arch or platform specific code to pass in information
to the specific uart drivers about uart bugs known to impact
certain platforms that would otherwise be hard to detect from
within the context of the driver itself.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/tty/serial/8250.c | 25 ++++++++++++-------------
include/linux/serial_core.h | 1 +
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index eeadf1b..7c94dbc 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -134,7 +134,6 @@ struct uart_8250_port {
struct timer_list timer; /* "no irq" timer */
struct list_head list; /* ports on this IRQ */
unsigned short capabilities; /* port capabilities */
- unsigned short bugs; /* port bugs */
unsigned int tx_loadsz; /* transmit fifo load size */
unsigned char acr;
unsigned char ier;
@@ -828,7 +827,7 @@ static void autoconfig_has_efr(struct uart_8250_port *up)
* when DLL is 0.
*/
if (id3 == 0x52 && rev == 0x01)
- up->bugs |= UART_BUG_QUOT;
+ up->port.bugs |= UART_BUG_QUOT;
return;
}
@@ -1102,7 +1101,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
spin_lock_irqsave(&up->port.lock, flags);
up->capabilities = 0;
- up->bugs = 0;
+ up->port.bugs = 0;
if (!(up->port.flags & UPF_BUGGY_UART)) {
/*
@@ -1337,7 +1336,7 @@ static void serial8250_start_tx(struct uart_port *port)
up->ier |= UART_IER_THRI;
serial_out(up, UART_IER, up->ier);
- if (up->bugs & UART_BUG_TXEN) {
+ if (port->bugs & UART_BUG_TXEN) {
unsigned char lsr;
lsr = serial_in(up, UART_LSR);
up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
@@ -1373,7 +1372,7 @@ static void serial8250_enable_ms(struct uart_port *port)
container_of(port, struct uart_8250_port, port);
/* no MSR capabilities */
- if (up->bugs & UART_BUG_NOMSR)
+ if (port->bugs & UART_BUG_NOMSR)
return;
up->ier |= UART_IER_MSI;
@@ -2089,7 +2088,7 @@ static int serial8250_startup(struct uart_port *port)
* kick the UART on a regular basis.
*/
if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
- up->bugs |= UART_BUG_THRE;
+ port->bugs |= UART_BUG_THRE;
pr_debug("ttyS%d - using backup timer\n",
serial_index(port));
}
@@ -2099,7 +2098,7 @@ static int serial8250_startup(struct uart_port *port)
* The above check will only give an accurate result the first time
* the port is opened so this value needs to be preserved.
*/
- if (up->bugs & UART_BUG_THRE) {
+ if (port->bugs & UART_BUG_THRE) {
up->timer.function = serial8250_backup_timeout;
up->timer.data = (unsigned long)up;
mod_timer(&up->timer, jiffies +
@@ -2162,13 +2161,13 @@ static int serial8250_startup(struct uart_port *port)
serial_outp(up, UART_IER, 0);
if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) {
- if (!(up->bugs & UART_BUG_TXEN)) {
- up->bugs |= UART_BUG_TXEN;
+ if (!(port->bugs & UART_BUG_TXEN)) {
+ port->bugs |= UART_BUG_TXEN;
pr_debug("ttyS%d - enabling bad tx status workarounds\n",
serial_index(port));
}
} else {
- up->bugs &= ~UART_BUG_TXEN;
+ port->bugs &= ~UART_BUG_TXEN;
}
dont_test_tx_en:
@@ -2323,7 +2322,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
/*
* Oxford Semi 952 rev B workaround
*/
- if (up->bugs & UART_BUG_QUOT && (quot & 0xff) == 0)
+ if (port->bugs & UART_BUG_QUOT && (quot & 0xff) == 0)
quot++;
if (up->capabilities & UART_CAP_FIFO && up->port.fifosize > 1) {
@@ -2390,7 +2389,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
* CTS flow control flag and modem status interrupts
*/
up->ier &= ~UART_IER_MSI;
- if (!(up->bugs & UART_BUG_NOMSR) &&
+ if (!(port->bugs & UART_BUG_NOMSR) &&
UART_ENABLE_MS(&up->port, termios->c_cflag))
up->ier |= UART_IER_MSI;
if (up->capabilities & UART_CAP_UUE)
@@ -2666,7 +2665,7 @@ static void serial8250_config_port(struct uart_port *port, int flags)
/* if access method is AU, it is a 16550 with a quirk */
if (up->port.type == PORT_16550A && up->port.iotype == UPIO_AU)
- up->bugs |= UART_BUG_NOMSR;
+ port->bugs |= UART_BUG_NOMSR;
if (up->port.type != PORT_UNKNOWN && flags & UART_CONFIG_IRQ)
autoconfig_irq(up);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index eadf33d..791536c 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -323,6 +323,7 @@ struct uart_port {
unsigned int read_status_mask; /* driver specific */
unsigned int ignore_status_mask; /* driver specific */
+ unsigned short bugs; /* driver specific */
struct uart_state *state; /* pointer to parent state */
struct uart_icount icount; /* statistics */
--
1.7.7
^ permalink raw reply related
* [PATCH 0/3] RFC Fix Fsl 8250 BRK bug via letting plat code set bugs
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
This is a respin of an earlier patch[1] that enabled a workaround
via Kconfig for an errata issue when using BRK on FSL 16550 UARTs.
In this version, the 8250 specific bug field is moved to the generic
uart_port struct, since hardware bugs aren't the unique domain of
just the 8250 class of uarts.
Then the ability to set a known bugs value via the platform_device entry
point is added, so that it can be set externally vs. requiring some sort
of autodetection from the actual driver(s).
Then, the FSL errata fix is added to 8250.c by using the above support.
An entry in a DTS file is used to flag boards/platforms with the issue.
It is also added in a way that is optimized away for other architectures.
Kumar has a patch pending[2] that updates all the dts files. I've not
included it here, so that review focus can be on the implementation.
I've re-tested it on an sbc8641D, where sysrq was useless before; with
this fix, it works as expected.
Thanks,
Paul.
[1] http://patchwork.ozlabs.org/patch/46609/
[2] http://patchwork.ozlabs.org/patch/128070/
--------
Paul Gortmaker (3):
serial: make bugs field not specific to 8250 type uarts.
serial: allow passing in hardware bug info via platform device
8250: add workaround for MPC8[356]xx UART break IRQ storm
arch/powerpc/kernel/legacy_serial.c | 11 ++++++++++
drivers/tty/serial/8250.c | 37 +++++++++++++++++++++-------------
drivers/tty/serial/8250.h | 5 ----
include/linux/serial_8250.h | 11 ++++++++++
include/linux/serial_core.h | 1 +
5 files changed, 46 insertions(+), 19 deletions(-)
--
1.7.7
^ permalink raw reply
* [PATCH 2/3] serial: allow passing in hardware bug info via platform device
From: Paul Gortmaker @ 2011-12-01 23:47 UTC (permalink / raw)
To: gregkh, alan, galak, scottwood; +Cc: linuxppc-dev, linux-kernel, linux-serial
In-Reply-To: <1322783258-20443-1-git-send-email-paul.gortmaker@windriver.com>
The normal arch hook into the 8250 serial world is via passing
in a plat_serial8250_port struct. However, this struct does
not have a bugs field, so there is no way to have the arch
code pass in info about known uart issues.
Add a bug field to the plat_serial8250_port struct, so that the
arch can pass in this information. Also don't do a blanket
overwrite of the bugs setting in the 8250.c driver. Finally,
relocate the known bug #define list to a globally visible header
so that the arch can assign any appropriate values from the list.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/tty/serial/8250.c | 3 ++-
drivers/tty/serial/8250.h | 5 -----
include/linux/serial_8250.h | 6 ++++++
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 7c94dbc..f99f27c 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -1101,7 +1101,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
spin_lock_irqsave(&up->port.lock, flags);
up->capabilities = 0;
- up->port.bugs = 0;
if (!(up->port.flags & UPF_BUGGY_UART)) {
/*
@@ -3075,6 +3074,7 @@ static int __devinit serial8250_probe(struct platform_device *dev)
port.serial_out = p->serial_out;
port.handle_irq = p->handle_irq;
port.set_termios = p->set_termios;
+ port.bugs = p->bugs;
port.pm = p->pm;
port.dev = &dev->dev;
port.irqflags |= irqflag;
@@ -3225,6 +3225,7 @@ int serial8250_register_port(struct uart_port *port)
uart->port.regshift = port->regshift;
uart->port.iotype = port->iotype;
uart->port.flags = port->flags | UPF_BOOT_AUTOCONF;
+ uart->port.bugs = port->bugs;
uart->port.mapbase = port->mapbase;
uart->port.private_data = port->private_data;
if (port->dev)
diff --git a/drivers/tty/serial/8250.h b/drivers/tty/serial/8250.h
index 6edf4a6..caefe00 100644
--- a/drivers/tty/serial/8250.h
+++ b/drivers/tty/serial/8250.h
@@ -44,11 +44,6 @@ struct serial8250_config {
#define UART_CAP_UUE (1 << 12) /* UART needs IER bit 6 set (Xscale) */
#define UART_CAP_RTOIE (1 << 13) /* UART needs IER bit 4 set (Xscale, Tegra) */
-#define UART_BUG_QUOT (1 << 0) /* UART has buggy quot LSB */
-#define UART_BUG_TXEN (1 << 1) /* UART has buggy TX IIR status */
-#define UART_BUG_NOMSR (1 << 2) /* UART has buggy MSR status bits (Au1x00) */
-#define UART_BUG_THRE (1 << 3) /* UART has buggy THRE reassertion */
-
#define PROBE_RSA (1 << 0)
#define PROBE_ANY (~0)
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 1f05bbe..8c660af 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -14,6 +14,11 @@
#include <linux/serial_core.h>
#include <linux/platform_device.h>
+#define UART_BUG_QUOT (1 << 0) /* buggy quot LSB */
+#define UART_BUG_TXEN (1 << 1) /* buggy TX IIR status */
+#define UART_BUG_NOMSR (1 << 2) /* buggy MSR status bits (Au1x00) */
+#define UART_BUG_THRE (1 << 3) /* buggy THRE reassertion */
+
/*
* This is the platform device platform_data structure
*/
@@ -30,6 +35,7 @@ struct plat_serial8250_port {
unsigned char hub6;
upf_t flags; /* UPF_* flags */
unsigned int type; /* If UPF_FIXED_TYPE */
+ unsigned short bugs; /* hardware specific bugs */
unsigned int (*serial_in)(struct uart_port *, int);
void (*serial_out)(struct uart_port *, int, int);
void (*set_termios)(struct uart_port *,
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 5/6] powerpc/boot: Add mfdcrx
From: Segher Boessenkool @ 2011-12-01 22:55 UTC (permalink / raw)
To: David Laight; +Cc: LinuxPPC-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AEF3@saturn3.aculab.com>
>>> +#define mfdcrx(rn) \
>>> + ({ \
>>> + unsigned long rval; \
>>> + asm volatile("mfdcrx %0,%1" : "=r"(rval) : "g"(rn)); \
>>> + rval; \
>>> + })
>>
>> "g" is never correct on PowerPC, you want "r" here. You can write
>> this as a static inline btw, you only need the #define stuff when
>> there is an "i" constraint involved.
>
> I think you can still use static inlines even when a
> constraint is one that requires a compile time constant.
Marking a function inline does not guarantee the compiler will
perform inline substitution on it, which you need here. I don't
think it's a problem with recent GCC in the context of the Linux
kernel though, if you use __always_inline that is.
Segher
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 22:10 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7FA38.6030606@freescale.com>
Scott Wood wrote:
>
> Does of_find_i2c_device_by_node() do what you want?
Yeah, that'll work. I wasn't thinking about looking for an i2c device. Thanks.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Scott Wood @ 2011-12-01 22:05 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7F8CE.10801@freescale.com>
On 12/01/2011 03:59 PM, Timur Tabi wrote:
> Scott Wood wrote:
>> Ideally we would have a field in struct device_node that points to
>> struct device.
>
> I don't think there's a single place in the code where the connection
> between the device and device_node is made. In fact, I think
> dev->of_node needs to be manually initialized by the driver during
> the OF probe.
The i2c core does this.
Does of_find_i2c_device_by_node() do what you want?
-Scott
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 21:59 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, kumar.gala
In-Reply-To: <4ED7F72F.7050405@freescale.com>
Scott Wood wrote:
> Ideally we would have a field in struct device_node that points to
> struct device.
I don't think there's a single place in the code where the connection between the device and device_node is made. In fact, I think dev->of_node needs to be manually initialized by the driver during the OF probe.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] i2c-mpc: use the cell-index property to enumerate the I2C adapters
From: Timur Tabi @ 2011-12-01 21:59 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, linuxppc-dev, kumar.gala
In-Reply-To: <CACxGe6sZgG5q6NKgsGaWLyTzWHXNGqHke9nQMyqzw0audS3=tw@mail.gmail.com>
Grant Likely wrote:
> It is better to walk the list of i2c adapters and look for one that
> has the matching node pointer. It really isn't an expensive operation
> to do it that way.
That's what I was thinking. I can't figure out how to walk the list, though. i2c_get_adapter() takes an adapter number, but I don't think this is going to work:
unsigned int i = 0;
struct i2c_adapter adap = i2c_get_adapter(0);
while (adap) {
if (adap->nr == nr)
break;
adap = i2c_get_adapter(++i);
}
--
Timur Tabi
Linux kernel developer at Freescale
^ 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