Re: 3.7-rc7: BUG: MAX_STACK_TRACE_ENTRIES too low!

2013-01-14 Thread Li Zhong
On Sat, 2013-01-12 at 12:43 -0800, Christian Kujau wrote:
> On Wed, 28 Nov 2012 at 16:41, Li Zhong wrote:
> > On Tue, 2012-11-27 at 19:22 -0800, Christian Kujau wrote:
> > > On Tue, 27 Nov 2012 at 19:06, Christian Kujau wrote:
> > > > the same thing[0] happened again in 3.7-rc7, after ~20h uptime:
> > > 
> > > I found the following on patchwork, but this seems to deal with powerpc64 
> > > only, while this PowerBook G4 of mine is powerpc32:
> > > 
> > >   http://patchwork.ozlabs.org/patch/193414/
> > > 
> > > It looks related, but then again, I fail to parse assember...
> > 
> > Hi Christian, 
> > 
> > Would you please help to try the following fix? I don't have a powerpc32
> > machine for test...
> 
> The patch hasn't made it into mainline yet. Any chance that this will be 
> included in 3.8?

I don't know...

FYI, it is already in the next of ppc tree 
http://git.kernel.org/?p=linux/kernel/git/benh/powerpc.git;a=shortlog;h=refs/heads/next

I guess it would get into 3.9, at least. 

Thanks, Zhong

> 
> Thanks,
> Christian.
> 
> > ==
> > diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> > index 9499385..cadebfd 100644
> > --- a/arch/powerpc/kernel/entry_32.S
> > +++ b/arch/powerpc/kernel/entry_32.S
> > @@ -439,6 +439,8 @@ ret_from_fork:
> >  ret_from_kernel_thread:
> > REST_NVGPRS(r1)
> > bl  schedule_tail
> > +   li  r3,0
> > +   stw r3,0(r1)
> > mtlrr14
> > mr  r3,r15
> > PPC440EP_ERR42
> > ==
> > 
> > > 
> > > Christian.
> > > 
> > > > [40007.339487] [sched_delayed] sched: RT throttling activated
> > > > [69731.388717] BUG: MAX_STACK_TRACE_ENTRIES too low!
> > > > [69731.390371] turning off the locking correctness validator.
> > > > [69731.391942] Call Trace:
> > > > [69731.393525] [c9a61c10] [c0009064] show_stack+0x70/0x1bc (unreliable)
> > > > [69731.395152] [c9a61c50] [c0077460] save_trace+0xfc/0x114
> > > > [69731.396735] [c9a61c60] [c007be20] __lock_acquire+0x1568/0x19b8
> > > > [69731.398296] [c9a61d00] [c007c2c0] lock_acquire+0x50/0x70
> > > > [69731.399857] [c9a61d20] [c0550e28] _raw_spin_lock_irq+0x5c/0x78
> > > > [69731.401419] [c9a61d40] [c054fb58] __schedule+0xd8/0x534
> > > > [69731.402972] [c9a61da0] [c0550094] _cond_resched+0x50/0x68
> > > > [69731.404527] [c9a61db0] [c0479908] dst_gc_task+0xbc/0x258
> > > > [69731.406070] [c9a61e40] [c004eeb8] process_one_work+0x1f4/0x49c
> > > > [69731.407585] [c9a61e80] [c004f644] worker_thread+0x14c/0x400
> > > > [69731.409075] [c9a61eb0] [c0057634] kthread+0xbc/0xc0
> > > > [69731.410521] [c9a61f40] [c0011ad4] ret_from_kernel_thread+0x5c/0x64
> > > > [...repeated 54 times...]
> > > > 
> > > > Anyone knows what this is about?
> > > > 
> > > > Thanks,
> > > > Christian.
> > > > 
> > > > [0] http://lkml.indiana.edu/hypermail/linux/kernel/1211.0/03025.html
> > 
> > 
> > 
> 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v5 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

2013-01-14 Thread Peter Chen
As we use platform_device_id for fsl-usb2-udc driver, it needs to
change clk connection-id, or the related devm_clk_get will be failed.

Signed-off-by: Peter Chen 
---
 arch/arm/mach-imx/clk-imx25.c   |6 +++---
 arch/arm/mach-imx/clk-imx27.c   |6 +++---
 arch/arm/mach-imx/clk-imx31.c   |6 +++---
 arch/arm/mach-imx/clk-imx35.c   |6 +++---
 arch/arm/mach-imx/clk-imx51-imx53.c |6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index b197aa7..67e353d 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -254,9 +254,9 @@ int __init mx25_clocks_init(void)
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
clk_register_clkdev(clk[usbotg_ahb], "ahb", "mxc-ehci.2");
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usbotg_ahb], "ahb", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx25");
+   clk_register_clkdev(clk[usbotg_ahb], "ahb", "imx-udc-mx25");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx25");
clk_register_clkdev(clk[nfc_ipg_per], NULL, "imx25-nand.0");
/* i.mx25 has the i.mx35 type cspi */
clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index 4c1d1e4..1ffe3b5 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref)
clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx21-fb.0");
clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0");
clk_register_clkdev(clk[per4_gate], "per", "imx27-camera.0");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+   clk_register_clkdev(clk[usb_ipg_gate], "ipg", "imx-udc-mx27");
+   clk_register_clkdev(clk[usb_ahb_gate], "ahb", "imx-udc-mx27");
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
clk_register_clkdev(clk[usb_ipg_gate], "ipg", "mxc-ehci.0");
clk_register_clkdev(clk[usb_ahb_gate], "ahb", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c
index 8be64e0..ef66eaf 100644
--- a/arch/arm/mach-imx/clk-imx31.c
+++ b/arch/arm/mach-imx/clk-imx31.c
@@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref)
clk_register_clkdev(clk[usb_div_post], "per", "mxc-ehci.2");
clk_register_clkdev(clk[usb_gate], "ahb", "mxc-ehci.2");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
-   clk_register_clkdev(clk[usb_div_post], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_gate], "ahb", "fsl-usb2-udc");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div_post], "per", "imx-udc-mx31");
+   clk_register_clkdev(clk[usb_gate], "ahb", "imx-udc-mx31");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx31");
clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
/* i.mx31 has the i.mx21 type uart */
clk_register_clkdev(clk[uart1_gate], "per", "imx21-uart.0");
diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index 66f3d65..69fe9c8 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -251,9 +251,9 @@ int __init mx35_clocks_init()
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.2");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usbotg_gate], "ahb", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx35");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx35");
+   clk_register_clkdev(clk[usbotg_gate], "ahb", "imx-udc-mx35");
clk_register_clkdev(clk[wdog_gate], NULL, "imx2-wdt.0");
clk_register_clkdev(clk[nfc_div], NULL, "imx25-nand.0");
clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c 
b/arch/arm/mach-imx/clk-imx51-imx53.c
index 579023f..fb7cb84 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long 
rate_ckil,
clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.2");
clk_register_clkdev(clk[usboh3_gate], "ipg", "mx

[PATCH v5 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Peter Chen
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.

Signed-off-by: Peter Chen 
---
 drivers/usb/gadget/fsl_mxc_udc.c  |   30 +++---
 drivers/usb/gadget/fsl_udc_core.c |4 +++-
 drivers/usb/gadget/fsl_usb2_udc.h |5 +++--
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1176bd8..bb65c46 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
 
 /* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET  0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET  0x8
 #define USBPHYCTRL_EVDO(1 << 23)
 
 int fsl_udc_clk_init(struct platform_device *pdev)
@@ -77,25 +78,40 @@ eclkrate:
return ret;
 }
 
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-   unsigned int v;
+   int ret = 0;
 
/* workaround ENGcm09152 for i.MX35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-   v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-   USBPHYCTRL_OTGBASE_OFFSET));
+   unsigned int v;
+   struct resource *res = platform_get_resource
+   (pdev, IORESOURCE_MEM, 0);
+   void __iomem *phy_regs = ioremap(res->start +
+   MX35_USBPHYCTRL_OFFSET, 512);
+   if (!phy_regs) {
+   dev_err(&pdev->dev, "ioremap for phy address fails\n");
+   ret = -EINVAL;
+   goto ioremap_err;
+   }
+
+   v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
writel(v | USBPHYCTRL_EVDO,
-   MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-   USBPHYCTRL_OTGBASE_OFFSET));
+   phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
+
+   iounmap(phy_regs);
}
 
+
+ioremap_err:
/* ULPI transceivers don't need usbpll */
if (pdata->phy_mode == FSL_USB2_PHY_ULPI) {
clk_disable_unprepare(mxc_per_clk);
mxc_per_clk = NULL;
}
+
+   return ret;
 }
 
 void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index c971e84..347b1ed 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2543,7 +2543,9 @@ static int __init fsl_udc_probe(struct platform_device 
*pdev)
dr_controller_setup(udc_controller);
}
 
-   fsl_udc_clk_finalize(pdev);
+   ret = fsl_udc_clk_finalize(pdev);
+   if (ret)
+   goto err_free_irq;
 
/* Setup gadget structure */
udc_controller->gadget.ops = &fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h 
b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..c6703bb 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -592,15 +592,16 @@ static inline struct ep_queue_head *get_qh_by_ep(struct 
fsl_ep *ep)
 struct platform_device;
 #ifdef CONFIG_ARCH_MXC
 int fsl_udc_clk_init(struct platform_device *pdev);
-void fsl_udc_clk_finalize(struct platform_device *pdev);
+int fsl_udc_clk_finalize(struct platform_device *pdev);
 void fsl_udc_clk_release(void);
 #else
 static inline int fsl_udc_clk_init(struct platform_device *pdev)
 {
return 0;
 }
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
+   return 0;
 }
 static inline void fsl_udc_clk_release(void)
 {
-- 
1.7.0.4


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Peter Chen
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
already used pdata to differentiate runtime

Meanwhile we update the platform code accordingly.

Signed-off-by: Peter Chen 
---
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ---
 drivers/usb/gadget/fsl_mxc_udc.c  |   24 +---
 drivers/usb/gadget/fsl_udc_core.c |   42 +
 4 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h 
b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
 
 #include 
 struct imx_fsl_usb2_udc_data {
+   const char *devid;
resource_size_t iobase;
resource_size_t irq;
 };
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c 
b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..fb527c7 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_fsl_usb2_udc_data_entry_single(soc)
\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)
\
{   \
+   .devid = _devid,\
.iobase = soc ## _USB_OTG_BASE_ADDR,\
.irq = soc ## _INT_USB_OTG, \
}
 
 #ifdef CONFIG_SOC_IMX25
 const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX25);
+   imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx25");
 #endif /* ifdef CONFIG_SOC_IMX25 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX27);
+   imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX31);
+   imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx31");
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX35);
+   imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx35");
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 #ifdef CONFIG_SOC_IMX51
 const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX51);
+   imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
 #endif
 
 struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
.flags = IORESOURCE_IRQ,
},
};
-   return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
+   return imx_add_platform_device_dmamask(data->devid, -1,
res, ARRAY_SIZE(res),
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
 }
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..1176bd8 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
 #include 
 #include 
 
-#include 
-
 static struct clk *mxc_ahb_clk;
 static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
clk_prepare_enable(mxc_per_clk);
 
