* [PATCH] [v2] powerpc/fsl: add MSI support for the Freescale hypervisor
From: Timur Tabi @ 2011-12-13 18:52 UTC (permalink / raw)
To: kumar.gala, scottwood, linuxppc-dev
Add support for vmpic-msi nodes to the fsl_msi driver. The MSI is
virtualized by the hypervisor, so the vmpic-msi does not contain a 'reg'
property. Instead, the driver uses hcalls.
Add support for the "msi-address-64" property to the fsl_pci driver.
The Freescale hypervisor typically puts the virtualized MSIIR register
in the page after the end of DDR, so we extend the DDR ATMU to cover it.
Any other location for MSIIR is not supported, for now.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
arch/powerpc/sysdev/fsl_msi.c | 68 +++++++++++++++++++++++++++++------------
arch/powerpc/sysdev/fsl_msi.h | 7 ++--
arch/powerpc/sysdev/fsl_pci.c | 30 ++++++++++++++++++
3 files changed, 82 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 89548e0..7dc473f 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -23,6 +23,8 @@
#include <asm/hw_irq.h>
#include <asm/ppc-pci.h>
#include <asm/mpic.h>
+#include <asm/fsl_hcalls.h>
+
#include "fsl_msi.h"
#include "fsl_pci.h"
@@ -163,11 +165,13 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
*/
np = of_parse_phandle(hose->dn, "fsl,msi", 0);
if (np) {
- if (of_device_is_compatible(np, "fsl,mpic-msi"))
+ if (of_device_is_compatible(np, "fsl,mpic-msi") ||
+ of_device_is_compatible(np, "fsl,vmpic-msi"))
phandle = np->phandle;
else {
- dev_err(&pdev->dev, "node %s has an invalid fsl,msi"
- " phandle\n", hose->dn->full_name);
+ dev_err(&pdev->dev,
+ "node %s has an invalid fsl,msi phandle %u\n",
+ hose->dn->full_name, np->phandle);
return -EINVAL;
}
}
@@ -196,16 +200,14 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
if (hwirq < 0) {
rc = hwirq;
- pr_debug("%s: fail allocating msi interrupt\n",
- __func__);
+ dev_err(&pdev->dev, "could not allocate MSI interrupt\n");
goto out_free;
}
virq = irq_create_mapping(msi_data->irqhost, hwirq);
if (virq == NO_IRQ) {
- pr_debug("%s: fail mapping hwirq 0x%x\n",
- __func__, hwirq);
+ dev_err(&pdev->dev, "fail mapping hwirq %i\n", hwirq);
msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
rc = -ENOSPC;
goto out_free;
@@ -234,6 +236,7 @@ static void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
u32 intr_index;
u32 have_shift = 0;
struct fsl_msi_cascade_data *cascade_data;
+ unsigned int ret;
cascade_data = irq_get_handler_data(irq);
msi_data = cascade_data->msi_data;
@@ -265,6 +268,14 @@ static void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
case FSL_PIC_IP_IPIC:
msir_value = fsl_msi_read(msi_data->msi_regs, msir_index * 0x4);
break;
+ case FSL_PIC_IP_VMPIC:
+ ret = fh_vmpic_get_msir(virq_to_hw(irq), &msir_value);
+ if (ret) {
+ pr_err("fsl-msi: fh_vmpic_get_msir() failed for "
+ "irq %u (ret=%u)\n", irq, ret);
+ msir_value = 0;
+ }
+ break;
}
while (msir_value) {
@@ -282,6 +293,7 @@ static void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
switch (msi_data->feature & FSL_PIC_IP_MASK) {
case FSL_PIC_IP_MPIC:
+ case FSL_PIC_IP_VMPIC:
chip->irq_eoi(idata);
break;
case FSL_PIC_IP_IPIC:
@@ -311,7 +323,8 @@ static int fsl_of_msi_remove(struct platform_device *ofdev)
}
if (msi->bitmap.bitmap)
msi_bitmap_free(&msi->bitmap);
- iounmap(msi->msi_regs);
+ if ((msi->feature & FSL_PIC_IP_MASK) != FSL_PIC_IP_VMPIC)
+ iounmap(msi->msi_regs);
kfree(msi);
return 0;
@@ -383,26 +396,32 @@ static int __devinit fsl_of_msi_probe(struct platform_device *dev)
goto error_out;
}
- /* Get the MSI reg base */
- err = of_address_to_resource(dev->dev.of_node, 0, &res);
- if (err) {
- dev_err(&dev->dev, "%s resource error!\n",
+ /*
+ * Under the Freescale hypervisor, the msi nodes don't have a 'reg'
+ * property. Instead, we use hypercalls to access the MSI.
+ */
+ if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) != FSL_PIC_IP_VMPIC) {
+ err = of_address_to_resource(dev->dev.of_node, 0, &res);
+ if (err) {
+ dev_err(&dev->dev, "invalid resource for node %s\n",
dev->dev.of_node->full_name);
- goto error_out;
- }
+ goto error_out;
+ }
- msi->msi_regs = ioremap(res.start, resource_size(&res));
- if (!msi->msi_regs) {
- dev_err(&dev->dev, "ioremap problem failed\n");
- goto error_out;
+ msi->msi_regs = ioremap(res.start, resource_size(&res));
+ if (!msi->msi_regs) {
+ dev_err(&dev->dev, "could not map node %s\n",
+ dev->dev.of_node->full_name);
+ goto error_out;
+ }
+ msi->msiir_offset =
+ features->msiir_offset + (res.start & 0xfffff);
}
msi->feature = features->fsl_pic_ip;
msi->irqhost->host_data = msi;
- msi->msiir_offset = features->msiir_offset + (res.start & 0xfffff);
-
/*
* Remember the phandle, so that we can match with any PCI nodes
* that have an "fsl,msi" property.
@@ -476,6 +495,11 @@ static const struct fsl_msi_feature ipic_msi_feature = {
.msiir_offset = 0x38,
};
+static const struct fsl_msi_feature vmpic_msi_feature = {
+ .fsl_pic_ip = FSL_PIC_IP_VMPIC,
+ .msiir_offset = 0,
+};
+
static const struct of_device_id fsl_of_msi_ids[] = {
{
.compatible = "fsl,mpic-msi",
@@ -485,6 +509,10 @@ static const struct of_device_id fsl_of_msi_ids[] = {
.compatible = "fsl,ipic-msi",
.data = (void *)&ipic_msi_feature,
},
+ {
+ .compatible = "fsl,vmpic-msi",
+ .data = (void *)&vmpic_msi_feature,
+ },
{}
};
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
index b5d25ba..f6c646a 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -20,9 +20,10 @@
#define IRQS_PER_MSI_REG 32
#define NR_MSI_IRQS (NR_MSI_REG * IRQS_PER_MSI_REG)
-#define FSL_PIC_IP_MASK 0x0000000F
-#define FSL_PIC_IP_MPIC 0x00000001
-#define FSL_PIC_IP_IPIC 0x00000002
+#define FSL_PIC_IP_MASK 0x0000000F
+#define FSL_PIC_IP_MPIC 0x00000001
+#define FSL_PIC_IP_IPIC 0x00000002
+#define FSL_PIC_IP_VMPIC 0x00000003
struct fsl_msi {
struct irq_host *irqhost;
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 4ce547e..cd82613 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -113,6 +113,8 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
u32 piwar = PIWAR_EN | PIWAR_PF | PIWAR_TGI_LOCAL |
PIWAR_READ_SNOOP | PIWAR_WRITE_SNOOP;
char *name = hose->dn->full_name;
+ const u64 *reg;
+ int len;
pr_debug("PCI memory map start 0x%016llx, size 0x%016llx\n",
(u64)rsrc->start, (u64)resource_size(rsrc));
@@ -205,6 +207,34 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
/* Setup inbound mem window */
mem = memblock_end_of_DRAM();
+
+ /*
+ * The msi-address-64 property, if it exists, indicates the physical
+ * address of the MSIIR register. Normally, this register is located
+ * inside CCSR, so the ATMU that covers all of CCSR is used. But if
+ * this property exists, then we normally need to create a new ATMU
+ * for it. For now, however, we cheat. The only entity that creates
+ * this property is the Freescale hypervisor, and the address is
+ * specified in the partition configuration. Typically, the address
+ * is located in the page immediately after the end of DDR. If so, we
+ * can avoid allocating a new ATMU by extending the DDR ATMU by one
+ * page.
+ */
+ reg = of_get_property(hose->dn, "msi-address-64", &len);
+ if (reg && (len == sizeof(u64))) {
+ u64 address = be64_to_cpup(reg);
+
+ if ((address >= mem) && (address < (mem + PAGE_SIZE))) {
+ pr_info("%s: extending DDR ATMU to cover MSIIR", name);
+ mem += PAGE_SIZE;
+ } else {
+ /* TODO: Create a new ATMU for MSIIR */
+ pr_warn("%s: msi-address-64 address of %llx in node %s"
+ " is unsupported\n", name, address,
+ hose->dn->full_name);
+ }
+ }
+
sz = min(mem, paddr_lo);
mem_log = __ilog2_u64(sz);
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Wolfram Sang @ 2011-12-13 18:23 UTC (permalink / raw)
To: Grant Likely
Cc: linuxppc-dev, Anatolij Gustschin, linux-kernel, Linus Walleij
In-Reply-To: <20111213181659.GA29243@ponder.secretlab.ca>
[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]
On Tue, Dec 13, 2011 at 11:16:59AM -0700, Grant Likely wrote:
> On Tue, Dec 13, 2011 at 10:12:48AM +0100, Wolfram Sang wrote:
> > Add a 5121-custom reject if an input-only pin is requested to be output
> > (see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
> > consume less lines which scales better.
> >
> > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> > ---
> > drivers/gpio/gpio-mpc8xxx.c | 17 ++++++++++++-----
> > 1 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> > index ec3fcf0..25dc736 100644
> > --- a/drivers/gpio/gpio-mpc8xxx.c
> > +++ b/drivers/gpio/gpio-mpc8xxx.c
> > @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> > return 0;
> > }
> >
> > +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> > +{
> > + /* GPIO 28..31 are input only on MPC5121 */
> > + if (gpio >= 28)
> > + return -EINVAL;
> > +
> > + return mpc8xxx_gpio_dir_out(gc, gpio, val);
> > +}
> > static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>
> This actually caused a build failure. mpc8xxx_gpio_dir_out() was used before
> it was declared. I moved the symbol to immediately below and applied anyway,
> but how did it compile for you? Should I drop this patch until you retest?
Huh, I am surprised as well. Will investigate tomorrow. Sorry for the
inconvenience.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Grant Likely @ 2011-12-13 18:16 UTC (permalink / raw)
To: Wolfram Sang
Cc: linuxppc-dev, Anatolij Gustschin, linux-kernel, Linus Walleij
In-Reply-To: <1323767568-9565-1-git-send-email-w.sang@pengutronix.de>
On Tue, Dec 13, 2011 at 10:12:48AM +0100, Wolfram Sang wrote:
> Add a 5121-custom reject if an input-only pin is requested to be output
> (see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
> consume less lines which scales better.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
> drivers/gpio/gpio-mpc8xxx.c | 17 ++++++++++++-----
> 1 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index ec3fcf0..25dc736 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> return 0;
> }
>
> +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + /* GPIO 28..31 are input only on MPC5121 */
> + if (gpio >= 28)
> + return -EINVAL;
> +
> + return mpc8xxx_gpio_dir_out(gc, gpio, val);
> +}
> static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
This actually caused a build failure. mpc8xxx_gpio_dir_out() was used before
it was declared. I moved the symbol to immediately below and applied anyway,
but how did it compile for you? Should I drop this patch until you retest?
g.
^ permalink raw reply
* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Grant Likely @ 2011-12-13 18:07 UTC (permalink / raw)
To: Wolfram Sang
Cc: linuxppc-dev, Anatolij Gustschin, linux-kernel, Linus Walleij
In-Reply-To: <1323767568-9565-1-git-send-email-w.sang@pengutronix.de>
On Tue, Dec 13, 2011 at 10:12:48AM +0100, Wolfram Sang wrote:
> Add a 5121-custom reject if an input-only pin is requested to be output
> (see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
> consume less lines which scales better.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Applied, thanks.
g.
> ---
> drivers/gpio/gpio-mpc8xxx.c | 17 ++++++++++++-----
> 1 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index ec3fcf0..25dc736 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> return 0;
> }
>
> +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + /* GPIO 28..31 are input only on MPC5121 */
> + if (gpio >= 28)
> + return -EINVAL;
> +
> + return mpc8xxx_gpio_dir_out(gc, gpio, val);
> +}
> static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
> {
> struct of_mm_gpio_chip *mm = to_of_mm_gpio_chip(gc);
> @@ -340,11 +348,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
> mm_gc->save_regs = mpc8xxx_gpio_save_regs;
> gc->ngpio = MPC8XXX_GPIO_PINS;
> gc->direction_input = mpc8xxx_gpio_dir_in;
> - gc->direction_output = mpc8xxx_gpio_dir_out;
> - if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
> - gc->get = mpc8572_gpio_get;
> - else
> - gc->get = mpc8xxx_gpio_get;
> + gc->direction_output = of_device_is_compatible(np, "fsl,mpc5121-gpio") ?
> + mpc5121_gpio_dir_out : mpc8xxx_gpio_dir_out;
> + gc->get = of_device_is_compatible(np, "fsl,mpc8572-gpio") ?
> + mpc8572_gpio_get : mpc8xxx_gpio_get;
> gc->set = mpc8xxx_gpio_set;
> gc->to_irq = mpc8xxx_gpio_to_irq;
>
> --
> 1.7.7.1
>
^ permalink raw reply
* [PATCH V2] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Wolfram Sang @ 2011-12-13 17:46 UTC (permalink / raw)
To: linux-kernel; +Cc: Anatolij Gustschin, linuxppc-dev, Linus Walleij
Add a 5121-custom reject if an input-only pin is requested to be output
(see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
consume less lines which scales better.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
Since V1, added an empty line after the new function.
drivers/gpio/gpio-mpc8xxx.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index ec3fcf0..bb4975f 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -115,6 +115,15 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
return 0;
}
+static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ /* GPIO 28..31 are input only on MPC5121 */
+ if (gpio >= 28)
+ return -EINVAL;
+
+ return mpc8xxx_gpio_dir_out(gc, gpio, val);
+}
+
static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
{
struct of_mm_gpio_chip *mm = to_of_mm_gpio_chip(gc);
@@ -340,11 +349,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
mm_gc->save_regs = mpc8xxx_gpio_save_regs;
gc->ngpio = MPC8XXX_GPIO_PINS;
gc->direction_input = mpc8xxx_gpio_dir_in;
- gc->direction_output = mpc8xxx_gpio_dir_out;
- if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
- gc->get = mpc8572_gpio_get;
- else
- gc->get = mpc8xxx_gpio_get;
+ gc->direction_output = of_device_is_compatible(np, "fsl,mpc5121-gpio") ?
+ mpc5121_gpio_dir_out : mpc8xxx_gpio_dir_out;
+ gc->get = of_device_is_compatible(np, "fsl,mpc8572-gpio") ?
+ mpc8572_gpio_get : mpc8xxx_gpio_get;
gc->set = mpc8xxx_gpio_set;
gc->to_irq = mpc8xxx_gpio_to_irq;
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v2 01/19] mxc_udc: add workaround for ENGcm09152 for i.MX25
From: Felipe Balbi @ 2011-12-13 14:18 UTC (permalink / raw)
To: Eric Bénard
Cc: Greg Kroah-Hartman, open list:FREESCALE USB PER..., open list,
Felipe Balbi, Sascha Hauer, open list:FREESCALE USB PER...,
linux-arm-kernel
In-Reply-To: <1323785377-22463-1-git-send-email-eric@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 2468 bytes --]
Hi,
On Tue, Dec 13, 2011 at 03:09:37PM +0100, Eric Bénard wrote:
> this patch gives the possibility to workaround bug ENGcm09152
> on i.MX25 when the hardware workaround is also implemented on
> the board.
> It covers the workaround described on page 42 of the following Errata,
> titled "USB: UTMI_USBPHY VBUS input impedance implementation error" :
> http://cache.freescale.com/files/dsp/doc/errata/IMX25CE.pdf
>
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Li Yang <leoli@freescale.com>
Sascha, are you taking this one or should I take it ? One comment below
though.
> ---
> drivers/usb/gadget/fsl_mxc_udc.c | 22 +++++++++++++---------
> 1 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
> index dcbc0a2..4aff05d 100644
> --- a/drivers/usb/gadget/fsl_mxc_udc.c
> +++ b/drivers/usb/gadget/fsl_mxc_udc.c
> @@ -23,7 +23,7 @@
> static struct clk *mxc_ahb_clk;
> static struct clk *mxc_usb_clk;
>
> -/* workaround ENGcm09152 for i.MX35 */
> +/* workaround ENGcm09152 for i.MX25/35 */
> #define USBPHYCTRL_OTGBASE_OFFSET 0x608
> #define USBPHYCTRL_EVDO (1 << 23)
>
> @@ -89,16 +89,20 @@ eenahb:
> void fsl_udc_clk_finalize(struct platform_device *pdev)
> {
> struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> - if (cpu_is_mx35()) {
> + if (cpu_is_mx25() || cpu_is_mx35()) {
> unsigned int v;
> -
> - /* workaround ENGcm09152 for i.MX35 */
> + void __iomem *otgbase;
> + if (cpu_is_mx25())
> + otgbase = MX25_IO_ADDRESS(MX25_USB_BASE_ADDR +
> + USBPHYCTRL_OTGBASE_OFFSET);
> + else if (cpu_is_mx35())
> + otgbase = MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
> + USBPHYCTRL_OTGBASE_OFFSET);
I don't want to see cpu_is_* or machine_is_* or <plat/*> or <mach/*>
anywhere on drivers, please make sure to remove them for 3.4 merge
window the latest. You should also make sure that this driver has no
static globals. IOW, I would like to be able to have multiple instances
of this driver working fine.
Granted, you have no boards with such setup, but we made that mistake on
musb driver and now we have a bunch of other stuff to rework in order to
get our dual-MUSB board to work ;-)
I also see that fsl_udc_core.c is still not using dev_pm_ops, please fix
that too.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v2 01/19] mxc_udc: add workaround for ENGcm09152 for i.MX25
From: Eric Bénard @ 2011-12-13 14:09 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Greg Kroah-Hartman, open list:FREESCALE USB PER..., open list,
Felipe Balbi, Sascha Hauer, open list:FREESCALE USB PER...
In-Reply-To: <20111213101846.GC2619@pengutronix.de>
this patch gives the possibility to workaround bug ENGcm09152
on i.MX25 when the hardware workaround is also implemented on
the board.
It covers the workaround described on page 42 of the following Errata,
titled "USB: UTMI_USBPHY VBUS input impedance implementation error" :
http://cache.freescale.com/files/dsp/doc/errata/IMX25CE.pdf
Signed-off-by: Eric Bénard <eric@eukrea.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Li Yang <leoli@freescale.com>
---
drivers/usb/gadget/fsl_mxc_udc.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index dcbc0a2..4aff05d 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,7 @@
static struct clk *mxc_ahb_clk;
static struct clk *mxc_usb_clk;
-/* workaround ENGcm09152 for i.MX35 */
+/* workaround ENGcm09152 for i.MX25/35 */
#define USBPHYCTRL_OTGBASE_OFFSET 0x608
#define USBPHYCTRL_EVDO (1 << 23)
@@ -89,16 +89,20 @@ eenahb:
void fsl_udc_clk_finalize(struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
- if (cpu_is_mx35()) {
+ if (cpu_is_mx25() || cpu_is_mx35()) {
unsigned int v;
-
- /* workaround ENGcm09152 for i.MX35 */
+ void __iomem *otgbase;
+ if (cpu_is_mx25())
+ otgbase = MX25_IO_ADDRESS(MX25_USB_BASE_ADDR +
+ USBPHYCTRL_OTGBASE_OFFSET);
+ else if (cpu_is_mx35())
+ otgbase = MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+ USBPHYCTRL_OTGBASE_OFFSET);
+
+ /* workaround ENGcm09152 for i.MX25/35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
- writel(v | USBPHYCTRL_EVDO,
- MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ v = readl(otgbase);
+ writel(v | USBPHYCTRL_EVDO, otgbase);
}
}
--
1.7.6.4
^ permalink raw reply related
* Re: [PATCH 3/4] ppc32/kprobe: complete kprobe and migrate exception frame
From: tiejun.chen @ 2011-12-13 10:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <4EE6DA8C.8090107@windriver.com>
>> You need to hook into "resume_kernel" instead.
>
I regenerate this with hooking into resume_kernel in below.
> Maybe I'm misunderstanding what you mean since as I recall you suggestion we
> should do this at the end of do_work.
>
>> Also, we may want to simplify the whole thing, instead of checking user
>> vs. kernel first etc... we could instead have a single _TIF_WORK_MASK
>> which includes both the bits for user work and the new bit for kernel
>> work. With preempt, the kernel work bits would also include
>> _TIF_NEED_RESCHED.
>>
>> Then you have in the common exit path, a single test for that, with a
>> fast path that skips everything and just goes to "restore" for both
>> kernel and user.
>>
>> The only possible issue is the setting of dbcr0 for BookE and 44x and we
>> can keep that as a special case keyed of MSR_PR in the resume path under
>> ifdef BOOKE (we'll probably sanitize that later with some different
>> rework anyway).
>>
>> So the exit path because something like:
>>
>> ret_from_except:
>> .. hard disable interrupts (unchanged) ...
>> read TIF flags
>> andi with _TIF_WORK_MASK
>> nothing set -> restore
>> check PR
>> set -> do_work_user
>> no set -> do_work_kernel (kprobes & preempt)
>> (both loop until relevant _TIF flags are all clear)
>> restore:
>> #ifdef BOOKE & 44x test PR & do dbcr0 stuff if needed
>> ... nornal restore ...
>
> Do you mean we should reorganize current ret_from_except for ppc32 as well?
>
I assume it may not necessary to reorganize ret_from_except for *ppc32* .
>>> do_user_signal: /* r10 contains MSR_KERNEL here */
>>> ori r10,r10,MSR_EE
>>> SYNC
>>> @@ -1202,6 +1204,30 @@ do_user_signal: /* r10 contains MSR_KERNEL here */
>>> REST_NVGPRS(r1)
>>> b recheck
>>>
>>> +restore_kprobe:
>>> + lwz r3,GPR1(r1)
>>> + subi r3,r3,INT_FRAME_SIZE; /* Allocate a trampoline exception frame */
>>> + mr r4,r1
>>> + bl copy_exc_stack /* Copy from the original to the trampoline */
>>> +
>>> + /* Do real stw operation to complete stwu */
>>> + mr r4,r1
>>> + addi r4,r4,INT_FRAME_SIZE /* Get kprobed entry */
>>> + lwz r5,GPR1(r1) /* Backup r1 */
>>> + stw r4,GPR1(r1) /* Now store that safely */
>> The above confuses me. Shouldn't you do instead something like
>>
>> lwz r4,GPR1(r1)
Now GPR1(r1) is already pointed with new r1 in emulate_step().
>> subi r3,r4,INT_FRAME_SIZE
Here we need this, 'mr r4,r1', since r1 holds current exception stack.
>> li r5,INT_FRAME_SIZE
>> bl memcpy
Then the current exception stack is migrated below the kprobed function stack.
stack flow:
-------------------------- -> old r1 when hit 'stwu r1, -AA(r1)' in our
^ ^ kprobed function entry.
| |
| |------------> AA allocated for the kprobed function
| |
| v
--------|----------------- -> new r1, also GPR1(r1). It holds the kprobed
^ | function stack , -AA(r1).
| |
| |--------------------> INT_FRAME_SIZE for program exception
| |
| v
---|--------------------- -> r1 is updated to hold program exception stack.
|
|
|------------------------> migrate the exception stack (r1) below the
| kprobed after memcpy with INT_FRAME_SIZE.
v
------------------------- -> reroute this as r1 for program exception stack.
>>
>
> Anyway I'll try this if you think memcpy is fine/safe in exception return codes.
>
>> To start with, then you need to know the "old" r1 value which may or may
>> not be related to your current r1. The emulation code should stash it
>
> If the old r1 is not related to our current r1, it shouldn't be possible to go
> restore_kprob since we set that new flag only for the current.
>
> If I'm wrong please correct me :)
If you agree what I say above, and its also avoid any issue introduced with
orig_gpr3, please check the follow:
=========
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 56212bc..277029d 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -813,9 +813,40 @@ restore_user:
#ifdef CONFIG_PREEMPT
b restore
+#endif
-/* N.B. the only way to get here is from the beq following ret_from_except. */
resume_kernel:
+#ifdef CONFIG_KPROBES
+ /* check current_thread_info, _TIF_EMULATE_STACK_STORE */
+ rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
+ lwz r0,TI_FLAGS(r9)
+ andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
+ beq+ restore_kernel
+
+ addi r9,r1,INT_FRAME_SIZE /* Get the kprobed function entry */
+
+ lwz r3,GPR1(r1)
+ subi r3,r3,INT_FRAME_SIZE /* dst: Allocate a trampoline exception
frame */
+ mr r4,r1 /* src: current exception frame */
+ li r5,INT_FRAME_SIZE /* size: INT_FRAME_SIZE */
+ mr r1,r3 /* Reroute the trampoline frame to r1 */
+ bl memcpy /* Copy from the original to the
trampoline */
+
+ /* Do real store operation to complete stwu */
+ lwz r5,GPR1(r1)
+ stw r9,0(r5)
+
+ /* Clear _TIF_EMULATE_STACK_STORE flag */
+ rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
+ lwz r0,TI_FLAGS(r9)
+ rlwinm r0,r0,0,_TIF_EMULATE_STACK_STORE
+ stw r0,TI_FLAGS(r9)
+
+restore_kernel:
+#endif
+
+#ifdef CONFIG_PREEMPT
+/* N.B. the only way to get here is from the beq following ret_from_except. */
/* check current_thread_info->preempt_count */
rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
lwz r0,TI_PREEMPT(r9)
@@ -844,8 +875,6 @@ resume_kernel:
*/
bl trace_hardirqs_on
#endif
-#else
-resume_kernel:
#endif /* CONFIG_PREEMPT */
/* interrupts are hard-disabled at this point */
Tiejun
^ permalink raw reply related
* Re: [PATCH 01/19] mxc_udc: add workaround for ENGcm09152 for i.MX25
From: Wolfram Sang @ 2011-12-13 10:18 UTC (permalink / raw)
To: Eric Bénard
Cc: Greg Kroah-Hartman, open list:FREESCALE USB PER..., open list,
Felipe Balbi, Sascha Hauer, open list:FREESCALE USB PER...,
linux-arm-kernel
In-Reply-To: <1323757911-25217-1-git-send-email-eric@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]
On Tue, Dec 13, 2011 at 07:31:33AM +0100, Eric Bénard wrote:
> this patch gives the possibility to workaround bug ENGcm09152
> on i.MX25 when the hardware workaround is also implemented on
> the board.
> It covers the workaround described on page 42 of the following Errata :
> http://cache.freescale.com/files/dsp/doc/errata/IMX25CE.pdf
Nit: The title of the ENG would help reviewing, like it is preferred to
cite patch titles when mentioning the commit-id.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 01/19] mxc_udc: add workaround for ENGcm09152 for i.MX25
From: Sascha Hauer @ 2011-12-13 10:17 UTC (permalink / raw)
To: Eric Bénard
Cc: Greg Kroah-Hartman, open list:FREESCALE USB PER..., open list,
Felipe Balbi, Sascha Hauer, open list:FREESCALE USB PER...,
linux-arm-kernel
In-Reply-To: <1323757911-25217-1-git-send-email-eric@eukrea.com>
Hi Eric,
To make the handling of your patches easier, please
- send a cover letter for multiple patches
- separate them at least by fixes and features
- if possible, do not send patches aimed at multiple
maintainers in a single series.
I picked every patch looking like a fix for now myself,
but please remember next time. I'll have a look at the non-fix
patches later.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 3/4] ppc32/kprobe: complete kprobe and migrate exception frame
From: tiejun.chen @ 2011-12-13 10:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <4EE70B13.7000102@windriver.com>
Sorry please ignore this email since I'm missing something here :(
Tiejun
tiejun.chen wrote:
>>> You need to hook into "resume_kernel" instead.
>> Maybe I'm misunderstanding what you mean since as I recall you suggestion we
>> should do this at the end of do_work.
>>
>
> I regenerate this with hooking into resume_kernel in below.
>
>>> Also, we may want to simplify the whole thing, instead of checking user
>>> vs. kernel first etc... we could instead have a single _TIF_WORK_MASK
>>> which includes both the bits for user work and the new bit for kernel
>>> work. With preempt, the kernel work bits would also include
>>> _TIF_NEED_RESCHED.
>>>
>>> Then you have in the common exit path, a single test for that, with a
>>> fast path that skips everything and just goes to "restore" for both
>>> kernel and user.
>>>
>>> The only possible issue is the setting of dbcr0 for BookE and 44x and we
>>> can keep that as a special case keyed of MSR_PR in the resume path under
>>> ifdef BOOKE (we'll probably sanitize that later with some different
>>> rework anyway).
>>>
>>> So the exit path because something like:
>>>
>>> ret_from_except:
>>> .. hard disable interrupts (unchanged) ...
>>> read TIF flags
>>> andi with _TIF_WORK_MASK
>>> nothing set -> restore
>>> check PR
>>> set -> do_work_user
>>> no set -> do_work_kernel (kprobes & preempt)
>>> (both loop until relevant _TIF flags are all clear)
>>> restore:
>>> #ifdef BOOKE & 44x test PR & do dbcr0 stuff if needed
>>> ... nornal restore ...
>> Do you mean we should reorganize current ret_from_except for ppc32 as well?
>
> I assume it may not necessary to reorganize ret_from_except for *ppc32*.
>
>>>> do_user_signal: /* r10 contains MSR_KERNEL here */
>>>> ori r10,r10,MSR_EE
>>>> SYNC
>>>> @@ -1202,6 +1204,30 @@ do_user_signal: /* r10 contains MSR_KERNEL here */
>>>> REST_NVGPRS(r1)
>>>> b recheck
>>>>
>>>> +restore_kprobe:
>>>> + lwz r3,GPR1(r1)
>>>> + subi r3,r3,INT_FRAME_SIZE; /* Allocate a trampoline exception frame */
>>>> + mr r4,r1
>>>> + bl copy_exc_stack /* Copy from the original to the trampoline */
>>>> +
>>>> + /* Do real stw operation to complete stwu */
>>>> + mr r4,r1
>>>> + addi r4,r4,INT_FRAME_SIZE /* Get kprobed entry */
>>>> + lwz r5,GPR1(r1) /* Backup r1 */
>>>> + stw r4,GPR1(r1) /* Now store that safely */
>>> The above confuses me. Shouldn't you do instead something like
>>>
>>> lwz r4,GPR1(r1)
>
> Now GPR1(r1) is already pointed with new r1 in emulate_step().
>
>>> subi r3,r4,INT_FRAME_SIZE
>
> Here we need this, 'mr r4,r1', since r1 holds current exception stack.
>
>>> li r5,INT_FRAME_SIZE
>>> bl memcpy
>
> Then the current exception stack is migrated below the kprobed function stack.
>
> stack flow:
>
> -------------------------- -> old r1 when hit 'stwu r1, -AA(r1)' in our
> ^ ^ kprobed function entry.
> | |
> | |------------> AA allocated for the kprobed function
> | |
> | v
> --------|----------------- -> new r1, also GPR1(r1). It holds the kprobed
> ^ | function stack , -AA(r1).
> | |
> | |--------------------> INT_FRAME_SIZE for program exception
> | |
> | v
> ---|--------------------- -> r1 is updated to hold program exception stack.
> |
> |
> |------------------------> migrate the exception stack (r1) below the
> | kprobed after memcpy with INT_FRAME_SIZE.
> v
> ------------------------- -> reroute this as r1 for program exception stack.
>
>> Anyway I'll try this if you think memcpy is fine/safe in exception return codes.
>>
>>> To start with, then you need to know the "old" r1 value which may or may
>>> not be related to your current r1. The emulation code should stash it
>> If the old r1 is not related to our current r1, it shouldn't be possible to go
>> restore_kprob since we set that new flag only for the current.
>
> If you agree what I say above, please check the follow:
> ======
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 56212bc..b6554c1 100644
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 56212bc..b6554c1 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -813,12 +813,40 @@ restore_user:
>
> #ifdef CONFIG_PREEMPT
> b restore
> +#endif
>
> -/* N.B. the only way to get here is from the beq following ret_from_except. */
> resume_kernel:
> /* check current_thread_info->preempt_count */
> rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
> lwz r0,TI_PREEMPT(r9)
> + andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
> + beq+ restore
> +
> + lwz r3,GPR1(r1)
> + subi r3,r3,INT_FRAME_SIZE /* Allocate a trampoline exception frame */
> + mr r4,r1
> + li r5,INT_FRAME_SIZE
> + bl memcpy /* Copy from the original to the
> trampoline */
> +
> + /* Do real store operation to complete stwu */
> + addi r4,r1,INT_FRAME_SIZE /* Get the kprobed function entry */
> + lwz r5,GPR1(r1)
> + stw r4,0(r5) /* Now store that safely */
> +
> + /* Reroute the trampoline frame to r1 */
> + subi r1,r5,INT_FRAME_SIZE
> +
> + /* Clear _TIF_EMULATE_STACK_STORE flag */
> + rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
> + lwz r0,TI_FLAGS(r9)
> + rlwinm r0,r0,0,_TIF_EMULATE_STACK_STORE
> + stw r0,TI_FLAGS(r9)
> +
> +#ifdef CONFIG_PREEMPT
> +/* N.B. the only way to get here is from the beq following ret_from_except. */
> + /* check current_thread_info->preempt_count */
> + rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
> + lwz r0,TI_PREEMPT(r9)
> cmpwi 0,r0,0 /* if non-zero, just restore regs and return */
> bne restore
> lwz r0,TI_FLAGS(r9)
> @@ -844,8 +872,6 @@ resume_kernel:
> */
> bl trace_hardirqs_on
> #endif
> -#else
> -resume_kernel:
> #endif /* CONFIG_PREEMPT */
>
> /* interrupts are hard-disabled at this point */
>
> Tiejun
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Wolfram Sang @ 2011-12-13 10:03 UTC (permalink / raw)
To: Anatolij Gustschin; +Cc: linuxppc-dev, linux-kernel, Linus Walleij
In-Reply-To: <20111213105243.101ced70@wker>
[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]
On Tue, Dec 13, 2011 at 10:52:43AM +0100, Anatolij Gustschin wrote:
> Hi Wolfram,
>
> Looks mostly good to me. Please see minor comments below.
>
> On Tue, 13 Dec 2011 10:12:48 +0100
> Wolfram Sang <w.sang@pengutronix.de> wrote:
> ...
> > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> > index ec3fcf0..25dc736 100644
> > --- a/drivers/gpio/gpio-mpc8xxx.c
> > +++ b/drivers/gpio/gpio-mpc8xxx.c
> > @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> > return 0;
> > }
> >
> > +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>
> Line over 80 chars.
In this case, I think the alternative would be far less readable.
> > +{
> > + /* GPIO 28..31 are input only on MPC5121 */
> > + if (gpio >= 28)
> > + return -EINVAL;
> > +
> > + return mpc8xxx_gpio_dir_out(gc, gpio, val);
> > +}
> > static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
>
> Please separate by an empty line. Thanks.
Ups, yes.
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Anatolij Gustschin @ 2011-12-13 9:52 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev, linux-kernel, Linus Walleij
In-Reply-To: <1323767568-9565-1-git-send-email-w.sang@pengutronix.de>
Hi Wolfram,
Looks mostly good to me. Please see minor comments below.
On Tue, 13 Dec 2011 10:12:48 +0100
Wolfram Sang <w.sang@pengutronix.de> wrote:
...
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index ec3fcf0..25dc736 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> return 0;
> }
>
> +static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
Line over 80 chars.
> +{
> + /* GPIO 28..31 are input only on MPC5121 */
> + if (gpio >= 28)
> + return -EINVAL;
> +
> + return mpc8xxx_gpio_dir_out(gc, gpio, val);
> +}
> static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
Please separate by an empty line. Thanks.
Anatolij
^ permalink raw reply
* [PATCH RESEND] gpio: mpc8xxx: don't allow input-only pins to be output for MPC5121
From: Wolfram Sang @ 2011-12-13 9:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anatolij Gustschin, linux-kernel, Linus Walleij
Add a 5121-custom reject if an input-only pin is requested to be output
(see 18.3.1.1 in the refman). Also, rewrite mach-specific quirk setup to
consume less lines which scales better.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
drivers/gpio/gpio-mpc8xxx.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index ec3fcf0..25dc736 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -115,6 +115,14 @@ static int mpc8xxx_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
return 0;
}
+static int mpc5121_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ /* GPIO 28..31 are input only on MPC5121 */
+ if (gpio >= 28)
+ return -EINVAL;
+
+ return mpc8xxx_gpio_dir_out(gc, gpio, val);
+}
static int mpc8xxx_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
{
struct of_mm_gpio_chip *mm = to_of_mm_gpio_chip(gc);
@@ -340,11 +348,10 @@ static void __init mpc8xxx_add_controller(struct device_node *np)
mm_gc->save_regs = mpc8xxx_gpio_save_regs;
gc->ngpio = MPC8XXX_GPIO_PINS;
gc->direction_input = mpc8xxx_gpio_dir_in;
- gc->direction_output = mpc8xxx_gpio_dir_out;
- if (of_device_is_compatible(np, "fsl,mpc8572-gpio"))
- gc->get = mpc8572_gpio_get;
- else
- gc->get = mpc8xxx_gpio_get;
+ gc->direction_output = of_device_is_compatible(np, "fsl,mpc5121-gpio") ?
+ mpc5121_gpio_dir_out : mpc8xxx_gpio_dir_out;
+ gc->get = of_device_is_compatible(np, "fsl,mpc8572-gpio") ?
+ mpc8572_gpio_get : mpc8xxx_gpio_get;
gc->set = mpc8xxx_gpio_set;
gc->to_irq = mpc8xxx_gpio_to_irq;
--
1.7.7.1
^ permalink raw reply related
* Re: [PATCH 3/4] ppc32/kprobe: complete kprobe and migrate exception frame
From: tiejun.chen @ 2011-12-13 8:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <4EE6DA8C.8090107@windriver.com>
>>
>> You need to hook into "resume_kernel" instead.
>
> Maybe I'm misunderstanding what you mean since as I recall you suggestion we
> should do this at the end of do_work.
>
I regenerate this with hooking into resume_kernel in below.
>> Also, we may want to simplify the whole thing, instead of checking user
>> vs. kernel first etc... we could instead have a single _TIF_WORK_MASK
>> which includes both the bits for user work and the new bit for kernel
>> work. With preempt, the kernel work bits would also include
>> _TIF_NEED_RESCHED.
>>
>> Then you have in the common exit path, a single test for that, with a
>> fast path that skips everything and just goes to "restore" for both
>> kernel and user.
>>
>> The only possible issue is the setting of dbcr0 for BookE and 44x and we
>> can keep that as a special case keyed of MSR_PR in the resume path under
>> ifdef BOOKE (we'll probably sanitize that later with some different
>> rework anyway).
>>
>> So the exit path because something like:
>>
>> ret_from_except:
>> .. hard disable interrupts (unchanged) ...
>> read TIF flags
>> andi with _TIF_WORK_MASK
>> nothing set -> restore
>> check PR
>> set -> do_work_user
>> no set -> do_work_kernel (kprobes & preempt)
>> (both loop until relevant _TIF flags are all clear)
>> restore:
>> #ifdef BOOKE & 44x test PR & do dbcr0 stuff if needed
>> ... nornal restore ...
>
> Do you mean we should reorganize current ret_from_except for ppc32 as well?
I assume it may not necessary to reorganize ret_from_except for *ppc32*.
>
>>> do_user_signal: /* r10 contains MSR_KERNEL here */
>>> ori r10,r10,MSR_EE
>>> SYNC
>>> @@ -1202,6 +1204,30 @@ do_user_signal: /* r10 contains MSR_KERNEL here */
>>> REST_NVGPRS(r1)
>>> b recheck
>>>
>>> +restore_kprobe:
>>> + lwz r3,GPR1(r1)
>>> + subi r3,r3,INT_FRAME_SIZE; /* Allocate a trampoline exception frame */
>>> + mr r4,r1
>>> + bl copy_exc_stack /* Copy from the original to the trampoline */
>>> +
>>> + /* Do real stw operation to complete stwu */
>>> + mr r4,r1
>>> + addi r4,r4,INT_FRAME_SIZE /* Get kprobed entry */
>>> + lwz r5,GPR1(r1) /* Backup r1 */
>>> + stw r4,GPR1(r1) /* Now store that safely */
>> The above confuses me. Shouldn't you do instead something like
>>
>> lwz r4,GPR1(r1)
Now GPR1(r1) is already pointed with new r1 in emulate_step().
>> subi r3,r4,INT_FRAME_SIZE
Here we need this, 'mr r4,r1', since r1 holds current exception stack.
>> li r5,INT_FRAME_SIZE
>> bl memcpy
Then the current exception stack is migrated below the kprobed function stack.
stack flow:
-------------------------- -> old r1 when hit 'stwu r1, -AA(r1)' in our
^ ^ kprobed function entry.
| |
| |------------> AA allocated for the kprobed function
| |
| v
--------|----------------- -> new r1, also GPR1(r1). It holds the kprobed
^ | function stack , -AA(r1).
| |
| |--------------------> INT_FRAME_SIZE for program exception
| |
| v
---|--------------------- -> r1 is updated to hold program exception stack.
|
|
|------------------------> migrate the exception stack (r1) below the
| kprobed after memcpy with INT_FRAME_SIZE.
v
------------------------- -> reroute this as r1 for program exception stack.
>>
>
> Anyway I'll try this if you think memcpy is fine/safe in exception return codes.
>
>> To start with, then you need to know the "old" r1 value which may or may
>> not be related to your current r1. The emulation code should stash it
>
> If the old r1 is not related to our current r1, it shouldn't be possible to go
> restore_kprob since we set that new flag only for the current.
If you agree what I say above, please check the follow:
======
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 56212bc..b6554c1 100644
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 56212bc..b6554c1 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -813,12 +813,40 @@ restore_user:
#ifdef CONFIG_PREEMPT
b restore
+#endif
-/* N.B. the only way to get here is from the beq following ret_from_except. */
resume_kernel:
/* check current_thread_info->preempt_count */
rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
lwz r0,TI_PREEMPT(r9)
+ andis. r0,r0,_TIF_EMULATE_STACK_STORE@h
+ beq+ restore
+
+ lwz r3,GPR1(r1)
+ subi r3,r3,INT_FRAME_SIZE /* Allocate a trampoline exception frame */
+ mr r4,r1
+ li r5,INT_FRAME_SIZE
+ bl memcpy /* Copy from the original to the
trampoline */
+
+ /* Do real store operation to complete stwu */
+ addi r4,r1,INT_FRAME_SIZE /* Get the kprobed function entry */
+ lwz r5,GPR1(r1)
+ stw r4,0(r5) /* Now store that safely */
+
+ /* Reroute the trampoline frame to r1 */
+ subi r1,r5,INT_FRAME_SIZE
+
+ /* Clear _TIF_EMULATE_STACK_STORE flag */
+ rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
+ lwz r0,TI_FLAGS(r9)
+ rlwinm r0,r0,0,_TIF_EMULATE_STACK_STORE
+ stw r0,TI_FLAGS(r9)
+
+#ifdef CONFIG_PREEMPT
+/* N.B. the only way to get here is from the beq following ret_from_except. */
+ /* check current_thread_info->preempt_count */
+ rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
+ lwz r0,TI_PREEMPT(r9)
cmpwi 0,r0,0 /* if non-zero, just restore regs and return */
bne restore
lwz r0,TI_FLAGS(r9)
@@ -844,8 +872,6 @@ resume_kernel:
*/
bl trace_hardirqs_on
#endif
-#else
-resume_kernel:
#endif /* CONFIG_PREEMPT */
/* interrupts are hard-disabled at this point */
Tiejun
^ permalink raw reply related
* RE: [PATCH] mmc: sdhci-pltfm: Added sdhci-adjust-timeout quirk
From: Huang Changming-R66093 @ 2011-12-13 8:00 UTC (permalink / raw)
To: Xie Xiaobo-R63061, linuxppc-dev@lists.ozlabs.org
Cc: avorontsov@ru.mvista.com, linux-mmc@vger.kernel.org,
Xie Xiaobo-R63061
In-Reply-To: <1323075320-9138-1-git-send-email-X.Xie@freescale.com>
Xiaobo, I have one other similar patch, but the property is 'sdhci,adjust-t=
imeout'.
Maybe I can repost it with add your signed-off-by?
> -----Original Message-----
> From: linuxppc-dev-bounces+r66093=3Dfreescale.com@lists.ozlabs.org
> [mailto:linuxppc-dev-bounces+r66093=3Dfreescale.com@lists.ozlabs.org] On
> Behalf Of Xie Xiaobo
> Sent: Monday, December 05, 2011 4:55 PM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: avorontsov@ru.mvista.com; linux-mmc@vger.kernel.org; Xie Xiaobo-
> R63061
> Subject: [PATCH] mmc: sdhci-pltfm: Added sdhci-adjust-timeout quirk
>=20
> Some controller provides an incorrect timeout value for transfers,
> So it need the quirk to adjust timeout value to 0xE.
> E.g. eSDHC of MPC8536, P1010, and P2020.
>=20
> Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> ---
> drivers/mmc/host/sdhci-pltfm.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>=20
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-
> pltfm.c
> index a9e12ea..b5d6b3f 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -2,7 +2,7 @@
> * sdhci-pltfm.c Support for SDHCI platform devices
> * Copyright (c) 2009 Intel Corporation
> *
> - * Copyright (c) 2007 Freescale Semiconductor, Inc.
> + * Copyright (c) 2007, 2011 Freescale Semiconductor, Inc.
> * Copyright (c) 2009 MontaVista Software, Inc.
> *
> * Authors: Xiaobo Xie <X.Xie@freescale.com>
> @@ -68,6 +68,9 @@ void sdhci_get_of_property(struct platform_device *pdev=
)
> if (of_get_property(np, "sdhci,1-bit-only", NULL))
> host->quirks |=3D SDHCI_QUIRK_FORCE_1_BIT_DATA;
>=20
> + if (of_get_property(np, "sdhci,sdhci-adjust-timeout", NULL))
> + host->quirks |=3D SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> +
> if (sdhci_of_wp_inverted(np))
> host->quirks |=3D SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
>=20
> --
> 1.6.4
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* [PATCH 01/19] mxc_udc: add workaround for ENGcm09152 for i.MX25
From: Eric Bénard @ 2011-12-13 6:31 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Greg Kroah-Hartman, open list:FREESCALE USB PER..., open list,
Felipe Balbi, Sascha Hauer, open list:FREESCALE USB PER...
this patch gives the possibility to workaround bug ENGcm09152
on i.MX25 when the hardware workaround is also implemented on
the board.
It covers the workaround described on page 42 of the following Errata :
http://cache.freescale.com/files/dsp/doc/errata/IMX25CE.pdf
Signed-off-by: Eric Bénard <eric@eukrea.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Li Yang <leoli@freescale.com>
---
drivers/usb/gadget/fsl_mxc_udc.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index dcbc0a2..4aff05d 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,7 @@
static struct clk *mxc_ahb_clk;
static struct clk *mxc_usb_clk;
-/* workaround ENGcm09152 for i.MX35 */
+/* workaround ENGcm09152 for i.MX25/35 */
#define USBPHYCTRL_OTGBASE_OFFSET 0x608
#define USBPHYCTRL_EVDO (1 << 23)
@@ -89,16 +89,20 @@ eenahb:
void fsl_udc_clk_finalize(struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
- if (cpu_is_mx35()) {
+ if (cpu_is_mx25() || cpu_is_mx35()) {
unsigned int v;
-
- /* workaround ENGcm09152 for i.MX35 */
+ void __iomem *otgbase;
+ if (cpu_is_mx25())
+ otgbase = MX25_IO_ADDRESS(MX25_USB_BASE_ADDR +
+ USBPHYCTRL_OTGBASE_OFFSET);
+ else if (cpu_is_mx35())
+ otgbase = MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+ USBPHYCTRL_OTGBASE_OFFSET);
+
+ /* workaround ENGcm09152 for i.MX25/35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
- writel(v | USBPHYCTRL_EVDO,
- MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ v = readl(otgbase);
+ writel(v | USBPHYCTRL_EVDO, otgbase);
}
}
--
1.7.6.4
^ permalink raw reply related
* [PATCH] powerpc: Fix comment explaining our VSID layout
From: Anton Blanchard @ 2011-12-13 6:16 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
We support 16TB of user address space and half a million contexts
so update the comment to reflect this.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-powerpc/arch/powerpc/include/asm/mmu-hash64.h
===================================================================
--- linux-powerpc.orig/arch/powerpc/include/asm/mmu-hash64.h 2011-12-13 14:47:14.498301148 +1100
+++ linux-powerpc/arch/powerpc/include/asm/mmu-hash64.h 2011-12-13 14:58:01.085510915 +1100
@@ -312,10 +312,9 @@ extern void slb_set_size(u16 size);
* (i.e. everything above 0xC000000000000000), except the very top
* segment, which simplifies several things.
*
- * - We allow for 15 significant bits of ESID and 20 bits of
- * context for user addresses. i.e. 8T (43 bits) of address space for
- * up to 1M contexts (although the page table structure and context
- * allocation will need changes to take advantage of this).
+ * - We allow for 16 significant bits of ESID and 19 bits of
+ * context for user addresses. i.e. 16T (44 bits) of address space for
+ * up to half a million contexts.
*
* - The scramble function gives robust scattering in the hash
* table (at least based on some initial results). The previous
^ permalink raw reply
* Re: [PATCH 1/1] ppc64: fix missing to check all bits of _TIF_USER_WORK_MASK in preempt
From: tiejun.chen @ 2011-12-13 6:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1323755709.19891.60.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Tue, 2011-12-13 at 13:01 +0800, tiejun.chen wrote:
>> Tiejun Chen wrote:
>>> In entry_64.S version of ret_from_except_lite, you'll notice that
>>> in the !preempt case, after we've checked MSR_PR we test for any
>>> TIF flag in _TIF_USER_WORK_MASK to decide whether to go to do_work
>>> or not. However, in the preempt case, we do a convoluted trick to
>>> test SIGPENDING only if PR was set and always test NEED_RESCHED ...
>>> but we forget to test any other bit of _TIF_USER_WORK_MASK !!! So
>>> that means that with preempt, we completely fail to test for things
>>> like single step, syscall tracing, etc...
>>>
>>> This should be fixed as the following path:
>>>
>>> - Test PR. If set, go to test_work_user, else continue.
>>>
>>> - In test_work_user, always test for _TIF_USER_WORK_MASK to decide to
>>> go to do_work, maybe call it do_user_work
>>>
>>> - In test_work_kernel, test for _TIF_KERNEL_WORK_MASK which is set to
>>> our new flag along with NEED_RESCHED if preempt is enabled and branch to
>>> do_kernel_work.
>> Ben,
>>
>> Any comment for this?
>
> Sorry, I didn't get to review that one yet (nor reply to your newer
I'm nothing, please do this when you're fine completely.
Thanks
Tiejun
> responses), I have very sore eyes and basically had to get off the
> computer. Hopefully I'll be better tomorrow.
>
> Cheers,
> Ben.
^ permalink raw reply
* Re: [PATCH 1/1] ppc64: fix missing to check all bits of _TIF_USER_WORK_MASK in preempt
From: Benjamin Herrenschmidt @ 2011-12-13 5:55 UTC (permalink / raw)
To: tiejun.chen; +Cc: linuxppc-dev
In-Reply-To: <4EE6DC33.6090409@windriver.com>
On Tue, 2011-12-13 at 13:01 +0800, tiejun.chen wrote:
> Tiejun Chen wrote:
> > In entry_64.S version of ret_from_except_lite, you'll notice that
> > in the !preempt case, after we've checked MSR_PR we test for any
> > TIF flag in _TIF_USER_WORK_MASK to decide whether to go to do_work
> > or not. However, in the preempt case, we do a convoluted trick to
> > test SIGPENDING only if PR was set and always test NEED_RESCHED ...
> > but we forget to test any other bit of _TIF_USER_WORK_MASK !!! So
> > that means that with preempt, we completely fail to test for things
> > like single step, syscall tracing, etc...
> >
> > This should be fixed as the following path:
> >
> > - Test PR. If set, go to test_work_user, else continue.
> >
> > - In test_work_user, always test for _TIF_USER_WORK_MASK to decide to
> > go to do_work, maybe call it do_user_work
> >
> > - In test_work_kernel, test for _TIF_KERNEL_WORK_MASK which is set to
> > our new flag along with NEED_RESCHED if preempt is enabled and branch to
> > do_kernel_work.
>
> Ben,
>
> Any comment for this?
Sorry, I didn't get to review that one yet (nor reply to your newer
responses), I have very sore eyes and basically had to get off the
computer. Hopefully I'll be better tomorrow.
Cheers,
Ben.
^ permalink raw reply
* Linux port availability for p5010 processor
From: Vineeth @ 2011-12-13 5:33 UTC (permalink / raw)
To: linuxppc-dev; +Cc: rajarama.b, Senthil CGC
[-- Attachment #1: Type: text/plain, Size: 297 bytes --]
Do we have a linux port available for freescale P5010 processor (with
single E5500 core) ?
*(found arch/powerpc/platforms/pseries ; and a some details on
kernel/cputable.c *)
Is there any reference board which uses this processor ? any reference in
DTS file also will be helpful.
Thanks
Vineeth
[-- Attachment #2: Type: text/html, Size: 1076 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] ppc64: fix missing to check all bits of _TIF_USER_WORK_MASK in preempt
From: tiejun.chen @ 2011-12-13 5:01 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In-Reply-To: <1323681035-19350-1-git-send-email-tiejun.chen@windriver.com>
Tiejun Chen wrote:
> In entry_64.S version of ret_from_except_lite, you'll notice that
> in the !preempt case, after we've checked MSR_PR we test for any
> TIF flag in _TIF_USER_WORK_MASK to decide whether to go to do_work
> or not. However, in the preempt case, we do a convoluted trick to
> test SIGPENDING only if PR was set and always test NEED_RESCHED ...
> but we forget to test any other bit of _TIF_USER_WORK_MASK !!! So
> that means that with preempt, we completely fail to test for things
> like single step, syscall tracing, etc...
>
> This should be fixed as the following path:
>
> - Test PR. If set, go to test_work_user, else continue.
>
> - In test_work_user, always test for _TIF_USER_WORK_MASK to decide to
> go to do_work, maybe call it do_user_work
>
> - In test_work_kernel, test for _TIF_KERNEL_WORK_MASK which is set to
> our new flag along with NEED_RESCHED if preempt is enabled and branch to
> do_kernel_work.
Ben,
Any comment for this?
Tiejun
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
> arch/powerpc/kernel/entry_64.S | 33 +++++++++++++++------------------
> 1 files changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index d834425..9e70b9a 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -571,27 +571,26 @@ _GLOBAL(ret_from_except_lite)
> mtmsrd r9,1 /* Update machine state */
> #endif /* CONFIG_PPC_BOOK3E */
>
> -#ifdef CONFIG_PREEMPT
> - clrrdi r9,r1,THREAD_SHIFT /* current_thread_info() */
> - li r0,_TIF_NEED_RESCHED /* bits to check */
> - ld r3,_MSR(r1)
> - ld r4,TI_FLAGS(r9)
> - /* Move MSR_PR bit in r3 to _TIF_SIGPENDING position in r0 */
> - rlwimi r0,r3,32+TIF_SIGPENDING-MSR_PR_LG,_TIF_SIGPENDING
> - and. r0,r4,r0 /* check NEED_RESCHED and maybe SIGPENDING */
> - bne do_work
> -
> -#else /* !CONFIG_PREEMPT */
> ld r3,_MSR(r1) /* Returning to user mode? */
> andi. r3,r3,MSR_PR
> - beq restore /* if not, just restore regs and return */
> + bne test_work_user
>
> + clrrdi r9,r1,THREAD_SHIFT /* current_thread_info() */
> + li r0,_TIF_USER_WORK_MASK
> +#ifdef CONFIG_PREEMPT
> + ori r0,r0,_TIF_NEED_RESCHED
> +#endif
> + ld r4,TI_FLAGS(r9)
> + and. r0,r4,r0 /* check NEED_RESCHED and maybe _TIF_USER_WORK_MASK */
> + bne do_kernel_work
> + b restore /* if so, just restore regs and return */
> +
> +test_work_user:
> /* Check current_thread_info()->flags */
> clrrdi r9,r1,THREAD_SHIFT
> ld r4,TI_FLAGS(r9)
> andi. r0,r4,_TIF_USER_WORK_MASK
> - bne do_work
> -#endif
> + bne do_user_work
>
> restore:
> BEGIN_FW_FTR_SECTION
> @@ -693,10 +692,8 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
> b .ret_from_except_lite /* loop back and handle more */
> #endif
>
> -do_work:
> +do_kernel_work:
> #ifdef CONFIG_PREEMPT
> - andi. r0,r3,MSR_PR /* Returning to user mode? */
> - bne user_work
> /* Check that preempt_count() == 0 and interrupts are enabled */
> lwz r8,TI_PREEMPT(r9)
> cmpwi cr1,r8,0
> @@ -738,9 +735,9 @@ do_work:
> bne 1b
> b restore
>
> -user_work:
> #endif /* CONFIG_PREEMPT */
>
> +do_user_work:
> /* Enable interrupts */
> #ifdef CONFIG_PPC_BOOK3E
> wrteei 1
^ permalink raw reply
* Re: [PATCH 2/4] ppc32/kprobe: introduce copy_exc_stack
From: tiejun.chen @ 2011-12-13 4:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1323730871.19891.25.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Mon, 2011-12-12 at 16:50 +0800, Tiejun Chen wrote:
>> We need a copy mechanism to migrate exception stack. But looks copy_page()
>> already implement this well so we can complete copy_exc_stack() based on
>> that directly.
>
> I'd rather you don't hijack copy_page which is quite sensitive. The
> emulation isn't performance critical so a "dumber" routine would work
Yes, I just think we should introduce good performance so I 'steal' the original
copy_page().
> fine.
>
> Why not use memcpy ? You can call it from assembly.
I'd like to switch to memcpy.
Thanks
Tiejun
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/kprobe: introduce a new thread flag
From: tiejun.chen @ 2011-12-13 4:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1323730739.19891.23.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Mon, 2011-12-12 at 16:50 +0800, Tiejun Chen wrote:
>> We need to add a new thread flag, TIF_KPROBE/_TIF_DELAYED_KPROBE,
>> for handling kprobe operation while exiting exception.
>
> The basic idea is sane, however the instruction emulation isn't per-se
> kprobe specific. It could be used by xmon too for example. I'd rather
> use a different name, something like TIF_EMULATE_STACK_STORE or
Its good term so I'll use this directly :)
Thanks
Tiejun
> something like that.
>
> Cheers,
> Ben.
>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>> arch/powerpc/include/asm/thread_info.h | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
>> index 836f231..3378734 100644
>> --- a/arch/powerpc/include/asm/thread_info.h
>> +++ b/arch/powerpc/include/asm/thread_info.h
>> @@ -112,6 +112,7 @@ static inline struct thread_info *current_thread_info(void)
>> #define TIF_FREEZE 14 /* Freezing for suspend */
>> #define TIF_SYSCALL_TRACEPOINT 15 /* syscall tracepoint instrumentation */
>> #define TIF_RUNLATCH 16 /* Is the runlatch enabled? */
>> +#define TIF_KPROBE 17 /* Is the delayed kprobe operation? */
>>
>> /* as above, but as bit values */
>> #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
>> @@ -130,6 +131,7 @@ static inline struct thread_info *current_thread_info(void)
>> #define _TIF_FREEZE (1<<TIF_FREEZE)
>> #define _TIF_SYSCALL_TRACEPOINT (1<<TIF_SYSCALL_TRACEPOINT)
>> #define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
>> +#define _TIF_DELAYED_KPROBE (1<<TIF_KPROBE)
>> #define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
>> _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT)
>>
>
>
>
^ permalink raw reply
* Re: [PATCH 3/4] ppc32/kprobe: complete kprobe and migrate exception frame
From: tiejun.chen @ 2011-12-13 4:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1323731987.19891.40.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Mon, 2011-12-12 at 16:50 +0800, Tiejun Chen wrote:
>> We can't emulate stwu since that may corrupt current exception stack.
>> So we will have to do real store operation in the exception return code.
>>
>> Firstly we'll allocate a trampoline exception frame below the kprobed
>> function stack and copy the current exception frame to the trampoline.
>> Then we can do this real store operation to implement 'stwu', and reroute
>> the trampoline frame to r1 to complete this exception migration.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>> arch/powerpc/kernel/entry_32.S | 26 ++++++++++++++++++++++++++
>> 1 files changed, 26 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 56212bc..d56e311 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -1185,6 +1185,8 @@ recheck:
>> bne- do_resched
>> andi. r0,r9,_TIF_USER_WORK_MASK
>> beq restore_user
>> + andis. r0,r9,_TIF_DELAYED_KPROBE@h
>> + bne- restore_kprobe
>
> Same comment as earlier about name. Note that you're not hooking in the
> right place. "recheck" is only reached if you -already- went out of the
> normal exit path and only when going back to user space unless I'm
> missing something (which is really the case you don't care about).
>
> You need to hook into "resume_kernel" instead.
Maybe I'm misunderstanding what you mean since as I recall you suggestion we
should do this at the end of do_work.
>
> Also, we may want to simplify the whole thing, instead of checking user
> vs. kernel first etc... we could instead have a single _TIF_WORK_MASK
> which includes both the bits for user work and the new bit for kernel
> work. With preempt, the kernel work bits would also include
> _TIF_NEED_RESCHED.
>
> Then you have in the common exit path, a single test for that, with a
> fast path that skips everything and just goes to "restore" for both
> kernel and user.
>
> The only possible issue is the setting of dbcr0 for BookE and 44x and we
> can keep that as a special case keyed of MSR_PR in the resume path under
> ifdef BOOKE (we'll probably sanitize that later with some different
> rework anyway).
>
> So the exit path because something like:
>
> ret_from_except:
> .. hard disable interrupts (unchanged) ...
> read TIF flags
> andi with _TIF_WORK_MASK
> nothing set -> restore
> check PR
> set -> do_work_user
> no set -> do_work_kernel (kprobes & preempt)
> (both loop until relevant _TIF flags are all clear)
> restore:
> #ifdef BOOKE & 44x test PR & do dbcr0 stuff if needed
> ... nornal restore ...
Do you mean we should reorganize current ret_from_except for ppc32 as well?
>
>> do_user_signal: /* r10 contains MSR_KERNEL here */
>> ori r10,r10,MSR_EE
>> SYNC
>> @@ -1202,6 +1204,30 @@ do_user_signal: /* r10 contains MSR_KERNEL here */
>> REST_NVGPRS(r1)
>> b recheck
>>
>> +restore_kprobe:
>> + lwz r3,GPR1(r1)
>> + subi r3,r3,INT_FRAME_SIZE; /* Allocate a trampoline exception frame */
>> + mr r4,r1
>> + bl copy_exc_stack /* Copy from the original to the trampoline */
>> +
>> + /* Do real stw operation to complete stwu */
>> + mr r4,r1
>> + addi r4,r4,INT_FRAME_SIZE /* Get kprobed entry */
>> + lwz r5,GPR1(r1) /* Backup r1 */
>> + stw r4,GPR1(r1) /* Now store that safely */
>
> The above confuses me. Shouldn't you do instead something like
>
> lwz r4,GPR1(r1)
> subi r3,r4,INT_FRAME_SIZE
> li r5,INT_FRAME_SIZE
> bl memcpy
>
Anyway I'll try this if you think memcpy is fine/safe in exception return codes.
> To start with, then you need to know the "old" r1 value which may or may
> not be related to your current r1. The emulation code should stash it
If the old r1 is not related to our current r1, it shouldn't be possible to go
restore_kprob since we set that new flag only for the current.
If I'm wrong please correct me :)
Thanks
Tiejun
> into the int frame in an unused slot such as "orig_gpr3" (since that
> only pertains to restarting syscalls which we aren't doing here).
>
> Then you adjust your r1 and do something like
>
> lwz r3,GPR1(r1)
> lwz r0,ORIG_GPR3(r1)
> stw r0,0(r3)
>
> To perform the store, before doing the rest:
>
>> + /* Reroute the trampoline frame to r1 */
>> + subi r5,r5,INT_FRAME_SIZE
>> + mr r1,r5
>> +
>> + /* Clear _TIF_DELAYED_KPROBE flag */
>> + rlwinm r9,r1,0,0,(31-THREAD_SHIFT)
>> + lwz r0,TI_FLAGS(r9)
>> + rlwinm r0,r0,0,_TIF_DELAYED_KPROBE
>> + stw r0,TI_FLAGS(r9)
>> +
>> + b restore
>> +
>> /*
>> * We come here when we are at the end of handling an exception
>> * that occurred at a place where taking an exception will lose
^ 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