Re: 3.7-rc7: BUG: MAX_STACK_TRACE_ENTRIES too low!
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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