/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-   if (!cpu_is_mx51()) {
+   if (strcmp(pdev->id_entry->name, "imx-udc-mx51")) {
freq = clk_get_rate(mxc_per_clk);
if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
(freq < 5000 || freq > 60001000)) {
@@ -82,17 +80,15 @@ eclkrate:
 void fsl_udc_clk_finalize(struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-   if (cpu_is_mx35()) {
-   unsigned int v;
-
-   /* workaround ENGcm09152 for i.MX35 */
-   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));
- 

[PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c

2013-01-14 Thread Peter Chen
Changes for v5:
- Using strcmp to get specific SoC
- Delete one cpu_is_mx35() as it has already pdata runtime check

Changes for v4:
- Using pdev's struct resource to do ioremap
- Add ioremap return value check

Changes for v3:
- Split the one big patch into three patches

Changes for v2:
- Add const for fsl_udc_devtype
- Do ioremap for phy address at fsl-mxc-udc

Peter Chen (3):
  usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

 arch/arm/mach-imx/clk-imx25.c |6 +-
 arch/arm/mach-imx/clk-imx27.c |6 +-
 arch/arm/mach-imx/clk-imx31.c |6 +-
 arch/arm/mach-imx/clk-imx35.c |6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ---
 drivers/usb/gadget/fsl_mxc_udc.c  |   40 --
 drivers/usb/gadget/fsl_udc_core.c |   46 +
 drivers/usb/gadget/fsl_usb2_udc.h |5 +-
 10 files changed, 82 insertions(+), 55 deletions(-)


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Peter Chen
On Mon, Jan 14, 2013 at 07:57:24PM +0200, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 06:54:22PM +0100, Marc Kleine-Budde wrote:
> > On 01/14/2013 06:40 PM, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote:
> > > 
> > > 
> > > 
> >  Usually there isn't any Changelog between IP cores used in the 
> >  different
> >  fsl processors (at least available outside of fsl), that makes it quite
> >  difficult to say if something found on one imx is really the same as on
> >  the other one. And they (usually) don't provide any versioning
> >  information in a register or the documentation.
> > 
> >  just my 2¢
> > >>>
> > >>> $SUBJECT is trying to differentiate a single feature (or maybe two) to
> > >>> replace cpu_is_xxx(), then expose that on driver_data without creating
> > >>> one enum value for each release from fsl.
> > >>
> > >> Felipe, every one or two SoCs may have their special operations for
> > >> integrate PHY interface, clk operation, or workaround for IC
> > >> limitation.
> > > 
> > > the particular PHY and clk used should be hidden by phy layer and clk
> > > API respectively. Workarounds, fair enough, we need to handle them; but
> > > ideally those should be based on runtime revision detection, not some
> > > hackery using driver_data.
> > 
> > If this is actually possible, I'd love to do this. But IP vendor don't
> > include a version register in their cores. :(
> 
> then fair enough, driver_data or platform_data is the way to go, still
> my point (a) below is valid.
I will send v5 patch with your suggestion.

> 
> -- 
> balbi



-- 

Best Regards,
Peter Chen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: PS3 platform is broken on Linux 3.7.0

2013-01-14 Thread Geoff Levand
Hi,

On Fri, 2013-01-11 at 18:12 -0800, Geoff Levand wrote:
> I checked these, and Michael's 407821a34fce89b4f0b031dbab5cec7d059f46bc
> does indeed cause the LV1 hypervisor to panic early, and if that is
> reverted, Aneesh's 048ee0993ec8360abb0b51bdf8f8721e9ed62ec4 hits a BUG.

Just to give an update, I did a little more work on it and found that
the call to lv1_insert_htab_entry() inside ps3_hpte_insert() is
failing.

   
http://git.kernel.org/?p=linux/kernel/git/geoff/ps3-linux.git;a=blob;f=arch/powerpc/platforms/ps3/htab.c;hb=HEAD#l70

The values of the variables printed all look strange compared with
commit 048ee0993 reverted.  I'll try do some more work on it this
week.

-Geoff





___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc: kernel/kgdb.c: fix memory leakage

2013-01-14 Thread Cong Ding
the variable backup_current_thread_info isn't freed before existing the
function.

Signed-off-by: Cong Ding 
---
 arch/powerpc/kernel/kgdb.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 8747447..5ca82cd 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -154,12 +154,12 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
 static int kgdb_singlestep(struct pt_regs *regs)
 {
struct thread_info *thread_info, *exception_thread_info;
-   struct thread_info *backup_current_thread_info = \
-   (struct thread_info *)kmalloc(sizeof(struct thread_info), 
GFP_KERNEL);
+   struct thread_info *backup_current_thread_info;
 
if (user_mode(regs))
return 0;
 
+   backup_current_thread_info = (struct thread_info 
*)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
/*
 * On Book E and perhaps other processors, singlestep is handled on
 * the critical exception stack.  This causes current_thread_info()
@@ -185,6 +185,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
/* Restore current_thread_info lastly. */
memcpy(exception_thread_info, backup_current_thread_info, 
sizeof *thread_info);
 
+   kfree(backup_current_thread_info);
return 1;
 }
 
-- 
1.7.9.5

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH 1/4] powerpc/mpc5121: add common .dtsi and use it in mpc5121ads.dts

2013-01-14 Thread Anatolij Gustschin
Provide common mpc5121.dtsi file for mpc5121 SoC and modify
mpc5121ads.dts to use mpc5121.dtsi.

Signed-off-by: Anatolij Gustschin 
---
 arch/powerpc/boot/dts/mpc5121.dtsi   |  410 ++
 arch/powerpc/boot/dts/mpc5121ads.dts |  319 ---
 2 files changed, 449 insertions(+), 280 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/mpc5121.dtsi

diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi 
b/arch/powerpc/boot/dts/mpc5121.dtsi
new file mode 100644
index 000..723e292
--- /dev/null
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -0,0 +1,410 @@
+/*
+ * base MPC5121 Device Tree Source
+ *
+ * Copyright 2007-2008 Freescale Semiconductor Inc.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+/dts-v1/;
+
+/ {
+   model = "mpc5121";
+   compatible = "fsl,mpc5121";
+   #address-cells = <1>;
+   #size-cells = <1>;
+interrupt-parent = <&ipic>;
+
+   aliases {
+   ethernet0 = ð0;
+   pci = &pci;
+   };
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   PowerPC,5121@0 {
+   device_type = "cpu";
+   reg = <0>;
+   d-cache-line-size = <0x20>; /* 32 bytes */
+   i-cache-line-size = <0x20>; /* 32 bytes */
+   d-cache-size = <0x8000>;/* L1, 32K */
+   i-cache-size = <0x8000>;/* L1, 32K */
+   timebase-frequency = <4950>;/* 49.5 MHz (csb/4) */
+   bus-frequency = <19800>;/* 198 MHz csb bus */
+   clock-frequency = <39600>;  /* 396 MHz ppc core */
+   };
+   };
+
+   memory {
+   device_type = "memory";
+   reg = <0x 0x1000>;  /* 256MB at 0 */
+   };
+
+   mbx@2000 {
+   compatible = "fsl,mpc5121-mbx";
+   reg = <0x2000 0x4000>;
+   interrupts = <66 0x8>;
+   };
+
+   sram@3000 {
+   compatible = "fsl,mpc5121-sram";
+   reg = <0x3000 0x2>; /* 128K at 0x3000 */
+   };
+
+   nfc@4000 {
+   compatible = "fsl,mpc5121-nfc";
+   reg = <0x4000 0x10>;/* 1M at 0x4000 */
+   interrupts = <6 8>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   };
+
+   localbus@8020 {
+   compatible = "fsl,mpc5121-localbus";
+   #address-cells = <2>;
+   #size-cells = <1>;
+   reg = <0x8020 0x40>;
+   interrupts = <7 0x8>;
+   ranges = <0x0 0x0 0xfc00 0x0400>;
+   };
+
+   soc@8000 {
+   compatible = "fsl,mpc5121-immr";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   #interrupt-cells = <2>;
+   ranges = <0x0 0x8000 0x40>;
+   reg = <0x8000 0x40>;
+   bus-frequency = <6600>; /* 66 MHz ips bus */
+
+
+   /*
+* IPIC
+* interrupts cell = 
+* sense values match linux IORESOURCE_IRQ_* defines:
+* sense == 8: Level, low assertion
+* sense == 2: Edge, high-to-low change
+*/
+   ipic: interrupt-controller@c00 {
+   compatible = "fsl,mpc5121-ipic", "fsl,ipic";
+   interrupt-controller;
+   #address-cells = <0>;
+   #interrupt-cells = <2>;
+   reg = <0xc00 0x100>;
+   };
+
+   /* Watchdog timer */
+   wdt@900 {
+   compatible = "fsl,mpc5121-wdt";
+   reg = <0x900 0x100>;
+   };
+
+   /* Real time clock */
+   rtc@a00 {
+   compatible = "fsl,mpc5121-rtc";
+   reg = <0xa00 0x100>;
+   interrupts = <79 0x8 80 0x8>;
+   };
+
+   /* Reset module */
+   reset@e00 {
+   compatible = "fsl,mpc5121-reset";
+   reg = <0xe00 0x100>;
+   };
+
+   /* Clock control */
+   clock@f00 {
+   compatible = "fsl,mpc5121-clock";
+   reg = <0xf00 0x100>;
+   };
+
+   /* Power Management Controller */
+   pmc@1000{
+   compatible = "fsl,mpc5121-pmc";
+   reg = <0x1000 0x100>;
+   interrupts = <83 0

[PATCH 2/4] powerpc/mpc5121: pdm360ng.dts: use common mpc5121.dtsi

2013-01-14 Thread Anatolij Gustschin
Change dts file for pdm360ng board to use common mpc5121
SoC dtsi file.

Signed-off-by: Anatolij Gustschin 
---
 arch/powerpc/boot/dts/pdm360ng.dts |  273 
 1 files changed, 31 insertions(+), 242 deletions(-)

diff --git a/arch/powerpc/boot/dts/pdm360ng.dts 
b/arch/powerpc/boot/dts/pdm360ng.dts
index 94dfa5c..0b06947 100644
--- a/arch/powerpc/boot/dts/pdm360ng.dts
+++ b/arch/powerpc/boot/dts/pdm360ng.dts
@@ -13,7 +13,7 @@
  * option) any later version.
  */
 
-/dts-v1/;
+/include/ "mpc5121.dtsi"
 
 / {
model = "pdm360ng";
@@ -22,38 +22,12 @@
#size-cells = <1>;
interrupt-parent = <&ipic>;
 
-   aliases {
-   ethernet0 = ð0;
-   };
-
-   cpus {
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   PowerPC,5121@0 {
-   device_type = "cpu";
-   reg = <0>;
-   d-cache-line-size = <0x20>; // 32 bytes
-   i-cache-line-size = <0x20>; // 32 bytes
-   d-cache-size = <0x8000>;// L1, 32K
-   i-cache-size = <0x8000>;// L1, 32K
-   timebase-frequency = <4950>;// 49.5 MHz (csb/4)
-   bus-frequency = <19800>;// 198 MHz csb bus
-   clock-frequency = <39600>;  // 396 MHz ppc core
-   };
-   };
-
memory {
device_type = "memory";
reg = <0x 0x2000>;  // 512MB at 0
};
 
nfc@4000 {
-   compatible = "fsl,mpc5121-nfc";
-   reg = <0x4000 0x10>;
-   interrupts = <0x6 0x8>;
-   #address-cells = <0x1>;
-   #size-cells = <0x1>;
bank-width = <0x1>;
chips = <0x1>;
 
@@ -63,17 +37,7 @@
};
};
 
-   sram@5000 {
-   compatible = "fsl,mpc5121-sram";
-   reg = <0x5000 0x2>; // 128K at 0x5000
-   };
-
localbus@8020 {
-   compatible = "fsl,mpc5121-localbus";
-   #address-cells = <2>;
-   #size-cells = <1>;
-   reg = <0x8020 0x40>;
-
ranges = <0x0 0x0 0xf000 0x1000   /* Flash */
  0x2 0x0 0x5004 0x0002>; /* CS2: MRAM */
 
@@ -129,74 +93,8 @@
};
 
soc@8000 {
-   compatible = "fsl,mpc5121-immr";
-   #address-cells = <1>;
-   #size-cells = <1>;
-   #interrupt-cells = <2>;
-   ranges = <0x0 0x8000 0x40>;
-   reg = <0x8000 0x40>;
-   bus-frequency = <6600>; // 66 MHz ips bus
-
-   // IPIC
-   // interrupts cell = 
-   // sense values match linux IORESOURCE_IRQ_* defines:
-   // sense == 8: Level, low assertion
-   // sense == 2: Edge, high-to-low change
-   //
-   ipic: interrupt-controller@c00 {
-   compatible = "fsl,mpc5121-ipic", "fsl,ipic";
-   interrupt-controller;
-   #address-cells = <0>;
-   #interrupt-cells = <2>;
-   reg = <0xc00 0x100>;
-   };
-
-   rtc@a00 {   // Real time clock
-   compatible = "fsl,mpc5121-rtc";
-   reg = <0xa00 0x100>;
-   interrupts = <79 0x8 80 0x8>;
-   };
-
-   reset@e00 { // Reset module
-   compatible = "fsl,mpc5121-reset";
-   reg = <0xe00 0x100>;
-   };
-
-   clock@f00 { // Clock control
-   compatible = "fsl,mpc5121-clock";
-   reg = <0xf00 0x100>;
-   };
-
-   pmc@1000{   //Power Management Controller
-   compatible = "fsl,mpc5121-pmc";
-   reg = <0x1000 0x100>;
-   interrupts = <83 0x2>;
-   };
-
-   gpio@1100 {
-   compatible = "fsl,mpc5121-gpio";
-   reg = <0x1100 0x100>;
-   interrupts = <78 0x8>;
-   };
-
-   can@1300 {
-   compatible = "fsl,mpc5121-mscan";
-   interrupts = <12 0x8>;
-   reg = <0x1300 0x80>;
-   };
-
-   can@1380 {
-   compatible = "fsl,mpc5121-mscan";
-   interrupts = <13 0x8>;
-   reg = <0x1380 0x80>;
-   };
 
i2c@1700 {
-   #address-cells = <1>;
-   #size-cells = <0>;
-   compatible = "fsl,mpc5121-i2c";
-   re

[PATCH 4/4] mpc5121: don't check PSC ac97 using node name

2013-01-14 Thread Anatolij Gustschin
The .dtsi now names all PSC nodes as "psc", so this ac97
check won't work. Check for ac97 PSC using compatible
property.

Signed-off-by: Anatolij Gustschin 
---
 arch/powerpc/platforms/512x/clock.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/512x/clock.c 
b/arch/powerpc/platforms/512x/clock.c
index d0095c8..7937361 100644
--- a/arch/powerpc/platforms/512x/clock.c
+++ b/arch/powerpc/platforms/512x/clock.c
@@ -695,7 +695,7 @@ static void psc_clks_init(void)
 * AC97 is special rate clock does
 * not go through normal path
 */
-   if (strcmp("ac97", np->name) == 0)
+   if (of_device_is_compatible(np, "fsl,mpc5121-psc-ac97"))
clk->rate = ac97_clk.rate;
else
psc_calc_rate(clk, pscnum, np);
-- 
1.7.5.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH 3/4] mpc5121: remove obsolete cell-index property from PSC clock code

2013-01-14 Thread Anatolij Gustschin
Don't use cell-index from device tree, obtain the PSC number
from PSCx register offset.

Signed-off-by: Anatolij Gustschin 
---
 arch/powerpc/platforms/512x/clock.c |7 +++
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/512x/clock.c 
b/arch/powerpc/platforms/512x/clock.c
index 9f771e0..d0095c8 100644
--- a/arch/powerpc/platforms/512x/clock.c
+++ b/arch/powerpc/platforms/512x/clock.c
@@ -680,13 +680,12 @@ static void psc_calc_rate(struct clk *clk, int pscnum, 
struct device_node *np)
 static void psc_clks_init(void)
 {
struct device_node *np;
-   const u32 *cell_index;
struct platform_device *ofdev;
+   u32 reg;
 
for_each_compatible_node(np, NULL, "fsl,mpc5121-psc") {
-   cell_index = of_get_property(np, "cell-index", NULL);
-   if (cell_index) {
-   int pscnum = *cell_index;
+   if (!of_property_read_u32(np, "reg", ®)) {
+   int pscnum = (reg & 0xf00) >> 8;
struct clk *clk = psc_dev_clk(pscnum);
 
clk->flags = CLK_HAS_RATE | CLK_HAS_CTRL;
-- 
1.7.5.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Toshi Kani
On Mon, 2013-01-14 at 11:21 -0800, Greg KH wrote:
> On Mon, Jan 14, 2013 at 08:07:35PM +0100, Rafael J. Wysocki wrote:
> > On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> > > On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > > > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > > > device hotplug header and defines the order values of 
> > > > > > > ACPI-specific
> > > > > > > handlers.
> > > > > > > 
> > > > > > > Signed-off-by: Toshi Kani 
> > > > > > > ---
> > > > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > > > 
> > > > > > >  1 file changed, 48 insertions(+)
> > > > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > > > 
> > > > > > > diff --git a/include/acpi/sys_hotplug.h 
> > > > > > > b/include/acpi/sys_hotplug.h
> > > > > > > new file mode 100644
> > > > > > > index 000..ad80f61
> > > > > > > --- /dev/null
> > > > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > > > @@ -0,0 +1,48 @@
> > > > > > > +/*
> > > > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > > > + *
> > > > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > > > + *   Toshi Kani 
> > > > > > > + *
> > > > > > > + * This program is free software; you can redistribute it and/or 
> > > > > > > modify
> > > > > > > + * it under the terms of the GNU General Public License version 
> > > > > > > 2 as
> > > > > > > + * published by the Free Software Foundation.
> > > > > > > + */
> > > > > > > +
> > > > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > > > +
> > > > > > > +#include 
> > > > > > > +#include 
> > > > > > > +#include 
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * System device hot-plug operation proceeds in the following 
> > > > > > > order.
> > > > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > > > + *
> > > > > > > + * The order values below define the calling sequence of 
> > > > > > > ACPI-specific
> > > > > > > + * handlers for each phase in ascending order.  The order value 
> > > > > > > of
> > > > > > > + * platform-neutral handlers are defined in 
> > > > > > > .
> > > > > > > + */
> > > > > > > +
> > > > > > > +/* Add Validate order values */
> > > > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER  0   /* must 
> > > > > > > be first */
> > > > > > > +
> > > > > > > +/* Add Execute order values */
> > > > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER   10
> > > > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER   20
> > > > > > > +
> > > > > > > +/* Add Commit order values */
> > > > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER10
> > > > > > > +
> > > > > > > +/* Delete Validate order values */
> > > > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER  0   /* must 
> > > > > > > be first */
> > > > > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER  10
> > > > > > > +
> > > > > > > +/* Delete Execute order values */
> > > > > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER   100
> > > > > > > +
> > > > > > > +/* Delete Commit order values */
> > > > > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER100
> > > > > > > +
> > > > > > > +#endif   /* _ACPI_SYS_HOTPLUG_H */
> > > > > > > --
> > > > > > 
> > > > > > Why did you use the particular values above?
> > > > > 
> > > > > The ordering values above are used to define the relative order among
> > > > > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER 
> > > > > can
> > > > > potentially be 21 since it is still larger than 20 for
> > > > > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 
> > > > > 100
> > > > > so that more platform-neutral handlers can be added in between 20 and
> > > > > 100 in future.
> > > > 
> > > > I thought so, but I don't think it's a good idea to add gaps like this.
> > > 
> > > OK, I will use an equal gap of 10 for all values.  So, the 100 in the
> > > above example will be changed to 30.  
> > 
> > I wonder why you want to have those gaps at all.
> > 
> > Anyway, this is just a small detail and it doesn't mean I don't have more
> > comments.  I just need some more time to get the big picture idea of how 
> > this
> > is supposed to work and perhaps Greg will have some remarks too.
> 
> Yes, give me a few days to catch up on other patches before I get the
> chance to review these.

That's great!  Thanks Greg!
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Toshi Kani
On Mon, 2013-01-14 at 20:07 +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> > On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > > device hotplug header and defines the order values of ACPI-specific
> > > > > > handlers.
> > > > > > 
> > > > > > Signed-off-by: Toshi Kani 
> > > > > > ---
> > > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > > 
> > > > > >  1 file changed, 48 insertions(+)
> > > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > > 
> > > > > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > > > > new file mode 100644
> > > > > > index 000..ad80f61
> > > > > > --- /dev/null
> > > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > > @@ -0,0 +1,48 @@
> > > > > > +/*
> > > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > > + *
> > > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > > + * Toshi Kani 
> > > > > > + *
> > > > > > + * This program is free software; you can redistribute it and/or 
> > > > > > modify
> > > > > > + * it under the terms of the GNU General Public License version 2 
> > > > > > as
> > > > > > + * published by the Free Software Foundation.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +/*
> > > > > > + * System device hot-plug operation proceeds in the following 
> > > > > > order.
> > > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > > + *
> > > > > > + * The order values below define the calling sequence of 
> > > > > > ACPI-specific
> > > > > > + * handlers for each phase in ascending order.  The order value of
> > > > > > + * platform-neutral handlers are defined in .
> > > > > > + */
> > > > > > +
> > > > > > +/* Add Validate order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must 
> > > > > > be first */
> > > > > > +
> > > > > > +/* Add Execute order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > > > > > +
> > > > > > +/* Add Commit order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > > > > > +
> > > > > > +/* Delete Validate order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must 
> > > > > > be first */
> > > > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > > > > > +
> > > > > > +/* Delete Execute order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > > > > > +
> > > > > > +/* Delete Commit order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > > > > > +
> > > > > > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > > > > > --
> > > > > 
> > > > > Why did you use the particular values above?
> > > > 
> > > > The ordering values above are used to define the relative order among
> > > > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> > > > potentially be 21 since it is still larger than 20 for
> > > > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> > > > so that more platform-neutral handlers can be added in between 20 and
> > > > 100 in future.
> > > 
> > > I thought so, but I don't think it's a good idea to add gaps like this.
> > 
> > OK, I will use an equal gap of 10 for all values.  So, the 100 in the
> > above example will be changed to 30.  
> 
> I wonder why you want to have those gaps at all.

Oh, I see.  I think some gap is helpful since it allows a new handler to
come between without recompiling other modules.  For instance, OEM
vendors may want to add their own handlers with loadable modules after
the kernel is distributed.

> Anyway, this is just a small detail and it doesn't mean I don't have more
> comments.  I just need some more time to get the big picture idea of how this
> is supposed to work and perhaps Greg will have some remarks too.

Yes, I am well-aware of that. :-)  Please let me know if you have any
questions.  I'd be happy to explain any details.

Thanks a lot for reviewing!
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Greg KH
On Mon, Jan 14, 2013 at 08:07:35PM +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> > On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > > device hotplug header and defines the order values of ACPI-specific
> > > > > > handlers.
> > > > > > 
> > > > > > Signed-off-by: Toshi Kani 
> > > > > > ---
> > > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > > 
> > > > > >  1 file changed, 48 insertions(+)
> > > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > > 
> > > > > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > > > > new file mode 100644
> > > > > > index 000..ad80f61
> > > > > > --- /dev/null
> > > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > > @@ -0,0 +1,48 @@
> > > > > > +/*
> > > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > > + *
> > > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > > + * Toshi Kani 
> > > > > > + *
> > > > > > + * This program is free software; you can redistribute it and/or 
> > > > > > modify
> > > > > > + * it under the terms of the GNU General Public License version 2 
> > > > > > as
> > > > > > + * published by the Free Software Foundation.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +/*
> > > > > > + * System device hot-plug operation proceeds in the following 
> > > > > > order.
> > > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > > + *
> > > > > > + * The order values below define the calling sequence of 
> > > > > > ACPI-specific
> > > > > > + * handlers for each phase in ascending order.  The order value of
> > > > > > + * platform-neutral handlers are defined in .
> > > > > > + */
> > > > > > +
> > > > > > +/* Add Validate order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must 
> > > > > > be first */
> > > > > > +
> > > > > > +/* Add Execute order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > > > > > +
> > > > > > +/* Add Commit order values */
> > > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > > > > > +
> > > > > > +/* Delete Validate order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must 
> > > > > > be first */
> > > > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > > > > > +
> > > > > > +/* Delete Execute order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > > > > > +
> > > > > > +/* Delete Commit order values */
> > > > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > > > > > +
> > > > > > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > > > > > --
> > > > > 
> > > > > Why did you use the particular values above?
> > > > 
> > > > The ordering values above are used to define the relative order among
> > > > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> > > > potentially be 21 since it is still larger than 20 for
> > > > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> > > > so that more platform-neutral handlers can be added in between 20 and
> > > > 100 in future.
> > > 
> > > I thought so, but I don't think it's a good idea to add gaps like this.
> > 
> > OK, I will use an equal gap of 10 for all values.  So, the 100 in the
> > above example will be changed to 30.  
> 
> I wonder why you want to have those gaps at all.
> 
> Anyway, this is just a small detail and it doesn't mean I don't have more
> comments.  I just need some more time to get the big picture idea of how this
> is supposed to work and perhaps Greg will have some remarks too.

Yes, give me a few days to catch up on other patches before I get the
chance to review these.

greg k-h
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Toshi Kani
On Mon, 2013-01-14 at 19:48 +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 08:33:48 AM Toshi Kani wrote:
> > On Fri, 2013-01-11 at 22:23 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 10, 2013 04:40:19 PM Toshi Kani wrote:
> > > > Added include/linux/sys_hotplug.h, which defines the system device
> > > > hotplug framework interfaces used by the framework itself and
> > > > handlers.
> > > > 
> > > > The order values define the calling sequence of handlers.  For add
> > > > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > > > CPU so that threads on new CPUs can start using their local memory.
> > > > The ordering of the delete execute is symmetric to the add execute.
> > > > 
> > > > struct shp_request defines a hot-plug request information.  The
> > > > device resource information is managed with a list so that a single
> > > > request may target to multiple devices.
> > > > 
> >  :
> > > > +
> > > > +struct shp_device {
> > > > +   struct list_headlist;
> > > > +   struct device   *device;
> > > > +   enum shp_class  class;
> > > > +   union shp_dev_info  info;
> > > > +};
> > > > +
> > > > +/*
> > > > + * Hot-plug request
> > > > + */
> > > > +struct shp_request {
> > > > +   /* common info */
> > > > +   enum shp_operation  operation;  /* operation */
> > > > +
> > > > +   /* hot-plug event info: only valid for hot-plug operations */
> > > > +   void*handle;/* FW handle */
> > > 
> > > What's the role of handle here?
> > 
> > On ACPI-based platforms, the handle keeps a notified ACPI handle when a
> > hot-plug request is made.  ACPI bus handlers, acpi_add_execute() /
> > acpi_del_execute(), then scans / trims ACPI devices from the handle.
> 
> OK, so this is ACPI-specific and should be described as such.

Other FW interface I know is parisc, which has mod_index (module index)
to identify a unique object, just like what ACPI handle does.  The
handle can keep the mod_index as an opaque value as well.  But as you
said, I do not know if the handle works for all other FWs.  So, I will
add descriptions, such that the hot-plug event info is modeled after
ACPI and may need to be revisited when supporting other FW.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Rafael J. Wysocki
On Monday, January 14, 2013 11:42:09 AM Toshi Kani wrote:
> On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> > On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > > device hotplug header and defines the order values of ACPI-specific
> > > > > handlers.
> > > > > 
> > > > > Signed-off-by: Toshi Kani 
> > > > > ---
> > > > >  include/acpi/sys_hotplug.h |   48 
> > > > > 
> > > > >  1 file changed, 48 insertions(+)
> > > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > > 
> > > > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > > > new file mode 100644
> > > > > index 000..ad80f61
> > > > > --- /dev/null
> > > > > +++ b/include/acpi/sys_hotplug.h
> > > > > @@ -0,0 +1,48 @@
> > > > > +/*
> > > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > > + *
> > > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > > + *   Toshi Kani 
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or 
> > > > > modify
> > > > > + * it under the terms of the GNU General Public License version 2 as
> > > > > + * published by the Free Software Foundation.
> > > > > + */
> > > > > +
> > > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > > +
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +/*
> > > > > + * System device hot-plug operation proceeds in the following order.
> > > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > > + *
> > > > > + * The order values below define the calling sequence of 
> > > > > ACPI-specific
> > > > > + * handlers for each phase in ascending order.  The order value of
> > > > > + * platform-neutral handlers are defined in .
> > > > > + */
> > > > > +
> > > > > +/* Add Validate order values */
> > > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER  0   /* must 
> > > > > be first */
> > > > > +
> > > > > +/* Add Execute order values */
> > > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER   10
> > > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER   20
> > > > > +
> > > > > +/* Add Commit order values */
> > > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER10
> > > > > +
> > > > > +/* Delete Validate order values */
> > > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER  0   /* must 
> > > > > be first */
> > > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER  10
> > > > > +
> > > > > +/* Delete Execute order values */
> > > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER   100
> > > > > +
> > > > > +/* Delete Commit order values */
> > > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER100
> > > > > +
> > > > > +#endif   /* _ACPI_SYS_HOTPLUG_H */
> > > > > --
> > > > 
> > > > Why did you use the particular values above?
> > > 
> > > The ordering values above are used to define the relative order among
> > > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> > > potentially be 21 since it is still larger than 20 for
> > > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> > > so that more platform-neutral handlers can be added in between 20 and
> > > 100 in future.
> > 
> > I thought so, but I don't think it's a good idea to add gaps like this.
> 
> OK, I will use an equal gap of 10 for all values.  So, the 100 in the
> above example will be changed to 30.  

I wonder why you want to have those gaps at all.

Anyway, this is just a small detail and it doesn't mean I don't have more
comments.  I just need some more time to get the big picture idea of how this
is supposed to work and perhaps Greg will have some remarks too.

Thanks,
Rafael



-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Toshi Kani
On Mon, 2013-01-14 at 19:47 +0100, Rafael J. Wysocki wrote:
> On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> > On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > > device hotplug header and defines the order values of ACPI-specific
> > > > handlers.
> > > > 
> > > > Signed-off-by: Toshi Kani 
> > > > ---
> > > >  include/acpi/sys_hotplug.h |   48 
> > > > 
> > > >  1 file changed, 48 insertions(+)
> > > >  create mode 100644 include/acpi/sys_hotplug.h
> > > > 
> > > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > > new file mode 100644
> > > > index 000..ad80f61
> > > > --- /dev/null
> > > > +++ b/include/acpi/sys_hotplug.h
> > > > @@ -0,0 +1,48 @@
> > > > +/*
> > > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > > + *
> > > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > > + * Toshi Kani 
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License version 2 as
> > > > + * published by the Free Software Foundation.
> > > > + */
> > > > +
> > > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > > +#define _ACPI_SYS_HOTPLUG_H
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +/*
> > > > + * System device hot-plug operation proceeds in the following order.
> > > > + *   Validate phase -> Execute phase -> Commit phase
> > > > + *
> > > > + * The order values below define the calling sequence of ACPI-specific
> > > > + * handlers for each phase in ascending order.  The order value of
> > > > + * platform-neutral handlers are defined in .
> > > > + */
> > > > +
> > > > +/* Add Validate order values */
> > > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must 
> > > > be first */
> > > > +
> > > > +/* Add Execute order values */
> > > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > > > +
> > > > +/* Add Commit order values */
> > > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > > > +
> > > > +/* Delete Validate order values */
> > > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must 
> > > > be first */
> > > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > > > +
> > > > +/* Delete Execute order values */
> > > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > > > +
> > > > +/* Delete Commit order values */
> > > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > > > +
> > > > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > > > --
> > > 
> > > Why did you use the particular values above?
> > 
> > The ordering values above are used to define the relative order among
> > handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> > potentially be 21 since it is still larger than 20 for
> > SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> > so that more platform-neutral handlers can be added in between 20 and
> > 100 in future.
> 
> I thought so, but I don't think it's a good idea to add gaps like this.

OK, I will use an equal gap of 10 for all values.  So, the 100 in the
above example will be changed to 30.   

Thanks,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Rafael J. Wysocki
On Monday, January 14, 2013 08:33:48 AM Toshi Kani wrote:
> On Fri, 2013-01-11 at 22:23 +0100, Rafael J. Wysocki wrote:
> > On Thursday, January 10, 2013 04:40:19 PM Toshi Kani wrote:
> > > Added include/linux/sys_hotplug.h, which defines the system device
> > > hotplug framework interfaces used by the framework itself and
> > > handlers.
> > > 
> > > The order values define the calling sequence of handlers.  For add
> > > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > > CPU so that threads on new CPUs can start using their local memory.
> > > The ordering of the delete execute is symmetric to the add execute.
> > > 
> > > struct shp_request defines a hot-plug request information.  The
> > > device resource information is managed with a list so that a single
> > > request may target to multiple devices.
> > > 
>  :
> > > +
> > > +struct shp_device {
> > > + struct list_headlist;
> > > + struct device   *device;
> > > + enum shp_class  class;
> > > + union shp_dev_info  info;
> > > +};
> > > +
> > > +/*
> > > + * Hot-plug request
> > > + */
> > > +struct shp_request {
> > > + /* common info */
> > > + enum shp_operation  operation;  /* operation */
> > > +
> > > + /* hot-plug event info: only valid for hot-plug operations */
> > > + void*handle;/* FW handle */
> > 
> > What's the role of handle here?
> 
> On ACPI-based platforms, the handle keeps a notified ACPI handle when a
> hot-plug request is made.  ACPI bus handlers, acpi_add_execute() /
> acpi_del_execute(), then scans / trims ACPI devices from the handle.

OK, so this is ACPI-specific and should be described as such.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Rafael J. Wysocki
On Monday, January 14, 2013 08:53:53 AM Toshi Kani wrote:
> On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> > On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > > device hotplug header and defines the order values of ACPI-specific
> > > handlers.
> > > 
> > > Signed-off-by: Toshi Kani 
> > > ---
> > >  include/acpi/sys_hotplug.h |   48 
> > > 
> > >  1 file changed, 48 insertions(+)
> > >  create mode 100644 include/acpi/sys_hotplug.h
> > > 
> > > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > > new file mode 100644
> > > index 000..ad80f61
> > > --- /dev/null
> > > +++ b/include/acpi/sys_hotplug.h
> > > @@ -0,0 +1,48 @@
> > > +/*
> > > + * sys_hotplug.h - ACPI System device hot-plug framework
> > > + *
> > > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > > + *   Toshi Kani 
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#ifndef _ACPI_SYS_HOTPLUG_H
> > > +#define _ACPI_SYS_HOTPLUG_H
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +/*
> > > + * System device hot-plug operation proceeds in the following order.
> > > + *   Validate phase -> Execute phase -> Commit phase
> > > + *
> > > + * The order values below define the calling sequence of ACPI-specific
> > > + * handlers for each phase in ascending order.  The order value of
> > > + * platform-neutral handlers are defined in .
> > > + */
> > > +
> > > +/* Add Validate order values */
> > > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER  0   /* must be 
> > > first */
> > > +
> > > +/* Add Execute order values */
> > > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER   10
> > > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER   20
> > > +
> > > +/* Add Commit order values */
> > > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER10
> > > +
> > > +/* Delete Validate order values */
> > > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER  0   /* must be 
> > > first */
> > > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER  10
> > > +
> > > +/* Delete Execute order values */
> > > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER   100
> > > +
> > > +/* Delete Commit order values */
> > > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER100
> > > +
> > > +#endif   /* _ACPI_SYS_HOTPLUG_H */
> > > --
> > 
> > Why did you use the particular values above?
> 
> The ordering values above are used to define the relative order among
> handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
> potentially be 21 since it is still larger than 20 for
> SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
> so that more platform-neutral handlers can be added in between 20 and
> 100 in future.

I thought so, but I don't think it's a good idea to add gaps like this.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
On Mon, Jan 14, 2013 at 06:54:22PM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 06:40 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote:
> > 
> > 
> > 
>  Usually there isn't any Changelog between IP cores used in the different
>  fsl processors (at least available outside of fsl), that makes it quite
>  difficult to say if something found on one imx is really the same as on
>  the other one. And they (usually) don't provide any versioning
>  information in a register or the documentation.
> 
>  just my 2¢
> >>>
> >>> $SUBJECT is trying to differentiate a single feature (or maybe two) to
> >>> replace cpu_is_xxx(), then expose that on driver_data without creating
> >>> one enum value for each release from fsl.
> >>
> >> Felipe, every one or two SoCs may have their special operations for
> >> integrate PHY interface, clk operation, or workaround for IC
> >> limitation.
> > 
> > the particular PHY and clk used should be hidden by phy layer and clk
> > API respectively. Workarounds, fair enough, we need to handle them; but
> > ideally those should be based on runtime revision detection, not some
> > hackery using driver_data.
> 
> If this is actually possible, I'd love to do this. But IP vendor don't
> include a version register in their cores. :(

then fair enough, driver_data or platform_data is the way to go, still
my point (a) below is valid.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Marc Kleine-Budde
On 01/14/2013 06:40 PM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote:
> 
> 
> 
 Usually there isn't any Changelog between IP cores used in the different
 fsl processors (at least available outside of fsl), that makes it quite
 difficult to say if something found on one imx is really the same as on
 the other one. And they (usually) don't provide any versioning
 information in a register or the documentation.

 just my 2¢
>>>
>>> $SUBJECT is trying to differentiate a single feature (or maybe two) to
>>> replace cpu_is_xxx(), then expose that on driver_data without creating
>>> one enum value for each release from fsl.
>>
>> Felipe, every one or two SoCs may have their special operations for
>> integrate PHY interface, clk operation, or workaround for IC
>> limitation.
> 
> the particular PHY and clk used should be hidden by phy layer and clk
> API respectively. Workarounds, fair enough, we need to handle them; but
> ideally those should be based on runtime revision detection, not some
> hackery using driver_data.

If this is actually possible, I'd love to do this. But IP vendor don't
include a version register in their cores. :(

>> Maybe, it will add more future or SoCs (maybe not for this driver) in
>> the future, using enum is easier than string comparison for expanding
>> something.
> 
> a) I never told you to *not* use enum. I said that creating DEVICE_A,
> DEVICE_B, DEVICE_C, DEVICE_D and DEVICE_E values when DEVICE_B,
> DEVICE_C and DEVICE_E behave exactly the same is unnecessary.
> 
> b) you can't be expecting to add future SoCs support to fsl udc, I have
> already said and will repeat for the last time: move to chipidea ASAP.
> 
> New SoCs cannot be added to fsl udc, you *must* use chipidea for
> anything new and move the legacy to chipidea eventually. I will wait for
> a full year for you to do that, but after that I will have to start
> deleting drivers for the sake of avoid duplication of effort.

+1

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
Hi,

On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote:



> > > Usually there isn't any Changelog between IP cores used in the different
> > > fsl processors (at least available outside of fsl), that makes it quite
> > > difficult to say if something found on one imx is really the same as on
> > > the other one. And they (usually) don't provide any versioning
> > > information in a register or the documentation.
> > > 
> > > just my 2¢
> > 
> > $SUBJECT is trying to differentiate a single feature (or maybe two) to
> > replace cpu_is_xxx(), then expose that on driver_data without creating
> > one enum value for each release from fsl.
> 
> Felipe, every one or two SoCs may have their special operations for
> integrate PHY interface, clk operation, or workaround for IC
> limitation.

the particular PHY and clk used should be hidden by phy layer and clk
API respectively. Workarounds, fair enough, we need to handle them; but
ideally those should be based on runtime revision detection, not some
hackery using driver_data.

> Maybe, it will add more future or SoCs (maybe not for this driver) in
> the future, using enum is easier than string comparison for expanding
> something.

a) I never told you to *not* use enum. I said that creating DEVICE_A,
DEVICE_B, DEVICE_C, DEVICE_D and DEVICE_E values when DEVICE_B,
DEVICE_C and DEVICE_E behave exactly the same is unnecessary.

b) you can't be expecting to add future SoCs support to fsl udc, I have
already said and will repeat for the last time: move to chipidea ASAP.

New SoCs cannot be added to fsl udc, you *must* use chipidea for
anything new and move the legacy to chipidea eventually. I will wait for
a full year for you to do that, but after that I will have to start
deleting drivers for the sake of avoid duplication of effort.

cheers

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH v2 02/12] ACPI: Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Toshi Kani
On Fri, 2013-01-11 at 22:25 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 04:40:20 PM Toshi Kani wrote:
> > Added include/acpi/sys_hotplug.h, which is ACPI-specific system
> > device hotplug header and defines the order values of ACPI-specific
> > handlers.
> > 
> > Signed-off-by: Toshi Kani 
> > ---
> >  include/acpi/sys_hotplug.h |   48 
> > 
> >  1 file changed, 48 insertions(+)
> >  create mode 100644 include/acpi/sys_hotplug.h
> > 
> > diff --git a/include/acpi/sys_hotplug.h b/include/acpi/sys_hotplug.h
> > new file mode 100644
> > index 000..ad80f61
> > --- /dev/null
> > +++ b/include/acpi/sys_hotplug.h
> > @@ -0,0 +1,48 @@
> > +/*
> > + * sys_hotplug.h - ACPI System device hot-plug framework
> > + *
> > + * Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
> > + * Toshi Kani 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef _ACPI_SYS_HOTPLUG_H
> > +#define _ACPI_SYS_HOTPLUG_H
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/*
> > + * System device hot-plug operation proceeds in the following order.
> > + *   Validate phase -> Execute phase -> Commit phase
> > + *
> > + * The order values below define the calling sequence of ACPI-specific
> > + * handlers for each phase in ascending order.  The order value of
> > + * platform-neutral handlers are defined in .
> > + */
> > +
> > +/* Add Validate order values */
> > +#define SHP_ACPI_BUS_ADD_VALIDATE_ORDER0   /* must be 
> > first */
> > +
> > +/* Add Execute order values */
> > +#define SHP_ACPI_BUS_ADD_EXECUTE_ORDER 10
> > +#define SHP_ACPI_RES_ADD_EXECUTE_ORDER 20
> > +
> > +/* Add Commit order values */
> > +#define SHP_ACPI_BUS_ADD_COMMIT_ORDER  10
> > +
> > +/* Delete Validate order values */
> > +#define SHP_ACPI_BUS_DEL_VALIDATE_ORDER0   /* must be 
> > first */
> > +#define SHP_ACPI_RES_DEL_VALIDATE_ORDER10
> > +
> > +/* Delete Execute order values */
> > +#define SHP_ACPI_BUS_DEL_EXECUTE_ORDER 100
> > +
> > +/* Delete Commit order values */
> > +#define SHP_ACPI_BUS_DEL_COMMIT_ORDER  100
> > +
> > +#endif /* _ACPI_SYS_HOTPLUG_H */
> > --
> 
> Why did you use the particular values above?

The ordering values above are used to define the relative order among
handlers.  For instance, the 100 for SHP_ACPI_BUS_DEL_EXECUTE_ORDER can
potentially be 21 since it is still larger than 20 for
SHP_MEM_DEL_EXECUTE_ORDER defined in linux/sys_hotplug.h.  I picked 100
so that more platform-neutral handlers can be added in between 20 and
100 in future.

Thanks,
-Toshi


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH v2 01/12] Add sys_hotplug.h for system device hotplug framework

2013-01-14 Thread Toshi Kani
On Fri, 2013-01-11 at 22:23 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 10, 2013 04:40:19 PM Toshi Kani wrote:
> > Added include/linux/sys_hotplug.h, which defines the system device
> > hotplug framework interfaces used by the framework itself and
> > handlers.
> > 
> > The order values define the calling sequence of handlers.  For add
> > execute, the ordering is ACPI->MEM->CPU.  Memory is onlined before
> > CPU so that threads on new CPUs can start using their local memory.
> > The ordering of the delete execute is symmetric to the add execute.
> > 
> > struct shp_request defines a hot-plug request information.  The
> > device resource information is managed with a list so that a single
> > request may target to multiple devices.
> > 
 :
> > +
> > +struct shp_device {
> > +   struct list_headlist;
> > +   struct device   *device;
> > +   enum shp_class  class;
> > +   union shp_dev_info  info;
> > +};
> > +
> > +/*
> > + * Hot-plug request
> > + */
> > +struct shp_request {
> > +   /* common info */
> > +   enum shp_operation  operation;  /* operation */
> > +
> > +   /* hot-plug event info: only valid for hot-plug operations */
> > +   void*handle;/* FW handle */
> 
> What's the role of handle here?

On ACPI-based platforms, the handle keeps a notified ACPI handle when a
hot-plug request is made.  ACPI bus handlers, acpi_add_execute() /
acpi_del_execute(), then scans / trims ACPI devices from the handle.

Thanks,
-Toshi

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v4 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

2013-01-14 Thread Peter Chen
As we use platform_device_id for fsl-usb2-udc driver, it needs to
change clk connection-id, or the related devm_clk_get will be failed.

Signed-off-by: Peter Chen 
---
 arch/arm/mach-imx/clk-imx25.c   |6 +++---
 arch/arm/mach-imx/clk-imx27.c   |6 +++---
 arch/arm/mach-imx/clk-imx31.c   |6 +++---
 arch/arm/mach-imx/clk-imx35.c   |6 +++---
 arch/arm/mach-imx/clk-imx51-imx53.c |6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index b197aa7..67e353d 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -254,9 +254,9 @@ int __init mx25_clocks_init(void)
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
clk_register_clkdev(clk[usbotg_ahb], "ahb", "mxc-ehci.2");
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usbotg_ahb], "ahb", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx25");
+   clk_register_clkdev(clk[usbotg_ahb], "ahb", "imx-udc-mx25");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx25");
clk_register_clkdev(clk[nfc_ipg_per], NULL, "imx25-nand.0");
/* i.mx25 has the i.mx35 type cspi */
clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index 4c1d1e4..1ffe3b5 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref)
clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx21-fb.0");
clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0");
clk_register_clkdev(clk[per4_gate], "per", "imx27-camera.0");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+   clk_register_clkdev(clk[usb_ipg_gate], "ipg", "imx-udc-mx27");
+   clk_register_clkdev(clk[usb_ahb_gate], "ahb", "imx-udc-mx27");
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
clk_register_clkdev(clk[usb_ipg_gate], "ipg", "mxc-ehci.0");
clk_register_clkdev(clk[usb_ahb_gate], "ahb", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c
index 8be64e0..ef66eaf 100644
--- a/arch/arm/mach-imx/clk-imx31.c
+++ b/arch/arm/mach-imx/clk-imx31.c
@@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref)
clk_register_clkdev(clk[usb_div_post], "per", "mxc-ehci.2");
clk_register_clkdev(clk[usb_gate], "ahb", "mxc-ehci.2");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
-   clk_register_clkdev(clk[usb_div_post], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_gate], "ahb", "fsl-usb2-udc");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div_post], "per", "imx-udc-mx31");
+   clk_register_clkdev(clk[usb_gate], "ahb", "imx-udc-mx31");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx31");
clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
/* i.mx31 has the i.mx21 type uart */
clk_register_clkdev(clk[uart1_gate], "per", "imx21-uart.0");
diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index 66f3d65..69fe9c8 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -251,9 +251,9 @@ int __init mx35_clocks_init()
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.2");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usbotg_gate], "ahb", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx35");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx35");
+   clk_register_clkdev(clk[usbotg_gate], "ahb", "imx-udc-mx35");
clk_register_clkdev(clk[wdog_gate], NULL, "imx2-wdt.0");
clk_register_clkdev(clk[nfc_div], NULL, "imx25-nand.0");
clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c 
b/arch/arm/mach-imx/clk-imx51-imx53.c
index 579023f..fb7cb84 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long 
rate_ckil,
clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.2");
clk_register_clkdev(clk[usboh3_gate], "ipg", "mx

[PATCH v4 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Peter Chen
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.

Signed-off-by: Peter Chen 
---
 drivers/usb/gadget/fsl_mxc_udc.c  |   27 +--
 drivers/usb/gadget/fsl_udc_core.c |4 +++-
 drivers/usb/gadget/fsl_usb2_udc.h |4 ++--
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..e505d60 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
 
 /* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET  0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET  0x8
 #define USBPHYCTRL_EVDO(1 << 23)
 
 int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -77,28 +78,42 @@ eclkrate:
return ret;
 }
 
-void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
+   int ret = 0;
+
if (devtype == IMX35_UDC) {
unsigned int v;
+   struct resource *res = platform_get_resource
+   (pdev, IORESOURCE_MEM, 0);
+   void __iomem *phy_regs = ioremap(res->start +
+   MX35_USBPHYCTRL_OFFSET, 512);
+   if (!phy_regs) {
+   dev_err(&pdev->dev, "ioremap for phy address fails\n");
+   ret = -EINVAL;
+   goto ioremap_err;
+   }
 
/* workaround ENGcm09152 for i.MX35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-   v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-   USBPHYCTRL_OTGBASE_OFFSET));
+   v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
writel(v | USBPHYCTRL_EVDO,
-   MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-   USBPHYCTRL_OTGBASE_OFFSET));
+   phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
}
+
+   iounmap(phy_regs);
}
 
+ioremap_err:
/* ULPI transceivers don't need usbpll */
if (pdata->phy_mode == FSL_USB2_PHY_ULPI) {
clk_disable_unprepare(mxc_per_clk);
mxc_per_clk = NULL;
}
+
+   return ret;
 }
 
 void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index c32119b..4391d49 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2544,7 +2544,9 @@ static int __init fsl_udc_probe(struct platform_device 
*pdev)
dr_controller_setup(udc_controller);
}
 
-   fsl_udc_clk_finalize(udc_controller->devtype, pdev);
+   ret = fsl_udc_clk_finalize(udc_controller->devtype, pdev);
+   if (ret)
+   goto err_free_irq;
 
/* Setup gadget structure */
udc_controller->gadget.ops = &fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h 
b/drivers/usb/gadget/fsl_usb2_udc.h
index bc1f6d0..7ead5f7 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -594,7 +594,7 @@ static inline struct ep_queue_head *get_qh_by_ep(struct 
fsl_ep *ep)
 struct platform_device;
 #ifdef CONFIG_ARCH_MXC
 int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev);
-void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev);
 void fsl_udc_clk_release(void);
 #else
@@ -603,7 +603,7 @@ static inline int fsl_udc_clk_init(enum fsl_udc_type 
devtype,
 {
return 0;
 }
-static inline void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+static inline int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev)
 {
 }
-- 
1.7.0.4


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v4 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Peter Chen
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs.

Besides we update the platform code accordingly.

Signed-off-by: Peter Chen 
---
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
 drivers/usb/gadget/fsl_mxc_udc.c  |   11 ++--
 drivers/usb/gadget/fsl_udc_core.c |   52 +---
 drivers/usb/gadget/fsl_usb2_udc.h |   13 --
 include/linux/fsl_devices.h   |8 +++
 6 files changed, 65 insertions(+), 35 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h 
b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
 
 #include 
 struct imx_fsl_usb2_udc_data {
+   const char *devid;
resource_size_t iobase;
resource_size_t irq;
 };
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c 
b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..fb527c7 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_fsl_usb2_udc_data_entry_single(soc)
\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)
\
{   \
+   .devid = _devid,\
.iobase = soc ## _USB_OTG_BASE_ADDR,\
.irq = soc ## _INT_USB_OTG, \
}
 
 #ifdef CONFIG_SOC_IMX25
 const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX25);
+   imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx25");
 #endif /* ifdef CONFIG_SOC_IMX25 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX27);
+   imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX31);
+   imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx31");
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX35);
+   imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx35");
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 #ifdef CONFIG_SOC_IMX51
 const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX51);
+   imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
 #endif
 
 struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
.flags = IORESOURCE_IRQ,
},
};
-   return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
+   return imx_add_platform_device_dmamask(data->devid, -1,
res, ARRAY_SIZE(res),
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
 }
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..6df45f7 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
 #include 
 #include 
 
-#include 
-
 static struct clk *mxc_ahb_clk;
 static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
@@ -28,7 +26,7 @@ static struct clk *mxc_ipg_clk;
 #define USBPHYCTRL_OTGBASE_OFFSET  0x608
 #define USBPHYCTRL_EVDO(1 << 23)
 
-int fsl_udc_clk_init(struct platform_device *pdev)
+int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata;
unsigned long freq;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
clk_prepare_enable(mxc_per_clk);
 
/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-   if (!cpu_is_mx51()) {
+   if (!(devtype == IMX51_UDC)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
(freq < 5000 || freq > 60001000)) {
@@ -79,10 +77,11 @@ eclkrate:
return ret;
 }
 
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+   struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-   if (cpu_is_mx35()) {
+   if (devtyp

[PATCH v4 0/3] Fix the Build error for fsl_mxc_udc.c

2013-01-14 Thread Peter Chen
Changes for v4:
- Using pdev's struct resource to do ioremap
- Add ioremap return value check

Changes for v3:
- Split the one big patch into three patches

Changes for v2:
- Add const for fsl_udc_devtype
- Do ioremap for phy address at fsl-mxc-udc

Peter Chen (3):
  usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

 arch/arm/mach-imx/clk-imx25.c |6 +-
 arch/arm/mach-imx/clk-imx27.c |6 +-
 arch/arm/mach-imx/clk-imx31.c |6 +-
 arch/arm/mach-imx/clk-imx35.c |6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
 drivers/usb/gadget/fsl_mxc_udc.c  |   36 ++
 drivers/usb/gadget/fsl_udc_core.c |   54 ++---
 drivers/usb/gadget/fsl_usb2_udc.h |   13 --
 include/linux/fsl_devices.h   |8 +++
 11 files changed, 102 insertions(+), 55 deletions(-)


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Peter Chen
On Mon, Jan 14, 2013 at 01:10:56PM +, Russell King - ARM Linux wrote:
> On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> > @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
> > struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> > if (devtype == IMX35_UDC) {
> > unsigned int v;
> > +   void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> > +   MX35_USBPHYCTRL_OFFSET, 512);
> 
> Consider that ioremap() can fail.
> 
Thanks, will check NULL pointer.

-- 

Best Regards,
Peter Chen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Russell King - ARM Linux
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
>   struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
>   if (devtype == IMX35_UDC) {
>   unsigned int v;
> + void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> + MX35_USBPHYCTRL_OFFSET, 512);

Consider that ioremap() can fail.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Peter Chen
On Mon, Jan 14, 2013 at 12:17:08PM +0200, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> > As mach/hardware.h is deleted, we can't visit platform code at driver.
> > It has no phy driver to combine with this controller, so it has to use
> > ioremap to map phy address as a workaround.
> > 
> > Signed-off-by: Peter Chen 
> > ---
> >  drivers/usb/gadget/fsl_mxc_udc.c |   12 +++-
> >  1 files changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/fsl_mxc_udc.c 
> > b/drivers/usb/gadget/fsl_mxc_udc.c
> > index 6df45f7..0e858e6 100644
> > --- a/drivers/usb/gadget/fsl_mxc_udc.c
> > +++ b/drivers/usb/gadget/fsl_mxc_udc.c
> > @@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
> >  static struct clk *mxc_ipg_clk;
> >  
> >  /* workaround ENGcm09152 for i.MX35 */
> > -#define USBPHYCTRL_OTGBASE_OFFSET  0x608
> > +#define MX35_USBPHYCTRL_OFFSET 0x600
> > +#define USBPHYCTRL_OTGBASE_OFFSET  0x8
> >  #define USBPHYCTRL_EVDO(1 << 23)
> >  
> >  int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device 
> > *pdev)
> > @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
> > struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> > if (devtype == IMX35_UDC) {
> > unsigned int v;
> > +   void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> > +   MX35_USBPHYCTRL_OFFSET, 512);
> 
> as I said before, this should be passed via struct resource, not pdata.
My careless, will change at next version
> 
> -- 
> balbi



-- 

Best Regards,
Peter Chen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Peter Chen
On Mon, Jan 14, 2013 at 01:06:00PM +0200, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:
> > On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
> > >> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> > >>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
> >  On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> > > On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> > >> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> > >>> Hi,
> > >>>
> > >>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >  @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct 
> >  device *dev)
> >   
> > return fsl_udc_resume(NULL);
> >   }
> >  -
> >   
> >  /*-
> > Register entry point for the peripheral controller driver
> >   
> >  --*/
> >  -
> >  +static const struct platform_device_id fsl_udc_devtype[] = {
> >  +  {
> >  +  .name = "imx-udc-mx25",
> >  +  .driver_data = IMX25_UDC,
> >  +  }, {
> >  +  .name = "imx-udc-mx27",
> >  +  .driver_data = IMX27_UDC,
> >  +  }, {
> >  +  .name = "imx-udc-mx31",
> >  +  .driver_data = IMX31_UDC,
> >  +  }, {
> >  +  .name = "imx-udc-mx35",
> >  +  .driver_data = IMX35_UDC,
> >  +  }, {
> >  +  .name = "imx-udc-mx51",
> >  +  .driver_data = IMX51_UDC,
> >  +  }
> >  +};
> > >>>
> > >>> I wonder if your driver-data is actually needed since you can use 
> > >>> string
> > >>> comparisson to achieve the exact same outcome.
> > >>
> > >> Why use a string compare, if the kernel infrastructure already does 
> > >> this
> > >> for you?
> > >
> > > what do you mean ? What kernel infrastructure is doing waht for me ?
> > 
> >  The kernel infrastructure is doing the string compare for you to match
> >  the device against the driver (via platform_device_id->name). You get
> >  the a pointer to the driver_data for free. So you don't need any string
> >  compare in the driver later.
> > >>>
> > >>> but current driver data is just duplicating name with an integer, it's
> > >>> pretty useless driver data.
> > >>
> > >> I don't think so - another argument:
> > >> Less code. As struct platform_device_id is a static array the space is
> > >> allocated anyway. So it doesn't make any difference if driver_data is
> > >> NULL or not. Later you just need to make an integer comparison instead
> > >> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
> > >> enum, the compiler will warn you if you've missed an IMX variant.
> > > 
> > > fair enough, but then don't create a different enum value for each imx
> > > instance if they're mostly the same. Differentiate only what's actually
> > > different.
> > 
> > Usually there isn't any Changelog between IP cores used in the different
> > fsl processors (at least available outside of fsl), that makes it quite
> > difficult to say if something found on one imx is really the same as on
> > the other one. And they (usually) don't provide any versioning
> > information in a register or the documentation.
> > 
> > just my 2¢
> 
> $SUBJECT is trying to differentiate a single feature (or maybe two) to
> replace cpu_is_xxx(), then expose that on driver_data without creating
> one enum value for each release from fsl.

Felipe, every one or two SoCs may have their special operations for
integrate PHY interface, clk operation, or workaround for IC limitation.

Maybe, it will add more future or SoCs (maybe not for this driver)
in the future, using enum is easier than string comparison for expanding 
something.
> 
> -- 
> balbi



-- 

Best Regards,
Peter Chen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Marc Kleine-Budde
On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
>>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
 On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
 @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device 
 *dev)
  
return fsl_udc_resume(NULL);
  }
 -
  
 /*-
Register entry point for the peripheral controller driver
  
 --*/
 -
 +static const struct platform_device_id fsl_udc_devtype[] = {
 +  {
 +  .name = "imx-udc-mx25",
 +  .driver_data = IMX25_UDC,
 +  }, {
 +  .name = "imx-udc-mx27",
 +  .driver_data = IMX27_UDC,
 +  }, {
 +  .name = "imx-udc-mx31",
 +  .driver_data = IMX31_UDC,
 +  }, {
 +  .name = "imx-udc-mx35",
 +  .driver_data = IMX35_UDC,
 +  }, {
 +  .name = "imx-udc-mx51",
 +  .driver_data = IMX51_UDC,
 +  }
 +};
>>>
>>> I wonder if your driver-data is actually needed since you can use string
>>> comparisson to achieve the exact same outcome.
>>
>> Why use a string compare, if the kernel infrastructure already does this
>> for you?
>
> what do you mean ? What kernel infrastructure is doing waht for me ?

 The kernel infrastructure is doing the string compare for you to match
 the device against the driver (via platform_device_id->name). You get
 the a pointer to the driver_data for free. So you don't need any string
 compare in the driver later.
>>>
>>> but current driver data is just duplicating name with an integer, it's
>>> pretty useless driver data.
>>
>> I don't think so - another argument:
>> Less code. As struct platform_device_id is a static array the space is
>> allocated anyway. So it doesn't make any difference if driver_data is
>> NULL or not. Later you just need to make an integer comparison instead
>> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
>> enum, the compiler will warn you if you've missed an IMX variant.
> 
> fair enough, but then don't create a different enum value for each imx
> instance if they're mostly the same. Differentiate only what's actually
> different.

Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.

just my 2¢
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Marc Kleine-Budde
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
 On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>>  
>>  return fsl_udc_resume(NULL);
>>  }
>> -
>>  
>> /*-
>>  Register entry point for the peripheral controller driver
>>  
>> --*/
>> -
>> +static const struct platform_device_id fsl_udc_devtype[] = {
>> +{
>> +.name = "imx-udc-mx25",
>> +.driver_data = IMX25_UDC,
>> +}, {
>> +.name = "imx-udc-mx27",
>> +.driver_data = IMX27_UDC,
>> +}, {
>> +.name = "imx-udc-mx31",
>> +.driver_data = IMX31_UDC,
>> +}, {
>> +.name = "imx-udc-mx35",
>> +.driver_data = IMX35_UDC,
>> +}, {
>> +.name = "imx-udc-mx51",
>> +.driver_data = IMX51_UDC,
>> +}
>> +};
>
> I wonder if your driver-data is actually needed since you can use string
> comparisson to achieve the exact same outcome.

 Why use a string compare, if the kernel infrastructure already does this
 for you?
>>>
>>> what do you mean ? What kernel infrastructure is doing waht for me ?
>>
>> The kernel infrastructure is doing the string compare for you to match
>> the device against the driver (via platform_device_id->name). You get
>> the a pointer to the driver_data for free. So you don't need any string
>> compare in the driver later.
> 
> but current driver data is just duplicating name with an integer, it's
> pretty useless driver data.

I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Marc Kleine-Budde
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
 @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
  
return fsl_udc_resume(NULL);
  }
 -
  
 /*-
Register entry point for the peripheral controller driver
  
 --*/
 -
 +static const struct platform_device_id fsl_udc_devtype[] = {
 +  {
 +  .name = "imx-udc-mx25",
 +  .driver_data = IMX25_UDC,
 +  }, {
 +  .name = "imx-udc-mx27",
 +  .driver_data = IMX27_UDC,
 +  }, {
 +  .name = "imx-udc-mx31",
 +  .driver_data = IMX31_UDC,
 +  }, {
 +  .name = "imx-udc-mx35",
 +  .driver_data = IMX35_UDC,
 +  }, {
 +  .name = "imx-udc-mx51",
 +  .driver_data = IMX51_UDC,
 +  }
 +};
>>>
>>> I wonder if your driver-data is actually needed since you can use string
>>> comparisson to achieve the exact same outcome.
>>
>> Why use a string compare, if the kernel infrastructure already does this
>> for you?
> 
> what do you mean ? What kernel infrastructure is doing waht for me ?

The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id->name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Marc Kleine-Budde
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>>  
>>  return fsl_udc_resume(NULL);
>>  }
>> -
>>  /*-
>>  Register entry point for the peripheral controller driver
>>  --*/
>> -
>> +static const struct platform_device_id fsl_udc_devtype[] = {
>> +{
>> +.name = "imx-udc-mx25",
>> +.driver_data = IMX25_UDC,
>> +}, {
>> +.name = "imx-udc-mx27",
>> +.driver_data = IMX27_UDC,
>> +}, {
>> +.name = "imx-udc-mx31",
>> +.driver_data = IMX31_UDC,
>> +}, {
>> +.name = "imx-udc-mx35",
>> +.driver_data = IMX35_UDC,
>> +}, {
>> +.name = "imx-udc-mx51",
>> +.driver_data = IMX51_UDC,
>> +}
>> +};
> 
> I wonder if your driver-data is actually needed since you can use string
> comparisson to achieve the exact same outcome.

Why use a string compare, if the kernel infrastructure already does this
for you?

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[linuxppc-dev][PATCH] powerpc/fsl_pci: Store the pci controller device pointer in the pci controller structure.

2013-01-14 Thread Varun Sethi
The pci controller structure has a provision to store the device strcuture
pointer of the corresponding platform device. Currently this information is
not stored during fsl pci controller initialization. This information is
required while dealing with iommu groups for pci devices connected to the fsl
pci controller. For the case where the pci devices can't be paritioned, they 
would fall under the same device group as the pci controller.

This patch stores the platform device information in the pci controller
structure during initialization.

Signed-off-by: Varun Sethi 
---
 arch/powerpc/sysdev/fsl_pci.c |9 +++--
 arch/powerpc/sysdev/fsl_pci.h |2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 92a5915..b393ae7 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -421,13 +421,16 @@ void fsl_pcibios_fixup_bus(struct pci_bus *bus)
}
 }
 
-int __init fsl_add_bridge(struct device_node *dev, int is_primary)
+int __init fsl_add_bridge(struct platform_device *pdev, int is_primary)
 {
int len;
struct pci_controller *hose;
struct resource rsrc;
const int *bus_range;
u8 hdr_type, progif;
+   struct device_node *dev;
+
+   dev = pdev->dev.of_node;
 
if (!of_device_is_available(dev)) {
pr_warning("%s: disabled\n", dev->full_name);
@@ -453,6 +456,8 @@ int __init fsl_add_bridge(struct device_node *dev, int 
is_primary)
if (!hose)
return -ENOMEM;
 
+   /* set platform device as the parent */
+   hose->parent = &pdev->dev;
hose->first_busno = bus_range ? bus_range[0] : 0x0;
hose->last_busno = bus_range ? bus_range[1] : 0xff;
 
@@ -880,7 +885,7 @@ static int fsl_pci_probe(struct platform_device *pdev)
 #endif
 
node = pdev->dev.of_node;
-   ret = fsl_add_bridge(node, fsl_pci_primary == node);
+   ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
 
 #ifdef CONFIG_SWIOTLB
if (ret == 0) {
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index d078537..c495c00 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -91,7 +91,7 @@ struct ccsr_pci {
__be32  pex_err_cap_r3; /* 0x.e34 - PCIE error capture register 
0 */
 };
 
-extern int fsl_add_bridge(struct device_node *dev, int is_primary);
+extern int fsl_add_bridge(struct platform_device *pdev, int is_primary);
 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.4.1


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [Solved] Interrupt handler not executed

2013-01-14 Thread R . Patil
Hello Scott,

Thanks for your immediate response. The problem has been resolved. Actually, I 
was wrong in pointing out the problem. 

The problem was lying in interrupt handler. The first statement in interrupt 
handler was disable_irq() which was leading into a deadlock. I replaced 
'disable_irq' with 'disable_irq_nosync()' and it worked.  

Thanks and Regards,
Rahul Patil

-Scott Wood  wrote: -

To: 
From: Scott Wood 
Date: 01/11/2013 11:46PM
Cc: 
Subject: Re: Interrupt handler not executed

On 01/11/2013 01:36:29 AM, r.pa...@mei-india.com wrote:
> Hello,
> 
> We are working on board based on Freescale MPC8313ERDB. We have  
> ported linux 3.0.46 kernel on it. In one of device driver written by  
> us, we need to take some action upon asserting IRQ0 interrupt. For  
> this we have written interrupt handler which takes care of this. We  
> are able register interrupt handler successfully with the help of  
> 'request_irq'. We confirmed this by checking respective entry in  
> '/proc/interrupts'. We have also confirmed assertion of interrupt  
> line (IRQ0) on oscilloscope. The problem is, interrupt handler does  
> not execute upon asserting the interrupt line.

What IRQ number did you pass to request_irq()?  request_irq() takes  
virtual interrupt numbers, not anything out of the chip manual.

If that's not the issue, is the interrupt configured properly for level  
and sense?

-Scott

Email Disclaimer:
  ---

This e-mail and any files transmitted with it are for the sole use of the 
intended recipient(s) and may contain confidential and  privileged information. 
Computer viruses can be transmitted via email.The recipient should check this 
email and any  attachments for the presence of viruses. The company accepts no 
liability for any damage caused by any virus transmitted by this email.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> >>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
>  On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> > On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> >>> Hi,
> >>>
> >>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>  @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device 
>  *dev)
>   
>   return fsl_udc_resume(NULL);
>   }
>  -
>   
>  /*-
>   Register entry point for the peripheral controller driver
>   
>  --*/
>  -
>  +static const struct platform_device_id fsl_udc_devtype[] = {
>  +{
>  +.name = "imx-udc-mx25",
>  +.driver_data = IMX25_UDC,
>  +}, {
>  +.name = "imx-udc-mx27",
>  +.driver_data = IMX27_UDC,
>  +}, {
>  +.name = "imx-udc-mx31",
>  +.driver_data = IMX31_UDC,
>  +}, {
>  +.name = "imx-udc-mx35",
>  +.driver_data = IMX35_UDC,
>  +}, {
>  +.name = "imx-udc-mx51",
>  +.driver_data = IMX51_UDC,
>  +}
>  +};
> >>>
> >>> I wonder if your driver-data is actually needed since you can use 
> >>> string
> >>> comparisson to achieve the exact same outcome.
> >>
> >> Why use a string compare, if the kernel infrastructure already does 
> >> this
> >> for you?
> >
> > what do you mean ? What kernel infrastructure is doing waht for me ?
> 
>  The kernel infrastructure is doing the string compare for you to match
>  the device against the driver (via platform_device_id->name). You get
>  the a pointer to the driver_data for free. So you don't need any string
>  compare in the driver later.
> >>>
> >>> but current driver data is just duplicating name with an integer, it's
> >>> pretty useless driver data.
> >>
> >> I don't think so - another argument:
> >> Less code. As struct platform_device_id is a static array the space is
> >> allocated anyway. So it doesn't make any difference if driver_data is
> >> NULL or not. Later you just need to make an integer comparison instead
> >> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
> >> enum, the compiler will warn you if you've missed an IMX variant.
> > 
> > fair enough, but then don't create a different enum value for each imx
> > instance if they're mostly the same. Differentiate only what's actually
> > different.
> 
> Usually there isn't any Changelog between IP cores used in the different
> fsl processors (at least available outside of fsl), that makes it quite
> difficult to say if something found on one imx is really the same as on
> the other one. And they (usually) don't provide any versioning
> information in a register or the documentation.
> 
> just my 2¢

$SUBJECT is trying to differentiate a single feature (or maybe two) to
replace cpu_is_xxx(), then expose that on driver_data without creating
one enum value for each release from fsl.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
Hi,

On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> > On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> >>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>  On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> > Hi,
> >
> > On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device 
> >> *dev)
> >>  
> >>return fsl_udc_resume(NULL);
> >>  }
> >> -
> >>  
> >> /*-
> >>Register entry point for the peripheral controller driver
> >>  
> >> --*/
> >> -
> >> +static const struct platform_device_id fsl_udc_devtype[] = {
> >> +  {
> >> +  .name = "imx-udc-mx25",
> >> +  .driver_data = IMX25_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx27",
> >> +  .driver_data = IMX27_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx31",
> >> +  .driver_data = IMX31_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx35",
> >> +  .driver_data = IMX35_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx51",
> >> +  .driver_data = IMX51_UDC,
> >> +  }
> >> +};
> >
> > I wonder if your driver-data is actually needed since you can use string
> > comparisson to achieve the exact same outcome.
> 
>  Why use a string compare, if the kernel infrastructure already does this
>  for you?
> >>>
> >>> what do you mean ? What kernel infrastructure is doing waht for me ?
> >>
> >> The kernel infrastructure is doing the string compare for you to match
> >> the device against the driver (via platform_device_id->name). You get
> >> the a pointer to the driver_data for free. So you don't need any string
> >> compare in the driver later.
> > 
> > but current driver data is just duplicating name with an integer, it's
> > pretty useless driver data.
> 
> I don't think so - another argument:
> Less code. As struct platform_device_id is a static array the space is
> allocated anyway. So it doesn't make any difference if driver_data is
> NULL or not. Later you just need to make an integer comparison instead
> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
> enum, the compiler will warn you if you've missed an IMX variant.

fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> > On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> >>> Hi,
> >>>
> >>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>  @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>   
>   return fsl_udc_resume(NULL);
>   }
>  -
>   
>  /*-
>   Register entry point for the peripheral controller driver
>   
>  --*/
>  -
>  +static const struct platform_device_id fsl_udc_devtype[] = {
>  +{
>  +.name = "imx-udc-mx25",
>  +.driver_data = IMX25_UDC,
>  +}, {
>  +.name = "imx-udc-mx27",
>  +.driver_data = IMX27_UDC,
>  +}, {
>  +.name = "imx-udc-mx31",
>  +.driver_data = IMX31_UDC,
>  +}, {
>  +.name = "imx-udc-mx35",
>  +.driver_data = IMX35_UDC,
>  +}, {
>  +.name = "imx-udc-mx51",
>  +.driver_data = IMX51_UDC,
>  +}
>  +};
> >>>
> >>> I wonder if your driver-data is actually needed since you can use string
> >>> comparisson to achieve the exact same outcome.
> >>
> >> Why use a string compare, if the kernel infrastructure already does this
> >> for you?
> > 
> > what do you mean ? What kernel infrastructure is doing waht for me ?
> 
> The kernel infrastructure is doing the string compare for you to match
> the device against the driver (via platform_device_id->name). You get
> the a pointer to the driver_data for free. So you don't need any string
> compare in the driver later.

but current driver data is just duplicating name with an integer, it's
pretty useless driver data.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
> >>  
> >>return fsl_udc_resume(NULL);
> >>  }
> >> -
> >>  
> >> /*-
> >>Register entry point for the peripheral controller driver
> >>  
> >> --*/
> >> -
> >> +static const struct platform_device_id fsl_udc_devtype[] = {
> >> +  {
> >> +  .name = "imx-udc-mx25",
> >> +  .driver_data = IMX25_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx27",
> >> +  .driver_data = IMX27_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx31",
> >> +  .driver_data = IMX31_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx35",
> >> +  .driver_data = IMX35_UDC,
> >> +  }, {
> >> +  .name = "imx-udc-mx51",
> >> +  .driver_data = IMX51_UDC,
> >> +  }
> >> +};
> > 
> > I wonder if your driver-data is actually needed since you can use string
> > comparisson to achieve the exact same outcome.
> 
> Why use a string compare, if the kernel infrastructure already does this
> for you?

what do you mean ? What kernel infrastructure is doing waht for me ?

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Felipe Balbi
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> As mach/hardware.h is deleted, we can't visit platform code at driver.
> It has no phy driver to combine with this controller, so it has to use
> ioremap to map phy address as a workaround.
> 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/gadget/fsl_mxc_udc.c |   12 +++-
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/fsl_mxc_udc.c 
> b/drivers/usb/gadget/fsl_mxc_udc.c
> index 6df45f7..0e858e6 100644
> --- a/drivers/usb/gadget/fsl_mxc_udc.c
> +++ b/drivers/usb/gadget/fsl_mxc_udc.c
> @@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
>  static struct clk *mxc_ipg_clk;
>  
>  /* workaround ENGcm09152 for i.MX35 */
> -#define USBPHYCTRL_OTGBASE_OFFSET0x608
> +#define MX35_USBPHYCTRL_OFFSET   0x600
> +#define USBPHYCTRL_OTGBASE_OFFSET0x8
>  #define USBPHYCTRL_EVDO  (1 << 23)
>  
>  int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
> @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
>   struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
>   if (devtype == IMX35_UDC) {
>   unsigned int v;
> + void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> + MX35_USBPHYCTRL_OFFSET, 512);

as I said before, this should be passed via struct resource, not pdata.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Felipe Balbi
Hi,

On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
>  
>   return fsl_udc_resume(NULL);
>  }
> -
>  /*-
>   Register entry point for the peripheral controller driver
>  --*/
> -
> +static const struct platform_device_id fsl_udc_devtype[] = {
> + {
> + .name = "imx-udc-mx25",
> + .driver_data = IMX25_UDC,
> + }, {
> + .name = "imx-udc-mx27",
> + .driver_data = IMX27_UDC,
> + }, {
> + .name = "imx-udc-mx31",
> + .driver_data = IMX31_UDC,
> + }, {
> + .name = "imx-udc-mx35",
> + .driver_data = IMX35_UDC,
> + }, {
> + .name = "imx-udc-mx51",
> + .driver_data = IMX51_UDC,
> + }
> +};

I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v3 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

2013-01-14 Thread Peter Chen
As we use platform_device_id for fsl-usb2-udc driver, it needs to
change clk connection-id, or the related devm_clk_get will be failed.

Signed-off-by: Peter Chen 
---
 arch/arm/mach-imx/clk-imx25.c   |6 +++---
 arch/arm/mach-imx/clk-imx27.c   |6 +++---
 arch/arm/mach-imx/clk-imx31.c   |6 +++---
 arch/arm/mach-imx/clk-imx35.c   |6 +++---
 arch/arm/mach-imx/clk-imx51-imx53.c |6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index b197aa7..67e353d 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -254,9 +254,9 @@ int __init mx25_clocks_init(void)
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
clk_register_clkdev(clk[usbotg_ahb], "ahb", "mxc-ehci.2");
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usbotg_ahb], "ahb", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx25");
+   clk_register_clkdev(clk[usbotg_ahb], "ahb", "imx-udc-mx25");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx25");
clk_register_clkdev(clk[nfc_ipg_per], NULL, "imx25-nand.0");
/* i.mx25 has the i.mx35 type cspi */
clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index 4c1d1e4..1ffe3b5 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref)
clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx21-fb.0");
clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0");
clk_register_clkdev(clk[per4_gate], "per", "imx27-camera.0");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+   clk_register_clkdev(clk[usb_ipg_gate], "ipg", "imx-udc-mx27");
+   clk_register_clkdev(clk[usb_ahb_gate], "ahb", "imx-udc-mx27");
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
clk_register_clkdev(clk[usb_ipg_gate], "ipg", "mxc-ehci.0");
clk_register_clkdev(clk[usb_ahb_gate], "ahb", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c
index 8be64e0..ef66eaf 100644
--- a/arch/arm/mach-imx/clk-imx31.c
+++ b/arch/arm/mach-imx/clk-imx31.c
@@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref)
clk_register_clkdev(clk[usb_div_post], "per", "mxc-ehci.2");
clk_register_clkdev(clk[usb_gate], "ahb", "mxc-ehci.2");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
-   clk_register_clkdev(clk[usb_div_post], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usb_gate], "ahb", "fsl-usb2-udc");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div_post], "per", "imx-udc-mx31");
+   clk_register_clkdev(clk[usb_gate], "ahb", "imx-udc-mx31");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx31");
clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
/* i.mx31 has the i.mx21 type uart */
clk_register_clkdev(clk[uart1_gate], "per", "imx21-uart.0");
diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index 66f3d65..69fe9c8 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -251,9 +251,9 @@ int __init mx35_clocks_init()
clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.2");
-   clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-   clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-   clk_register_clkdev(clk[usbotg_gate], "ahb", "fsl-usb2-udc");
+   clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx35");
+   clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx35");
+   clk_register_clkdev(clk[usbotg_gate], "ahb", "imx-udc-mx35");
clk_register_clkdev(clk[wdog_gate], NULL, "imx2-wdt.0");
clk_register_clkdev(clk[nfc_div], NULL, "imx25-nand.0");
clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c 
b/arch/arm/mach-imx/clk-imx51-imx53.c
index 579023f..fb7cb84 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long 
rate_ckil,
clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.2");
clk_register_clkdev(clk[usboh3_gate], "ipg", "mx

[PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap

2013-01-14 Thread Peter Chen
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.

Signed-off-by: Peter Chen 
---
 drivers/usb/gadget/fsl_mxc_udc.c |   12 +++-
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..0e858e6 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
 
 /* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET  0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET  0x8
 #define USBPHYCTRL_EVDO(1 << 23)
 
 int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+   void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
+   MX35_USBPHYCTRL_OFFSET, 512);
 
/* workaround ENGcm09152 for i.MX35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-   v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-   USBPHYCTRL_OTGBASE_OFFSET));
+   v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
writel(v | USBPHYCTRL_EVDO,
-   MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-   USBPHYCTRL_OTGBASE_OFFSET));
+   phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
}
+   iounmap(phy_regs);
}
 
/* ULPI transceivers don't need usbpll */
-- 
1.7.0.4


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id

2013-01-14 Thread Peter Chen
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs.

Besides we update the platform code accordingly.

Signed-off-by: Peter Chen 
---
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
 drivers/usb/gadget/fsl_mxc_udc.c  |   11 ++--
 drivers/usb/gadget/fsl_udc_core.c |   52 +---
 drivers/usb/gadget/fsl_usb2_udc.h |   13 --
 include/linux/fsl_devices.h   |8 +++
 6 files changed, 65 insertions(+), 35 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h 
b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
 
 #include 
 struct imx_fsl_usb2_udc_data {
+   const char *devid;
resource_size_t iobase;
resource_size_t irq;
 };
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c 
b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..fb527c7 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_fsl_usb2_udc_data_entry_single(soc)
\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)
\
{   \
+   .devid = _devid,\
.iobase = soc ## _USB_OTG_BASE_ADDR,\
.irq = soc ## _INT_USB_OTG, \
}
 
 #ifdef CONFIG_SOC_IMX25
 const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX25);
+   imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx25");
 #endif /* ifdef CONFIG_SOC_IMX25 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX27);
+   imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX31);
+   imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx31");
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX35);
+   imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx35");
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 #ifdef CONFIG_SOC_IMX51
 const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
-   imx_fsl_usb2_udc_data_entry_single(MX51);
+   imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
 #endif
 
 struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
.flags = IORESOURCE_IRQ,
},
};
-   return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
+   return imx_add_platform_device_dmamask(data->devid, -1,
res, ARRAY_SIZE(res),
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
 }
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..6df45f7 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
 #include 
 #include 
 
-#include 
-
 static struct clk *mxc_ahb_clk;
 static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
@@ -28,7 +26,7 @@ static struct clk *mxc_ipg_clk;
 #define USBPHYCTRL_OTGBASE_OFFSET  0x608
 #define USBPHYCTRL_EVDO(1 << 23)
 
-int fsl_udc_clk_init(struct platform_device *pdev)
+int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata;
unsigned long freq;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
clk_prepare_enable(mxc_per_clk);
 
/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-   if (!cpu_is_mx51()) {
+   if (!(devtype == IMX51_UDC)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
(freq < 5000 || freq > 60001000)) {
@@ -79,10 +77,11 @@ eclkrate:
return ret;
 }
 
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+   struct platform_device *pdev)
 {
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-   if (cpu_is_mx35()) {
+   if (devtyp

[PATCH v3 0/3] Fix the Build error for fsl_mxc_udc.c

2013-01-14 Thread Peter Chen
Changes for v3:
- Split the one big patch into three patches

Changes for v2:
- Add const for fsl_udc_devtype
- Do ioremap for phy address at fsl-mxc-udc

Peter Chen (3):
  usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

 arch/arm/mach-imx/clk-imx25.c |6 +-
 arch/arm/mach-imx/clk-imx27.c |6 +-
 arch/arm/mach-imx/clk-imx31.c |6 +-
 arch/arm/mach-imx/clk-imx35.c |6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
 drivers/usb/gadget/fsl_mxc_udc.c  |   23 +
 drivers/usb/gadget/fsl_udc_core.c |   52 +---
 drivers/usb/gadget/fsl_usb2_udc.h |   13 --
 include/linux/fsl_devices.h   |8 +++
 11 files changed, 87 insertions(+), 55 deletions(-)


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] pci: Provide support for parsing PCI DT ranges property

2013-01-14 Thread Andrew Murray
On Sat, Dec 15, 2012 at 01:06:41AM +, Grant Likely wrote:
> On Wed, Dec 12, 2012 at 4:37 PM, Andrew Murray  wrote:
> > DT bindings for PCI host bridges often use the ranges property to describe
> > memory and IO ranges - this binding tends to be the same across 
> > architectures
> > yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> > arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> > arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicate
> > functionality provided by drivers/of/address.c.
> 
> Hi Andrew,
> 
> Thanks for looking into this. This definitely needs to be done.
> 
> However, I cannot merge this patch as-is because it actually makes
> things worse by adding yet another implementation of the parsing code.
> Plus it doesn't actually have any users.  :-)

I understand. Though I see Thierry has included this in his patch set - I
guess that means there is potentially one user now :).

> 
> Instead, move the existing code that you need out of
> arch/powerpc/kernel/pci-common.c into a shared place and add in the
> features you need. Bonus points if you fixup microblaze or others at
> the same time.

In most part the patch I submitted was the common code from powerpc but
without quirks and tie-ins to powerpc structures. I'd like to convert
powerpc to using my patch and others but won't get time to do this at
present :(

> 
> g.
> 
Andrew Murray

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] pci: Provide support for parsing PCI DT ranges property

2013-01-14 Thread Andrew Murray
On Thu, Dec 20, 2012 at 08:25:00AM +, Thierry Reding wrote:
> On Wed, Dec 12, 2012 at 04:37:50PM +, Andrew Murray wrote:
> [...]
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> [...]
> > +   start = of_get_property(node, "ranges", &rlen);
> > +   if (start == NULL)
> > +   return NULL;
> > +
> > +   end = start + rlen;
> 
> I'm currently rewriting large parts of the Tegra PCIe controller driver
> and I'm trying to use this new API. This seems to work fine, except that
> I think this line needs to be:
> 
>   end = start + rlen / sizeof(__be32);
> 
> Otherwise we'll try to process 4 times as many ranges as there are.
> 
> Thierry

Good catch. Thanks for taking this on.

Andrew Murray


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h

2013-01-14 Thread Felipe Balbi
On Mon, Jan 14, 2013 at 05:13:44PM +0800, Peter Chen wrote:
> On Mon, Jan 14, 2013 at 10:50:27AM +0200, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Jan 14, 2013 at 04:17:35PM +0800, Peter Chen wrote:
> > > On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote:
> > > > > It changes the driver to use platform_device_id rather than cpu_is_xxx
> > > > > to determine the SoC type, and updates the platform code accordingly.
> > > > > 
> > > > > Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable.
> > > > > Tested at mx51 bbg board, it works ok after enable phy clock
> > > > > (Need another patch to fix this problem)
> > > > > 
> > > > > Signed-off-by: Peter Chen 
> > > > 
> > > > not good for -rc. You have to break this down as you're solving at least
> > > > three different problems with this patch.
> > > 
> > > Felipe, all my changes are for this problem, these are fix build error and
> > > let it work.
> > > 
> > >  arch/arm/mach-imx/clk-imx25.c |6 +-
> > >  arch/arm/mach-imx/clk-imx27.c |6 +-
> > >  arch/arm/mach-imx/clk-imx31.c |6 +-
> > >  arch/arm/mach-imx/clk-imx35.c |6 +-
> > >  arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-
> > > 
> > >  As we change the connection-id, we need to change clock file or
> > >  the devm_clk_get will be failed.
> > 
> > right right, that's ok.
> > 
> > >  arch/arm/mach-imx/devices/devices-common.h|1 +
> > >  arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
> > > 
> > >  We need to differentiate SoCs, so I use platform_device_id to
> > >  instead of cpu_ix_xxx(), this is for machine file change
> > 
> > fair enough.
> > 
> > >  drivers/usb/gadget/fsl_mxc_udc.c  |   23 +
> > >  drivers/usb/gadget/fsl_udc_core.c |   52 
> > > +---
> > >  drivers/usb/gadget/fsl_usb2_udc.h |   13 --
> > >  include/linux/fsl_devices.h   |8 +++
> > > 
> > >  Need to get platform_device_id at driver, and replace the cpu_is_xxx to
> > >  platform_device_id. Meanwhile, needs a solution for replace
> > >  MX35_IO_ADDRESS.
> > 
> > ok, here we go:
> > 
> > You just listed to me three different fixes and each fix should be on a
> > separate patch. Meaning that you should have one single patch to convert
> > MX35_IO_ADDRESS() into ioremap(), another patch should be removing
> > cpu_is_xxx() and the third one fixing connection-id.
> > 
> > As you might remember, we want patches to be self-contained logical
> > units and your patch contains 3 of such logical units.
> > 
> 
> OK, three patches. Thanks.
> 
> The reason I thought two patches is this build break contains two problems,
> (MX35_IO_ADDRESS & cpu_is_xxx()).
> I would like fix it together, in that case, the git bisect will only
> show one build error at most.

right, still those are two different problems, so they should be fixed
in two separate patches, I'm sorry.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h

2013-01-14 Thread Peter Chen
On Mon, Jan 14, 2013 at 10:50:27AM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jan 14, 2013 at 04:17:35PM +0800, Peter Chen wrote:
> > On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote:
> > > > It changes the driver to use platform_device_id rather than cpu_is_xxx
> > > > to determine the SoC type, and updates the platform code accordingly.
> > > > 
> > > > Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable.
> > > > Tested at mx51 bbg board, it works ok after enable phy clock
> > > > (Need another patch to fix this problem)
> > > > 
> > > > Signed-off-by: Peter Chen 
> > > 
> > > not good for -rc. You have to break this down as you're solving at least
> > > three different problems with this patch.
> > 
> > Felipe, all my changes are for this problem, these are fix build error and
> > let it work.
> > 
> >  arch/arm/mach-imx/clk-imx25.c |6 +-
> >  arch/arm/mach-imx/clk-imx27.c |6 +-
> >  arch/arm/mach-imx/clk-imx31.c |6 +-
> >  arch/arm/mach-imx/clk-imx35.c |6 +-
> >  arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-
> > 
> >  As we change the connection-id, we need to change clock file or
> >  the devm_clk_get will be failed.
> 
> right right, that's ok.
> 
> >  arch/arm/mach-imx/devices/devices-common.h|1 +
> >  arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
> > 
> >  We need to differentiate SoCs, so I use platform_device_id to
> >  instead of cpu_ix_xxx(), this is for machine file change
> 
> fair enough.
> 
> >  drivers/usb/gadget/fsl_mxc_udc.c  |   23 +
> >  drivers/usb/gadget/fsl_udc_core.c |   52 
> > +---
> >  drivers/usb/gadget/fsl_usb2_udc.h |   13 --
> >  include/linux/fsl_devices.h   |8 +++
> > 
> >  Need to get platform_device_id at driver, and replace the cpu_is_xxx to
> >  platform_device_id. Meanwhile, needs a solution for replace
> >  MX35_IO_ADDRESS.
> 
> ok, here we go:
> 
> You just listed to me three different fixes and each fix should be on a
> separate patch. Meaning that you should have one single patch to convert
> MX35_IO_ADDRESS() into ioremap(), another patch should be removing
> cpu_is_xxx() and the third one fixing connection-id.
> 
> As you might remember, we want patches to be self-contained logical
> units and your patch contains 3 of such logical units.
> 

OK, three patches. Thanks.

The reason I thought two patches is this build break contains two problems,
(MX35_IO_ADDRESS & cpu_is_xxx()).
I would like fix it together, in that case, the git bisect will only
show one build error at most.

-- 

Best Regards,
Peter Chen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h

2013-01-14 Thread Felipe Balbi
Hi,

On Mon, Jan 14, 2013 at 04:17:35PM +0800, Peter Chen wrote:
> On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote:
> > > It changes the driver to use platform_device_id rather than cpu_is_xxx
> > > to determine the SoC type, and updates the platform code accordingly.
> > > 
> > > Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable.
> > > Tested at mx51 bbg board, it works ok after enable phy clock
> > > (Need another patch to fix this problem)
> > > 
> > > Signed-off-by: Peter Chen 
> > 
> > not good for -rc. You have to break this down as you're solving at least
> > three different problems with this patch.
> 
> Felipe, all my changes are for this problem, these are fix build error and
> let it work.
> 
>  arch/arm/mach-imx/clk-imx25.c |6 +-
>  arch/arm/mach-imx/clk-imx27.c |6 +-
>  arch/arm/mach-imx/clk-imx31.c |6 +-
>  arch/arm/mach-imx/clk-imx35.c |6 +-
>  arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-
> 
>  As we change the connection-id, we need to change clock file or
>  the devm_clk_get will be failed.

right right, that's ok.

>  arch/arm/mach-imx/devices/devices-common.h|1 +
>  arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---
> 
>  We need to differentiate SoCs, so I use platform_device_id to
>  instead of cpu_ix_xxx(), this is for machine file change

fair enough.

>  drivers/usb/gadget/fsl_mxc_udc.c  |   23 +
>  drivers/usb/gadget/fsl_udc_core.c |   52 +---
>  drivers/usb/gadget/fsl_usb2_udc.h |   13 --
>  include/linux/fsl_devices.h   |8 +++
> 
>  Need to get platform_device_id at driver, and replace the cpu_is_xxx to
>  platform_device_id. Meanwhile, needs a solution for replace
>  MX35_IO_ADDRESS.

ok, here we go:

You just listed to me three different fixes and each fix should be on a
separate patch. Meaning that you should have one single patch to convert
MX35_IO_ADDRESS() into ioremap(), another patch should be removing
cpu_is_xxx() and the third one fixing connection-id.

As you might remember, we want patches to be self-contained logical
units and your patch contains 3 of such logical units.

> > Besides there are parts which are not related to the build break.
> 
> I can split this patch to two patches, one is driver part, the other is
> machine part. It can also fix build break, but it needs two
> patches to let the udc work.

you will need as many patches as necessary, but they need to the broken
correclty, the way you suggested above is wrong.

-- 
balbi


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h

2013-01-14 Thread Peter Chen
On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote:
> > It changes the driver to use platform_device_id rather than cpu_is_xxx
> > to determine the SoC type, and updates the platform code accordingly.
> > 
> > Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable.
> > Tested at mx51 bbg board, it works ok after enable phy clock
> > (Need another patch to fix this problem)
> > 
> > Signed-off-by: Peter Chen 
> 
> not good for -rc. You have to break this down as you're solving at least
> three different problems with this patch.

Felipe, all my changes are for this problem, these are fix build error and
let it work.

 arch/arm/mach-imx/clk-imx25.c |6 +-
 arch/arm/mach-imx/clk-imx27.c |6 +-
 arch/arm/mach-imx/clk-imx31.c |6 +-
 arch/arm/mach-imx/clk-imx35.c |6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c   |6 +-

 As we change the connection-id, we need to change clock file or
 the devm_clk_get will be failed.
 
 arch/arm/mach-imx/devices/devices-common.h|1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 +++---

 We need to differentiate SoCs, so I use platform_device_id to
 instead of cpu_ix_xxx(), this is for machine file change

 drivers/usb/gadget/fsl_mxc_udc.c  |   23 +
 drivers/usb/gadget/fsl_udc_core.c |   52 +---
 drivers/usb/gadget/fsl_usb2_udc.h |   13 --
 include/linux/fsl_devices.h   |8 +++

 Need to get platform_device_id at driver, and replace the cpu_is_xxx to
 platform_device_id. Meanwhile, needs a solution for replace
 MX35_IO_ADDRESS.

> 
> Besides there are parts which are not related to the build break.

I can split this patch to two patches, one is driver part, the other is
machine part. It can also fix build break, but it needs two
patches to let the udc work.

> 
> -- 
> balbi



-- 

Best Regards,
Peter Chen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev