RE: [PATCH v5 04/11] usb: otg: nop: add support for multiple tranceiver
Hi On Wed, Jul 25, 2012 at 05:37:22PM +0530, Ajay Kumar Gupta wrote: diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c index 01203eb..eb6220f 100644 --- a/drivers/usb/musb/am35x.c +++ b/drivers/usb/musb/am35x.c @@ -364,7 +364,7 @@ static int am35x_musb_init(struct musb *musb) if (!rev) return -ENODEV; - usb_nop_xceiv_register(); + usb_nop_xceiv_register(musb-id); musb-xceiv = usb_get_phy(USB_PHY_TYPE_USB2); if (IS_ERR_OR_NULL(musb-xceiv)) return -ENODEV; @@ -408,7 +408,7 @@ static int am35x_musb_exit(struct musb *musb) data-set_phy_power(0); usb_put_phy(musb-xceiv); - usb_nop_xceiv_unregister(); + usb_nop_xceiv_unregister(musb-xceiv); Dude, this is so wrong... You don't know if there are other users of the nop xceiv in the system. If there are other users which already have claimed id 0, you will try to create duplicated directories under sysfs. MUSB's ID has nothing to do with NOP's ID. I could not understood this comment as I am not using id. Please clarify. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 03/11] usb: musb: am335x: add support for dual instance
Hi, On Wed, Jul 25, 2012 at 05:37:21PM +0530, Ajay Kumar Gupta wrote: AM335x and TI81xx platform has dual musb controller so updating the musb_dspc.c to support the same. Changes: - Moved otg_workaround timer to glue structure - Moved static local variable last_timer to glue structure - PHY on/off related cleanups Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musb_dsps.c | 93 + 1 files changed, 57 insertions(+), 36 deletions(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 2174699..a2c8a06 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -105,6 +105,8 @@ struct dsps_musb_wrapper { /* miscellaneous stuff */ u32 musb_core_offset; u8 poll_seconds; + /* number of musb instances */ + u8 instances; }; /** @@ -112,16 +114,18 @@ struct dsps_musb_wrapper { */ struct dsps_glue { struct device *dev; - struct platform_device *musb; /* child musb pdev */ + struct platform_device *musb[2];/* child musb pdev */ const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */ - struct timer_list timer;/* otg_workaround timer */ - u32 __iomem *usb_ctrl; + struct timer_list timer[2]; /* otg_workaround timer */ + unsigned long last_timer[2];/* last timer data for each instance */ + u32 __iomem *usb_ctrl[2]; u8 usbss_rev; }; /** * musb_dsps_phy_control - phy on/off * @glue: struct dsps_glue * + * @id: musb instance * @on: flag for phy to be switched on or off * * This is to enable the PHY using usb_ctrl register in system control @@ -130,11 +134,11 @@ struct dsps_glue { * XXX: This function will be removed once we have a seperate driver for * control module */ -static void musb_dsps_phy_control(struct dsps_glue *glue, u8 on) +static void musb_dsps_phy_control(struct dsps_glue *glue, u8 id, u8 on) { u32 usbphycfg; - usbphycfg = __raw_readl(glue-usb_ctrl); + usbphycfg = __raw_readl(glue-usb_ctrl[id]); if (on) { if (glue-usbss_rev == MUSB_USBSS_REV_816X) { @@ -157,7 +161,7 @@ static void musb_dsps_phy_control(struct dsps_glue *glue, u8 on) glue-usbss_rev == MUSB_USBSS_REV_33XX) usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; } - __raw_writel(usbphycfg, glue-usb_ctrl); + __raw_writel(usbphycfg, glue-usb_ctrl[id]); } /** * dsps_musb_enable - enable interrupts @@ -247,7 +251,7 @@ static void otg_timer(unsigned long _musb) devctl = dsps_readb(mregs, MUSB_DEVCTL); if (devctl MUSB_DEVCTL_BDEVICE) - mod_timer(glue-timer, + mod_timer(glue-timer[musb-id], I believe this musb is a struct musb pointer, right ? Yes. Why would you use that ID ? You can find the correct musb by grabbing its platform_device with to_platform_device(musb-controller) and use that pdev to get to the correct ID. Let me cook something with this and update. Ajay ditto to all others. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 02/11] usb: musb: kill global and static for multi instance
Hi, On Wed, Jul 25, 2012 at 05:42:06PM +0530, Ajay Kumar Gupta wrote: @@ -240,20 +238,24 @@ int __devinit musb_init_debugfs(struct musb *musb) struct dentry *root; struct dentry *file; int ret; + charname[10]; - root = debugfs_create_dir(musb, NULL); + sprintf(name, musb%d, musb-id); + root = debugfs_create_dir(name, NULL); I told you to use dev_name(musb) for a reason. See what happens when you use dev_name(musb); Do you not think about the other users of this driver ? Do you not know what's the ID on platform_devices which don't have an ID ?? We are now using musb_ida for all glue layers so '-1' issue will not come. Still if we can manage without 'id' then better to do that way. Ajay The drivers core set it to -1, then on everybody who's got a single musb instance, will have to access the musb-1 directory. That doesn't seem nice. Just use dev_name(musb). L if (!root) { ret = -ENOMEM; goto err0; } - file = debugfs_create_file(regdump, S_IRUGO, root, musb, + sprintf(name, regdump%d, musb-id); + file = debugfs_create_file(name, S_IRUGO, root, musb, musb_regdump_fops); if (!file) { ret = -ENOMEM; goto err1; } + sprintf(name, testmode%d, musb-id); this is unnecessary!! Only the directory name needs this trick, whatever's under the root directory does not need any ID appended to it. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 04/11] usb: otg: nop: add support for multiple tranceiver
Hi, On Thu, Jul 26, 2012 at 08:22:35AM +, Gupta, Ajay Kumar wrote: @@ -408,7 +408,7 @@ static int am35x_musb_exit(struct musb *musb) data-set_phy_power(0); usb_put_phy(musb-xceiv); - usb_nop_xceiv_unregister(); + usb_nop_xceiv_unregister(musb-xceiv); Dude, this is so wrong... You don't know if there are other users of the nop xceiv in the system. If there are other users which already have claimed id 0, you will try to create duplicated directories under sysfs. MUSB's ID has nothing to do with NOP's ID. I could not understood this comment as I am not using id. Please clarify. sorry, my bad. I put the comment on the wrong hunk: @@ -364,7 +364,7 @@ static int am35x_musb_init(struct musb *musb) if (!rev) return -ENODEV; - usb_nop_xceiv_register(); + usb_nop_xceiv_register(musb-id); musb-xceiv = usb_get_phy(USB_PHY_TYPE_USB2); if (IS_ERR_OR_NULL(musb-xceiv)) return -ENODEV; here. You pass musb-id to become NOP's ID. Got this, so something like musb_ida is needed even for NOP. Right? Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 02/11] usb: musb: kill global and static for multi instance
Hi, On Wed, Jul 25, 2012 at 05:42:06PM +0530, Ajay Kumar Gupta wrote: @@ -240,20 +238,24 @@ int __devinit musb_init_debugfs(struct musb *musb) struct dentry *root; struct dentry *file; int ret; + charname[10]; - root = debugfs_create_dir(musb, NULL); + sprintf(name, musb%d, musb-id); + root = debugfs_create_dir(name, NULL); I told you to use dev_name(musb) for a reason. See what happens when you use dev_name(musb); Do you not think about the other users of this driver ? Do you not know what's the ID on platform_devices which don't have an ID ?? We are now using musb_ida for all glue layers so '-1' issue will not come. true. Still if we can manage without 'id' then better to do that way. just make sure to notice that directory name will musb.0. Sure. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 04/11] usb: otg: nop: add support for multiple tranceiver
Hi, On Thu, Jul 26, 2012 at 08:29:06AM +, Gupta, Ajay Kumar wrote: Hi, On Thu, Jul 26, 2012 at 08:22:35AM +, Gupta, Ajay Kumar wrote: @@ -408,7 +408,7 @@ static int am35x_musb_exit(struct musb *musb) data-set_phy_power(0); usb_put_phy(musb-xceiv); - usb_nop_xceiv_unregister(); + usb_nop_xceiv_unregister(musb-xceiv); Dude, this is so wrong... You don't know if there are other users of the nop xceiv in the system. If there are other users which already have claimed id 0, you will try to create duplicated directories under sysfs. MUSB's ID has nothing to do with NOP's ID. I could not understood this comment as I am not using id. Please clarify. sorry, my bad. I put the comment on the wrong hunk: @@ -364,7 +364,7 @@ static int am35x_musb_init(struct musb *musb) if (!rev) return -ENODEV; - usb_nop_xceiv_register(); + usb_nop_xceiv_register(musb-id); musb-xceiv = usb_get_phy(USB_PHY_TYPE_USB2); if (IS_ERR_OR_NULL(musb-xceiv)) return -ENODEV; here. You pass musb-id to become NOP's ID. Got this, so something like musb_ida is needed even for NOP. Right? that's correct. But I guess you can avoid nop_get_id() and build that into nop_register and nop_unregister() calls, so that the changes are local to nop xceiv only. Correct. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 02/11] usb: musb: kill global and static for multi instance
Hi, On Wed, Jul 25, 2012 at 05:42:06PM +0530, Ajay Kumar Gupta wrote: @@ -240,20 +238,24 @@ int __devinit musb_init_debugfs(struct musb *musb) struct dentry *root; struct dentry *file; int ret; + charname[10]; - root = debugfs_create_dir(musb, NULL); + sprintf(name, musb%d, musb-id); + root = debugfs_create_dir(name, NULL); I told you to use dev_name(musb) for a reason. See what happens when you use dev_name(musb); Do you not think about the other users of this driver ? Do you not know what's the ID on platform_devices which don't have an ID ?? We are now using musb_ida for all glue layers so '-1' issue will not come. true. Still if we can manage without 'id' then better to do that way. just make sure to notice that directory name will musb.0. It creates musb-hdrc.0 and musb-hdrc.1 as glue layers use musb-hdrc name in platform_device_alloc() call. Ajay Sure. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 02/11] usb: musb: kill global and static for multi instance
Hi On Thu, Jul 26, 2012 at 12:46:43PM +, Gupta, Ajay Kumar wrote: Hi, On Wed, Jul 25, 2012 at 05:42:06PM +0530, Ajay Kumar Gupta wrote: @@ -240,20 +238,24 @@ int __devinit musb_init_debugfs(struct musb *musb) struct dentry *root; struct dentry *file; int ret; + charname[10]; - root = debugfs_create_dir(musb, NULL); + sprintf(name, musb%d, musb-id); + root = debugfs_create_dir(name, NULL); I told you to use dev_name(musb) for a reason. See what happens when you use dev_name(musb); Do you not think about the other users of this driver ? Do you not know what's the ID on platform_devices which don't have an ID ?? We are now using musb_ida for all glue layers so '-1' issue will not come. true. Still if we can manage without 'id' then better to do that way. just make sure to notice that directory name will musb.0. It creates musb-hdrc.0 and musb-hdrc.1 as glue layers use musb-hdrc name in platform_device_alloc() call. yes, indeed. That shouldn't be a problem though... what do you think ? I agree with you as that should be fine. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 01/11] usb: musb: add musb-id to identify core instance
Hi, On Thu, Jul 19, 2012 at 05:15:57PM +0530, Ajay Kumar Gupta wrote: diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 89d1871..3e09984 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -99,6 +99,7 @@ #include linux/prefetch.h #include linux/platform_device.h #include linux/io.h +#include linux/idr.h #include musb_core.h @@ -114,6 +115,7 @@ #define MUSB_DRIVER_NAME musb-hdrc const char musb_driver_name[] = MUSB_DRIVER_NAME; +static DEFINE_IDA(musb_ida); MODULE_DESCRIPTION(DRIVER_INFO); MODULE_AUTHOR(DRIVER_AUTHOR); @@ -130,6 +132,33 @@ static inline struct musb *dev_to_musb(struct device *dev) /* -*/ +int musb_get_id(struct device *dev, gfp_t gfp_mask) { + int ret; + int id; + + ret = ida_pre_get(musb_ida, gfp_mask); + if (!ret) { + dev_err(dev, failed to reserve resource for id\n); + return -ENOMEM; + } + + ret = ida_get_new(musb_ida, id); + if (ret 0) { + dev_err(dev, failed to allocate a new id\n); + return ret; + } + + return id; +} + +void musb_put_id(struct device *dev, int id) { + + dev_dbg(dev, removing id %d\n, id); + ida_remove(musb_ida, id); +} both of these should be EXPORT_SYMBOL_GPL() for cases when musb and glue are compiled as modules. Sure. I will fix this. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 02/11] usb: musb: kill global and static for multi instance
Hi, On Thu, Jul 19, 2012 at 05:15:58PM +0530, Ajay Kumar Gupta wrote: Moved global variable musb_debugfs_root and static variable old_state to 'struct musb' to help support multi instance of musb controller as present on AM335x platform. Also removed the global variable orig_dma_mask and filled the dev-dma_mask with parent device's dma_mask. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musb_core.c| 16 +++- drivers/usb/musb/musb_core.h|4 drivers/usb/musb/musb_debugfs.c | 14 -- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 3e09984..a5db4dd 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1802,10 +1802,9 @@ static const struct attribute_group musb_attr_group = { static void musb_irq_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, irq_work); - static int old_state; - if (musb-xceiv-state != old_state) { - old_state = musb-xceiv-state; + if (musb-xceiv-state != musb-xceiv_old_state) { + musb-xceiv_old_state = musb-xceiv-state; sysfs_notify(musb-controller-kobj, NULL, mode); } } @@ -2115,11 +2114,6 @@ fail0: /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just * bridge to a platform device; this driver then suffices. */ - -#ifndef CONFIG_MUSB_PIO_ONLY -static u64 *orig_dma_mask; -#endif - static int __devinit musb_probe(struct platform_device *pdev) { struct device *dev = pdev-dev; @@ -2138,10 +2132,6 @@ static int __devinit musb_probe(struct platform_device *pdev) return -ENOMEM; } -#ifndef CONFIG_MUSB_PIO_ONLY - /* clobbered by use_dma=n */ - orig_dma_mask = dev-dma_mask; -#endif status = musb_init_controller(dev, irq, base); if (status 0) iounmap(base); @@ -2166,7 +2156,7 @@ static int __devexit musb_remove(struct platform_device *pdev) iounmap(ctrl_base); device_init_wakeup(pdev-dev, 0); #ifndef CONFIG_MUSB_PIO_ONLY - pdev-dev.dma_mask = orig_dma_mask; + pdev-dev.dma_mask = (dev-dev.parent)-dma_mask; are you sure ?? This should be: dev-parent.dma_mask; Actually both are fine and I used parent based on your recommendation in my earlier version of patch at http://marc.info/?l=linux-usbm=134121391908734w=2 anyways I will change to : pdev-dev.dma_mask = dma_set_mask((pdev-dev, DMA_BIT_MASK(64)); Ajay In fact, while doing that you could use dma_set_mask() instead ;-) -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 02/11] usb: musb: kill global and static for multi instance
Hi, On Wed, Jul 25, 2012 at 10:34:49AM +, Gupta, Ajay Kumar wrote: Hi, On Thu, Jul 19, 2012 at 05:15:58PM +0530, Ajay Kumar Gupta wrote: Moved global variable musb_debugfs_root and static variable old_state to 'struct musb' to help support multi instance of musb controller as present on AM335x platform. Also removed the global variable orig_dma_mask and filled the dev-dma_mask with parent device's dma_mask. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musb_core.c| 16 +++- drivers/usb/musb/musb_core.h|4 drivers/usb/musb/musb_debugfs.c | 14 -- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 3e09984..a5db4dd 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1802,10 +1802,9 @@ static const struct attribute_group musb_attr_group = { static void musb_irq_work(struct work_struct *data) { struct musb *musb = container_of(data, struct musb, irq_work); - static int old_state; - if (musb-xceiv-state != old_state) { - old_state = musb-xceiv-state; + if (musb-xceiv-state != musb-xceiv_old_state) { + musb-xceiv_old_state = musb-xceiv-state; sysfs_notify(musb-controller-kobj, NULL, mode); } } @@ -2115,11 +2114,6 @@ fail0: /* all implementations (PCI bridge to FPGA, VLYNQ, etc) should just * bridge to a platform device; this driver then suffices. */ - -#ifndef CONFIG_MUSB_PIO_ONLY -static u64 *orig_dma_mask; -#endif - static int __devinit musb_probe(struct platform_device *pdev) { struct device *dev = pdev-dev; @@ -2138,10 +2132,6 @@ static int __devinit musb_probe(struct platform_device *pdev) return -ENOMEM; } -#ifndef CONFIG_MUSB_PIO_ONLY - /* clobbered by use_dma=n */ - orig_dma_mask = dev-dma_mask; -#endif status = musb_init_controller(dev, irq, base); if (status 0) iounmap(base); @@ -2166,7 +2156,7 @@ static int __devexit musb_remove(struct platform_device *pdev) iounmap(ctrl_base); device_init_wakeup(pdev-dev, 0); #ifndef CONFIG_MUSB_PIO_ONLY - pdev-dev.dma_mask = orig_dma_mask; + pdev-dev.dma_mask = (dev-dev.parent)-dma_mask; are you sure ?? This should be: dev-parent.dma_mask; Actually both are fine and I used parent based on your recommendation in my earlier version of patch at http://marc.info/?l=linux-usbm=134121391908734w=2 anyways I will change to : pdev-dev.dma_mask = dma_set_mask((pdev-dev, DMA_BIT_MASK(64)); no Ajay, you misunderstood... I don't see how this patch would compile. It was my bad. I am testing it with PIO mode only and so code was not compiled. dev was actually a pdev. the variable named dev is already a pointer to a struct device, and struct device doesn't have a dev field. what I asked to do while doing that is: dev_set_mask(dev, *dev-parent-dma_mask); Sure. Thanks. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 01/11] usb: musb: add musb-id to identify core instance
Hi, On Wed, Jul 25, 2012 at 05:37:19PM +0530, Ajay Kumar Gupta wrote: diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 89d1871..f5eb8a7 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c snip @@ -1889,6 +1921,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) pm_runtime_enable(musb-controller); spin_lock_init(musb-lock); + musb-id = pdev-id; I fail to see where this musb-id would be used. Care to clarify ? Sure, It is used in musb_init_debugfs(). Please refer 02/11. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 01/11] usb: musb: add musb-id to identify core instance
Hi, On Wed, Jul 25, 2012 at 12:19:32PM +, Gupta, Ajay Kumar wrote: Hi, On Wed, Jul 25, 2012 at 05:37:19PM +0530, Ajay Kumar Gupta wrote: diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 89d1871..f5eb8a7 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c snip @@ -1889,6 +1921,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) pm_runtime_enable(musb-controller); spin_lock_init(musb-lock); + musb-id = pdev-id; I fail to see where this musb-id would be used. Care to clarify ? Sure, It is used in musb_init_debugfs(). Please refer 02/11. Then it's a bit unnecessary right ? You can change the directory name to dev_name(musb-controller) as that will already have the id appended to it. Well, musb-id is used at many more places and it has to be there. Please refer 01/11 where all glue layers using it for: + musb_put_id(pdev-dev, glue-musb-id); 03/11 where musb dsps glue is extensively using it at multiple places. 04/11 where all glue using if then need nop. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 01/11] usb: musb: add musb-id to identify core instance
Hi, On Wed, Jul 25, 2012 at 12:30:12PM +, Gupta, Ajay Kumar wrote: Hi, On Wed, Jul 25, 2012 at 12:19:32PM +, Gupta, Ajay Kumar wrote: Hi, On Wed, Jul 25, 2012 at 05:37:19PM +0530, Ajay Kumar Gupta wrote: diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 89d1871..f5eb8a7 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c snip @@ -1889,6 +1921,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) pm_runtime_enable(musb-controller); spin_lock_init(musb-lock); + musb-id = pdev-id; I fail to see where this musb-id would be used. Care to clarify ? Sure, It is used in musb_init_debugfs(). Please refer 02/11. Then it's a bit unnecessary right ? You can change the directory name to dev_name(musb-controller) as that will already have the id appended to it. Well, musb-id is used at many more places and it has to be there. Please refer 01/11 where all glue layers using it for: + musb_put_id(pdev-dev, glue-musb-id); Here glue-musb is a platform_device and not struct musb. So it's limited at glue layer alone. All IDs are being saved in each pdev-id. We have two musb instances and so we have glue-musb[2] pdevs and so each IDs getting saved at glue-musb[0]-id and glue-musb[1]-id. How about musb-id (I mean struct musb here) uses at other places? 03/11 where musb dsps glue is extensively using it at multiple places. 04/11 where all glue using if they need nop. Ajay your struct *_glue{}; should be the one holding the ID as it's the one who requested it. If you request multiple IDs, you hold multiple IDs (glue-id0, glue-id1, glue-idN) and so on. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 07/11] usb: otg: nop: add dt support
Hi, Added device tree support for nop transceiver driver and updated the Documentation with device tree binding information for am33xx platform. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- .../devicetree/bindings/usb/am33xx-usb.txt |3 +++ drivers/usb/otg/nop-usb-xceiv.c| 12 2 files changed, 15 insertions(+), 0 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/am33xx-usb.txt b/Documentation/devicetree/bindings/usb/am33xx-usb.txt index ca8fa56..9782585 100644 --- a/Documentation/devicetree/bindings/usb/am33xx-usb.txt +++ b/Documentation/devicetree/bindings/usb/am33xx-usb.txt @@ -12,3 +12,6 @@ AM33XX MUSB GLUE represents PERIPHERAL. - power : Should be 250. This signifies the controller can supply upto 500mA when operating in host mode. + +NOP USB PHY + - compatible : Should be nop-xceiv-usb diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb- xceiv.c index 2e5e889..102e7d8 100644 --- a/drivers/usb/otg/nop-usb-xceiv.c +++ b/drivers/usb/otg/nop-usb-xceiv.c @@ -27,6 +27,7 @@ */ #include linux/module.h +#include linux/of.h #include linux/platform_device.h #include linux/dma-mapping.h #include linux/usb/otg.h @@ -152,12 +153,23 @@ static int __devexit nop_usb_xceiv_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_OF +static const struct of_device_id nop_xceiv_id_table[] = { + { .compatible = nop-xceiv-usb }, + {} +}; +MODULE_DEVICE_TABLE(of, nop_xceiv_id_table); +#else +#define nop_xceiv_id_table NULL +#endif The *#else* part is not needed as your *of_match_ptr* will take care of it. Ok, I will update this. Ajay + static struct platform_driver nop_usb_xceiv_driver = { .probe = nop_usb_xceiv_probe, .remove = __devexit_p(nop_usb_xceiv_remove), .driver = { .name = nop_usb_xceiv, .owner = THIS_MODULE, + .of_match_table = of_match_ptr(nop_xceiv_id_table), }, }; Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 04/11] usb: otg: nop: add support for multiple tranceiver
Hi, Currently we have one single nop transceiver support as same is defined as a global variable in drivers/usb/otg/nop-usb-xceiv.c. This need to be changed to support multiple otg controller each using nop transceiver on a platform such as am335x. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/mach-omap2/board-omap3evm.c |2 +- drivers/usb/musb/am35x.c |4 ++-- drivers/usb/musb/blackfin.c |4 ++-- drivers/usb/musb/da8xx.c |4 ++-- drivers/usb/musb/davinci.c |6 +++--- drivers/usb/musb/musb_dsps.c | 10 +- drivers/usb/musb/tusb6010.c |6 +++--- drivers/usb/otg/nop-usb-xceiv.c | 20 include/linux/usb/otg.h |9 + 9 files changed, 35 insertions(+), 30 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- omap2/board-omap3evm.c index ef230a0..a3393bc 100644 [...] diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index 4636d39..36cc791 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -95,6 +95,7 @@ struct usb_phy { struct device *dev; const char *label; unsigned int flags; + u8 id; Do we really need to have *id* in phy?? I will update the patches dropping this. enum usb_phy_type type; enum usb_otg_state state; @@ -137,14 +138,14 @@ extern void usb_remove_phy(struct usb_phy *); #if defined(CONFIG_NOP_USB_XCEIV) || (defined(CONFIG_NOP_USB_XCEIV_MODULE) defined(MODULE)) /* sometimes transceivers are accessed only through e.g. ULPI */ -extern void usb_nop_xceiv_register(void); -extern void usb_nop_xceiv_unregister(void); +extern void usb_nop_xceiv_register(int id); +extern void usb_nop_xceiv_unregister(struct usb_phy *); IMHO, these declarations shouldn't be in usb/otg.h. It can be in a header file specific to usb_nop? They were in otg.h and just definition getting changed so I think they need to be in otg.h only as a logical change of this patch. Location can be changed as a separate patch later. Thanks, Ajay Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4 09/11] drivers: usb: musb: Add device tree support for omap musb glue
Hi, Signed-off-by: Kishon Vijay Abraham I kis...@ti.com --- Documentation/devicetree/bindings/usb/omap-usb.txt | 34 - drivers/usb/musb/omap2430.c| 55 2 files changed, 88 insertions(+), 1 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt index 80a28c9..39cdffb 100644 --- a/Documentation/devicetree/bindings/usb/omap-usb.txt +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt @@ -1,4 +1,4 @@ -OMAP USB PHY +OMAP USB PHY AND GLUE OMAP USB2 PHY @@ -14,3 +14,35 @@ usb2phy@0x4a0ad080 { compatible = ti,omap-usb2; reg = 0x4a0ad080 0x58; }; + +OMAP MUSB GLUE + - compatible : Should be ti,musb-omap2430 + - ti,hwmods : must be usb_otg_hs + - multipoint : Should be 1 indicating the musb controller supports + multipoint. This is a MUSB configuration-specific setting. + - num_eps : Specifies the number of endpoints. This is also a + MUSB configuration-specific setting. Should be set to 16 + - ram_bits : Specifies the ram address size. Should be set to 12 + - interface_type : This is a board specific setting to describe the type of + interface between the controller and the phy. It should be 0 or 1 + specifying ULPI and UTMI respectively. + - mode : Should be 3 to represent OTG. 1 signifies HOST and 2 + represents PERIPHERAL. + - power : Should be 50. This signifies the controller can supply upto + 100mA when operating in host mode. + +SOC specific device node entry +usb_otg_hs: usb_otg_hs@4a0ab000 { + compatible = ti,musb-omap2430; + ti,hwmods = usb_otg_hs; + multipoint = 1; + num_eps = 16; + ram_bits = 12; +}; + +Board specific device node entry +usb_otg_hs { + interface_type = 1; + mode = 3; + power = 50; +}; diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index addbebf..331e477 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -30,6 +30,7 @@ #include linux/init.h #include linux/list.h #include linux/io.h +#include linux/of.h #include linux/platform_device.h #include linux/dma-mapping.h #include linux/pm_runtime.h @@ -469,8 +470,11 @@ static u64 omap2430_dmamask = DMA_BIT_MASK(32); static int __devinit omap2430_probe(struct platform_device *pdev) { struct musb_hdrc_platform_data *pdata = pdev-dev.platform_data; + struct omap_musb_board_data *data; struct platform_device *musb; struct omap2430_glue*glue; + struct device_node *np = pdev-dev.of_node; + struct musb_hdrc_config *config; struct resource *res; int ret = -ENOMEM; @@ -500,6 +504,43 @@ static int __devinit omap2430_probe(struct platform_device *pdev) if (glue-control_otghs == NULL) dev_dbg(pdev-dev, Failed to obtain control memory\n); + if (np) { + pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) { + dev_err(pdev-dev, + failed to allocate musb platfrom data\n); + ret = -ENOMEM; + goto err1; + } + + data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL); + if (!data) { + dev_err(pdev-dev, + failed to allocate musb board data\n); + ret = -ENOMEM; + goto err1; + } + + config = devm_kzalloc(pdev-dev, sizeof(*config), GFP_KERNEL); + if (!data) { + dev_err(pdev-dev, + failed to allocate musb hdrc config\n); + goto err1; + } + + of_property_read_u32(np, mode, (u32 *)pdata-mode); + of_property_read_u32(np, interface_type, + (u32 *)data-interface_type); + of_property_read_u32(np, num_eps, (u32 *)config-num_eps); + of_property_read_u32(np, ram_bits, (u32 *)config-ram_bits); + of_property_read_u32(np, mode, (u32 *)pdata-mode); pdata-mode is already read so above should be removed. Ajay + of_property_read_u32(np, power, (u32 *)pdata-power); + config-multipoint = of_property_read_bool(np, multipoint); + + pdata-board_data = data; + pdata-config = config; + } + pdata-platform_ops = omap2430_ops; platform_set_drvdata(pdev, glue); @@ -597,12 +638,26 @@ static struct dev_pm_ops omap2430_pm_ops = { #define DEV_PM_OPS NULL #endif +#ifdef CONFIG_OF +static const struct of_device_id omap2430_id_table[] = { + { + .compatible =
RE: [PATCH v2 07/11] usb: otg: nop: add dt support
Hi, On 17-07-2012 13:13, Ajay Kumar Gupta wrote: Added device tree support for nop transceiver driver and updated the Documentation with device tree binding information for am33xx platform. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- .../devicetree/bindings/usb/am33xx-usb.txt |3 +++ drivers/usb/otg/nop-usb-xceiv.c| 12 2 files changed, 15 insertions(+), 0 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/am33xx-usb.txt b/Documentation/devicetree/bindings/usb/am33xx-usb.txt index ca8fa56..a314720 100644 --- a/Documentation/devicetree/bindings/usb/am33xx-usb.txt +++ b/Documentation/devicetree/bindings/usb/am33xx-usb.txt @@ -12,3 +12,6 @@ AM33XX MUSB GLUE represents PERIPHERAL. - power : Should be 250. This signifies the controller can supply upto 500mA when operating in host mode. + +NOP USB PHY + - compatible : Should be ti,nop-xceiv-usb Why are you declaring it as TI specific. There's nothing company specific in the NOP transceiver. Thanks for review. I will change this to nop-xceiv-usb. Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 08/11] arm/dts: am33xx: add dt data for usb nop phy
Hi, AM33xx has two musb controller and they have one NOP PHY each. Added the device tree data for NOP PHY. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/boot/dts/am33xx.dtsi |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index b572803..3bd9911 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -155,6 +155,14 @@ ti,hwmods = i2c3; }; + usb0_phy: phy@1 { + compatible = ti,nop-xceiv-usb; + }; + + usb1_phy: phy@2 { + compatible = ti,nop-xceiv-usb; + }; No reg property again, and address postfix in the node name? I will update this to + usb0_phy: phy0 { + usb1_phy: phy1 { Thanks, Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 06/11] arm/dts: am33xx: Add dt data for usbss
Hi, On 17-07-2012 13:13, Ajay Kumar Gupta wrote: Added device tree data for usbss on am33xx. There are two musb controllers on am33xx platform so have port0_mode and port1_mode additional data. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/boot/dts/am33xx.dtsi | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 59509c4..b572803 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -154,5 +154,16 @@ #size-cells = 0; ti,hwmods = i2c3; }; + + usb_otg_hs: usb_otg_hs@4740 { The reg property is absent, so why did you give the node name the address postfix? Currently reg property is coming from ti hwmods. Thanks, Ajay + compatible = ti,musb-am33xx; + ti,hwmods = usb_otg_hs; + multipoint = 1; + num_eps = 16; + ram_bits = 12; + port0_mode = 3; + port1_mode = 1; + power = 250; + }; WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 06/11] arm/dts: am33xx: Add dt data for usbss
Hi On 17-07-2012 13:13, Ajay Kumar Gupta wrote: Added device tree data for usbss on am33xx. There are two musb controllers on am33xx platform so have port0_mode and port1_mode additional data. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/boot/dts/am33xx.dtsi | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 59509c4..b572803 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -154,5 +154,16 @@ #size-cells = 0; ti,hwmods = i2c3; }; + + usb_otg_hs: usb_otg_hs@4740 { The reg property is absent, so why did you give the node name the address postfix? Currently reg property is coming from ti hwmods. I meant base addresses are coming from hwmods and so there is no reg property added here. I will update the patch dropping address postfix. Ajay + compatible = ti,musb-am33xx; + ti,hwmods = usb_otg_hs; + multipoint = 1; + num_eps = 16; + ram_bits = 12; + port0_mode = 3; + port1_mode = 1; + power = 250; + }; WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3] usb: musb: dsps: add phy control logic to glue
Hi, On Mon, Jul 9, 2012 at 7:18 PM, Damodar Santhapuri x0132...@ti.com [...] Avoid getting resource by name.. With dt, you wont be able to do those.. When we are completely on DT then this would anyways go and we can get these resource data for each usb port from DT APIs. Even with dt, you will still get the resources using platform_get_resource() API's.. So why not have it that way from the beginning itself.. Ok got it. We will post an updated patch. Plalform_get_resource api works fine if we hardcode 3rd argument to 3+id for usb_cntrl register Since dsps_create_musb_pdev calls for each wrp-instances. static int __devinit dsps_create_musb_pdev(struct dsps_glue *glue, u8 id) { /* get memory resource for usb control register */ res = platform_get_resource(pdev, IORESOURCE_MEM, 3+id); dev_err(dev,Damodar :usb ctrl%d res=%x\n,id,res-start); if (!res) { dev_err(dev, %s get mem resource failed\n, res_name); ret = -ENODEV; goto err0; } } You can place the memory resources as {{usbss, .}, {musb0, .}, {usb0_ctrl, .}, {musb1, .}, {usb1_ctrl, .}} and then use index as 2 * id + 2 to get the usbX_ctrl base address. Ajay Damodar. Ajay Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 2/3] usb: musb: dsps: add phy control logic to glue
Hi, On Wed, Jul 11, 2012 at 3:59 PM, Damodar Santhapuri x0132...@ti.com wrote: From: Ajay Kumar Gupta ajay.gu...@ti.com AM335x uses NOP transceiver driver and need to enable builtin PHY by writing into usb_ctrl register available in system control module register space. This is being added at musb glue driver layer untill a separate system control module driver is available. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Damodar Santhapuri x0132...@ti.com --- Changes from v0: - Used platform_get_resource() instead of platform_get_resource_byname() based on Kishon's comment. arch/arm/mach-omap2/board-ti8168evm.c |1 - arch/arm/mach-omap2/omap_phy_internal.c | 35 arch/arm/plat-omap/include/plat/usb.h |5 +- drivers/usb/musb/musb_dsps.c| 87 +-- 4 files changed, 73 insertions(+), 55 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach- omap2/board-ti8168evm.c index d4c8392..0c7c098 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -26,7 +26,6 @@ #include plat/usb.h static struct omap_musb_board_data musb_board_data = { - .set_phy_power = ti81xx_musb_phy_power, .interface_type = MUSB_INTERFACE_ULPI, .mode = MUSB_OTG, .power = 500, diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index d52651a..d80bb16 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -254,38 +254,3 @@ void am35x_set_mode(u8 musb_mode) omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); } - -void ti81xx_musb_phy_power(u8 on) -{ - void __iomem *scm_base = NULL; - u32 usbphycfg; - - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); - if (!scm_base) { - pr_err(system control module ioremap failed\n); - return; - } - - usbphycfg = __raw_readl(scm_base + USBCTRL0); - - if (on) { - if (cpu_is_ti816x()) { - usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; - usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; - } else if (cpu_is_ti814x()) { - usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN - | USBPHY_DPINPUT | USBPHY_DMINPUT); - usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN - | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL); - } - } else { - if (cpu_is_ti816x()) - usbphycfg = ~TI816X_USBPHY0_NORMAL_MODE; - else if (cpu_is_ti814x()) - usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; - - } - __raw_writel(usbphycfg, scm_base + USBCTRL0); - - iounmap(scm_base); -} diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat- omap/include/plat/usb.h index 548a4c8..c2aa4ae 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -95,7 +95,6 @@ extern void am35x_musb_reset(void); extern void am35x_musb_phy_power(u8 on); extern void am35x_musb_clear_irq(void); extern void am35x_set_mode(u8 musb_mode); -extern void ti81xx_musb_phy_power(u8 on); /* AM35x */ /* USB 2.0 PHY Control */ @@ -120,8 +119,8 @@ extern void ti81xx_musb_phy_power(u8 on); #define CONF2_DATPOL (1 1) /* TI81XX specific definitions */ -#define USBCTRL0 0x620 -#define USBSTAT0 0x624 +#define MUSB_USBSS_REV_816X0x9 +#define MUSB_USBSS_REV_814X0xb /* TI816X PHY controls bits */ #define TI816X_USBPHY0_NORMAL_MODE (1 0) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 494772f..72eda64 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -115,9 +115,46 @@ struct dsps_glue { struct platform_device *musb; /* child musb pdev */ const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */ struct timer_list timer;/* otg_workaround timer */ + u32 __iomem *usb_ctrl; + u8 usbss_rev; }; /** + * musb_dsps_phy_control - phy on/off + * @glue: struct dsps_glue * + * @on: flag for phy to be switched on or off + * + * This is to enable the PHY using usb_ctrl register in system control + * module space. + * + * XXX: This function will be removed once we have a seperate driver for + * control module + */ +static void musb_dsps_phy_control(struct dsps_glue *glue, u8 on) I think this function should be added in your transceiver driver. I don't see glue as an appropriate place for this We use NOP
RE: [PATCH 2/3] usb: musb: dsps: add phy control logic to glue
Hi, On Mon, Jul 9, 2012 at 7:18 PM, Damodar Santhapuri x0132...@ti.com wrote: From: Ajay Kumar Gupta ajay.gu...@ti.com AM335x uses NOP transceiver driver and need to enable builtin PHY by writing into usb_ctrl register available in system control module register space. This is being added at musb glue driver layer untill a separate system control module driver is available. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Damodar Santhapuri x0132...@ti.com --- arch/arm/mach-omap2/board-ti8168evm.c |1 - arch/arm/mach-omap2/omap_phy_internal.c | 35 arch/arm/plat-omap/include/plat/usb.h |5 +- drivers/usb/musb/musb_dsps.c| 88 +-- 4 files changed, 74 insertions(+), 55 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach- omap2/board-ti8168evm.c index d4c8392..0c7c098 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -26,7 +26,6 @@ #include plat/usb.h static struct omap_musb_board_data musb_board_data = { - .set_phy_power = ti81xx_musb_phy_power, .interface_type = MUSB_INTERFACE_ULPI, .mode = MUSB_OTG, .power = 500, diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index d52651a..d80bb16 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -254,38 +254,3 @@ void am35x_set_mode(u8 musb_mode) omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); } - -void ti81xx_musb_phy_power(u8 on) -{ - void __iomem *scm_base = NULL; - u32 usbphycfg; - - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); - if (!scm_base) { - pr_err(system control module ioremap failed\n); - return; - } - - usbphycfg = __raw_readl(scm_base + USBCTRL0); - - if (on) { - if (cpu_is_ti816x()) { - usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; - usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; - } else if (cpu_is_ti814x()) { - usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN - | USBPHY_DPINPUT | USBPHY_DMINPUT); - usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN - | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL); - } - } else { - if (cpu_is_ti816x()) - usbphycfg = ~TI816X_USBPHY0_NORMAL_MODE; - else if (cpu_is_ti814x()) - usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; - - } - __raw_writel(usbphycfg, scm_base + USBCTRL0); - - iounmap(scm_base); -} diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat- omap/include/plat/usb.h index 548a4c8..c2aa4ae 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -95,7 +95,6 @@ extern void am35x_musb_reset(void); extern void am35x_musb_phy_power(u8 on); extern void am35x_musb_clear_irq(void); extern void am35x_set_mode(u8 musb_mode); -extern void ti81xx_musb_phy_power(u8 on); /* AM35x */ /* USB 2.0 PHY Control */ @@ -120,8 +119,8 @@ extern void ti81xx_musb_phy_power(u8 on); #define CONF2_DATPOL (1 1) /* TI81XX specific definitions */ -#define USBCTRL0 0x620 -#define USBSTAT0 0x624 +#define MUSB_USBSS_REV_816X0x9 +#define MUSB_USBSS_REV_814X0xb /* TI816X PHY controls bits */ #define TI816X_USBPHY0_NORMAL_MODE (1 0) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 494772f..f7271c3 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -115,9 +115,46 @@ struct dsps_glue { struct platform_device *musb; /* child musb pdev */ const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */ struct timer_list timer;/* otg_workaround timer */ + u32 __iomem *usb_ctrl; + u8 usbss_rev; }; /** + * musb_dsps_phy_control - phy on/off + * @glue: struct dsps_glue * + * @on: flag for phy to be switched on or off + * + * This is to enable the PHY using usb_ctrl register in system control + * module space. + * + * XXX: This function will be removed once we have a seperate driver for %s/seperate/separate + * control module + */ +static void musb_dsps_phy_control(struct dsps_glue *glue, u8 on) +{ + u32 usbphycfg; + + usbphycfg = __raw_readl(glue-usb_ctrl); How about using readl instead of __raw_readl here and below? + + if (on) { + if (glue-usbss_rev == MUSB_USBSS_REV_816X) { +
RE: [PATCH 2/3] usb: musb: dsps: add phy control logic to glue
Hi, On Tue, Jul 10, 2012 at 11:35 AM, Gupta, Ajay Kumar ajay.gu...@ti.com wrote: Hi, On Mon, Jul 9, 2012 at 7:18 PM, Damodar Santhapuri x0132...@ti.com wrote: From: Ajay Kumar Gupta ajay.gu...@ti.com AM335x uses NOP transceiver driver and need to enable builtin PHY by writing into usb_ctrl register available in system control module register space. This is being added at musb glue driver layer untill a separate system control module driver is available. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Damodar Santhapuri x0132...@ti.com --- arch/arm/mach-omap2/board-ti8168evm.c |1 - arch/arm/mach-omap2/omap_phy_internal.c | 35 arch/arm/plat-omap/include/plat/usb.h |5 +- drivers/usb/musb/musb_dsps.c| 88 +-- 4 files changed, 74 insertions(+), 55 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach- omap2/board-ti8168evm.c index d4c8392..0c7c098 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -26,7 +26,6 @@ #include plat/usb.h static struct omap_musb_board_data musb_board_data = { - .set_phy_power = ti81xx_musb_phy_power, .interface_type = MUSB_INTERFACE_ULPI, .mode = MUSB_OTG, .power = 500, diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index d52651a..d80bb16 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -254,38 +254,3 @@ void am35x_set_mode(u8 musb_mode) omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); } - -void ti81xx_musb_phy_power(u8 on) -{ - void __iomem *scm_base = NULL; - u32 usbphycfg; - - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); - if (!scm_base) { - pr_err(system control module ioremap failed\n); - return; - } - - usbphycfg = __raw_readl(scm_base + USBCTRL0); - - if (on) { - if (cpu_is_ti816x()) { - usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; - usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; - } else if (cpu_is_ti814x()) { - usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN - | USBPHY_DPINPUT | USBPHY_DMINPUT); - usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN - | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL); - } - } else { - if (cpu_is_ti816x()) - usbphycfg = ~TI816X_USBPHY0_NORMAL_MODE; - else if (cpu_is_ti814x()) - usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; - - } - __raw_writel(usbphycfg, scm_base + USBCTRL0); - - iounmap(scm_base); -} diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat- omap/include/plat/usb.h index 548a4c8..c2aa4ae 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -95,7 +95,6 @@ extern void am35x_musb_reset(void); extern void am35x_musb_phy_power(u8 on); extern void am35x_musb_clear_irq(void); extern void am35x_set_mode(u8 musb_mode); -extern void ti81xx_musb_phy_power(u8 on); /* AM35x */ /* USB 2.0 PHY Control */ @@ -120,8 +119,8 @@ extern void ti81xx_musb_phy_power(u8 on); #define CONF2_DATPOL (1 1) /* TI81XX specific definitions */ -#define USBCTRL0 0x620 -#define USBSTAT0 0x624 +#define MUSB_USBSS_REV_816X0x9 +#define MUSB_USBSS_REV_814X0xb /* TI816X PHY controls bits */ #define TI816X_USBPHY0_NORMAL_MODE (1 0) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index 494772f..f7271c3 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -115,9 +115,46 @@ struct dsps_glue { struct platform_device *musb; /* child musb pdev */ const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */ struct timer_list timer;/* otg_workaround timer */ + u32 __iomem *usb_ctrl; + u8 usbss_rev; }; /** + * musb_dsps_phy_control - phy on/off + * @glue: struct dsps_glue * + * @on: flag for phy to be switched on or off + * + * This is to enable the PHY using usb_ctrl register in system control + * module space. + * + * XXX: This function will be removed once we have a seperate driver for %s/seperate/separate + * control module + */ +static void musb_dsps_phy_control(struct dsps_glue *glue, u8 on) +{ + u32
RE: [PATCH v1 09/11] drivers: usb: musb: Add device tree support for omap musb glue
Hi, Documentation/devicetree/bindings/usb/omap-usb.txt | 34 - drivers/usb/musb/omap2430.c| 52 [...] + of_property_read_u32(np, mode, (u32 *)pdata-mode); + of_property_read_u32(np, interface_type, + (u32 *)data-interface_type); + of_property_read_u32(np, num_eps, (u32 *)config- num_eps); + of_property_read_u32(np, ram_bits, (u32 *)config- ram_bits); + of_property_read_u32(np, mode, (u32 *)pdata-mode); 'mode' has already been read so this should be dropped. Ajay + of_property_read_u32(np, power, (u32 *)pdata-power); + config-multipoint = of_property_read_bool(np, multipoint); + + pdata-board_data = data; + pdata-config = config; + } + pdata-platform_ops = omap2430_ops; platform_set_drvdata(pdev, glue); @@ -597,12 +638,23 @@ static struct dev_pm_ops omap2430_pm_ops = { #define DEV_PM_OPS NULL #endif +#ifdef CONFIG_OF +static const struct of_device_id omap2430_id_table[] = { + { .compatible = ti,musb-omap2430 }, + {} +}; +MODULE_DEVICE_TABLE(of, omap2430_id_table); +#else +#define omap2430_id_table NULL +#endif + static struct platform_driver omap2430_driver = { .probe = omap2430_probe, .remove = __devexit_p(omap2430_remove), .driver = { .name = musb-omap2430, .pm = DEV_PM_OPS, + .of_match_table = omap2430_id_table, }, }; -- 1.7.5.4 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 4/7] mfd: omap: control: usb-phy: introduce the ctrl-module usb driver
Hi, On Thu, Jun 28, 2012 at 10:28 AM, Eduardo Valentin eduardo.valen...@ti.com wrote: Hello, On Wed, Jun 27, 2012 at 10:05:00PM +0400, Konstantin Baydarov wrote: Created a new platform driver for the platform device created by the control module mfd core, wrt usb. This driver has API's to power on/off the phy and the API's to write to musb mailbox. USB PHY on am335x platform also need to be enabled using phy control register in system control module register space. I think we should make this driver generic enough so that it can be used by am335x like platforms also to avoid duplication of such drivers. Ajay Changes since previous version: - Bandgap and usb phy: drivers are now independent from control module driver, they use their own functions to acess scm registers. - Parent SCM platform device IOMEM resources is used to get the base address of SCM window. - SCM Dependency was removed from Kconfig. - Bandgap and usb phy: Added private spinlocks for bandgap and usb drivers. (p.s. the mailbox for musb in omap4 is present in system control module) [kis...@ti.com: wrote the original API's related to USB functions] Signed-off-by: Konstantin Baydarov kbaida...@dev.rtsoft.ru Signed-off-by: Kishon Vijay Abraham I kis...@ti.com Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com --- drivers/usb/otg/Kconfig | 12 +++ drivers/usb/otg/Makefile | 1 + drivers/usb/otg/omap4-usb-phy.c | 170 + include/linux/usb/omap4_usb_phy.h | 53 4 files changed, 236 insertions(+), 0 deletions(-) create mode 100644 drivers/usb/otg/omap4-usb-phy.c create mode 100644 include/linux/usb/omap4_usb_phy.h diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig index 5c87db0..0ed691b 100644 --- a/drivers/usb/otg/Kconfig +++ b/drivers/usb/otg/Kconfig @@ -78,6 +78,18 @@ config TWL6030_USB are hooked to this driver through platform_data structure. The definition of internal PHY APIs are in the mach-omap2 layer. +config OMAP4_USB_PHY + tristate Texas Instruments OMAP4+ USB pin control driver + help + If you say yes here you get support for the Texas Instruments + OMAP4+ USB pin control driver. The register set is part of system + control module. + + USB phy in OMAP configures control module register for powering on + the phy, configuring VBUSVALID, AVALID, IDDIG and SESSEND. For + performing the above mentioned configuration, API's are added in + by this children of the control module driver. + config NOP_USB_XCEIV tristate NOP USB Transceiver Driver select USB_OTG_UTILS diff --git a/drivers/usb/otg/Makefile b/drivers/usb/otg/Makefile index 41aa509..60c8c83 100644 --- a/drivers/usb/otg/Makefile +++ b/drivers/usb/otg/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_USB_GPIO_VBUS) += gpio_vbus.o obj-$(CONFIG_ISP1301_OMAP) += isp1301_omap.o obj-$(CONFIG_TWL4030_USB) += twl4030-usb.o obj-$(CONFIG_TWL6030_USB) += twl6030-usb.o +obj-$(CONFIG_OMAP4_USB_PHY) += omap4-usb-phy.o obj-$(CONFIG_NOP_USB_XCEIV) += nop-usb-xceiv.o obj-$(CONFIG_USB_ULPI) += ulpi.o obj-$(CONFIG_USB_ULPI_VIEWPORT) += ulpi_viewport.o diff --git a/drivers/usb/otg/omap4-usb-phy.c b/drivers/usb/otg/omap4-usb-phy.c new file mode 100644 index 000..cbea2ea --- /dev/null +++ b/drivers/usb/otg/omap4-usb-phy.c @@ -0,0 +1,170 @@ +/* + * OMAP4 system control module driver, USB control children + * + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ + * + * Contact: + * Kishon Vijay Abraham I kis...@ti.com + * Eduardo Valentin eduardo.valen...@ti.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/module.h +#include linux/init.h +#include linux/gpio.h +#include linux/delay.h +#include linux/err.h +#include linux/platform_device.h +#include linux/usb/omap4_usb_phy.h + +void __iomem *omap_usb_phy_base; +spinlock_t omap_usb_phy_lock; + +static int omap_usb_phy_readl(u32 reg, u32 *val) +{ + if (!omap_usb_phy_base) + return -EINVAL; + + *val = __raw_readl(omap_usb_phy_base +
RE: [PATCH 2/3 v5] arm: omap: am335x: enable phy controls
Hi Tony, * Ajay Kumar Gupta ajay.gu...@ti.com [120207 23:10]: Switch on the phy for am335x. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/mach-omap2/omap_phy_internal.c | 21 ++--- 1 files changed, 14 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index 4c90477..7129408 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -266,7 +266,11 @@ void ti81xx_musb_phy_power(u8 on) void __iomem *scm_base = NULL; u32 usbphycfg; - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); + if (cpu_is_ti81xx()) + scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); + else if (cpu_is_am33xx()) + scm_base = ioremap(AM33XX_SCM_BASE, SZ_2K); + if (!scm_base) { pr_err(system control module ioremap failed\n); return; @@ -278,16 +282,19 @@ void ti81xx_musb_phy_power(u8 on) if (cpu_is_ti816x()) { usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; - } else if (cpu_is_ti814x()) { - usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN - | USBPHY_DPINPUT | USBPHY_DMINPUT); - usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN - | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL); + } else if (cpu_is_ti814x() || cpu_is_am33xx()) { + usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN); + usbphycfg |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN; + if (cpu_is_ti814x()) { + usbphycfg = ~(USBPHY_DPINPUT | USBPHY_DMINPUT); + usbphycfg |= USBPHY_DPOPBUFCTL + | USBPHY_DMOPBUFCTL; + } } } else { if (cpu_is_ti816x()) usbphycfg = ~TI816X_USBPHY0_NORMAL_MODE; - else if (cpu_is_ti814x()) + else if (cpu_is_ti814x() || cpu_is_am33xx()) usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; } Hmm I think I might have commented on this earlier.. Yes, you have already provided this comment and I need to post the updated patches. But anyways, please change the code so cpu_is_ tests are done only once during the init, not every time phy_power gets called. So initialize something in phy_init so things get set for the board that's booted. I will send the updated patches soon. Thanks, Ajay Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v5] arm: omap: am335x: enable phy controls
Hi, On Mon, Feb 13, 2012 at 04:26:19AM +, Gupta, Ajay Kumar wrote: Hi, * Ajay Kumar Gupta ajay.gu...@ti.com [120207 19:55]: Switch on the phy for am335x. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/mach-omap2/omap_phy_internal.c | 21 ++ --- 1 files changed, 14 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index 4c90477..7129408 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -266,7 +266,11 @@ void ti81xx_musb_phy_power(u8 on) void __iomem *scm_base = NULL; u32 usbphycfg; - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); + if (cpu_is_ti81xx()) + scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); + else if (cpu_is_am33xx()) + scm_base = ioremap(AM33XX_SCM_BASE, SZ_2K); + if (!scm_base) { pr_err(system control module ioremap failed\n); return; @@ -278,16 +282,19 @@ void ti81xx_musb_phy_power(u8 on) if (cpu_is_ti816x()) { usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; - } else if (cpu_is_ti814x()) { - usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN - | USBPHY_DPINPUT | USBPHY_DMINPUT); - usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN - | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL); + } else if (cpu_is_ti814x() || cpu_is_am33xx()) { + usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN); + usbphycfg |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN; + if (cpu_is_ti814x()) { + usbphycfg = ~(USBPHY_DPINPUT | USBPHY_DMINPUT); + usbphycfg |= USBPHY_DPOPBUFCTL + | USBPHY_DMOPBUFCTL; + } } } else { if (cpu_is_ti816x()) usbphycfg = ~TI816X_USBPHY0_NORMAL_MODE; - else if (cpu_is_ti814x()) + else if (cpu_is_ti814x() || cpu_is_am33xx()) usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; Please remove all the cpu_is_ calls from the *_phy_power functions. You should need those only in *_phy_init to set the right flags once, not every time you enable or disable the phy. Tony, This is a common function used by ti814x, ti816x and am335x and they have mostly the same bit map but a few bits are different so we would need to have cpu_is_. We use this *_phy_power(1) for phy on/init and *_phy_power(0) for phy off and don't have separate *_phy_init(). Even in that case we would require cpu_is_() as phy power down bit is different for ti816x and same for ti814x and am335x. Please let me know how should we handle this. use cpu_is_() to set a flag, and check on the flag. Are you saying save the flags once in a local static flag and use the same later on? Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v5] arm: omap: am335x: enable phy controls
Hi, * Ajay Kumar Gupta ajay.gu...@ti.com [120207 19:55]: Switch on the phy for am335x. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/mach-omap2/omap_phy_internal.c | 21 ++--- 1 files changed, 14 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index 4c90477..7129408 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -266,7 +266,11 @@ void ti81xx_musb_phy_power(u8 on) void __iomem *scm_base = NULL; u32 usbphycfg; - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); + if (cpu_is_ti81xx()) + scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); + else if (cpu_is_am33xx()) + scm_base = ioremap(AM33XX_SCM_BASE, SZ_2K); + if (!scm_base) { pr_err(system control module ioremap failed\n); return; @@ -278,16 +282,19 @@ void ti81xx_musb_phy_power(u8 on) if (cpu_is_ti816x()) { usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; - } else if (cpu_is_ti814x()) { - usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN - | USBPHY_DPINPUT | USBPHY_DMINPUT); - usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN - | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL); + } else if (cpu_is_ti814x() || cpu_is_am33xx()) { + usbphycfg = ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN); + usbphycfg |= USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN; + if (cpu_is_ti814x()) { + usbphycfg = ~(USBPHY_DPINPUT | USBPHY_DMINPUT); + usbphycfg |= USBPHY_DPOPBUFCTL + | USBPHY_DMOPBUFCTL; + } } } else { if (cpu_is_ti816x()) usbphycfg = ~TI816X_USBPHY0_NORMAL_MODE; - else if (cpu_is_ti814x()) + else if (cpu_is_ti814x() || cpu_is_am33xx()) usbphycfg |= USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN; Please remove all the cpu_is_ calls from the *_phy_power functions. You should need those only in *_phy_init to set the right flags once, not every time you enable or disable the phy. Tony, This is a common function used by ti814x, ti816x and am335x and they have mostly the same bit map but a few bits are different so we would need to have cpu_is_. We use this *_phy_power(1) for phy on/init and *_phy_power(0) for phy off and don't have separate *_phy_init(). Even in that case we would require cpu_is_() as phy power down bit is different for ti816x and same for ti814x and am335x. Please let me know how should we handle this. Regards, Ajay Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3 v5] usb: musb: Add support for ti81xx platform
Hi, On Wed, Feb 08, 2012 at 11:46:00AM +0530, Shubhrajyoti wrote: On Wednesday 08 February 2012 09:56 AM, Ajay Kumar Gupta wrote: +static int dsps_musb_init(struct musb *musb) { + struct device *dev = musb-controller; + struct musb_hdrc_platform_data *plat = dev-platform_data; + struct platform_device *pdev = to_platform_device(dev- parent); + struct dsps_glue *glue = platform_get_drvdata(pdev); + const struct dsps_musb_wrapper *wrp = glue-wrp; + struct omap_musb_board_data *data = plat-board_data; + void __iomem *reg_base = musb-ctrl_base; + u32 rev, val; + int status; + + /* mentor core register starts at offset of 0x400 from musb base */ + musb-mregs += wrp-musb_core_offset; + + /* NOP driver needs change if supporting dual instance */ + usb_nop_xceiv_register(); + musb-xceiv = otg_get_transceiver(); + if (!musb-xceiv) + return -ENODEV; + + status = pm_runtime_get_sync(dev); + if (status 0) { + dev_err(dev, pm_runtime_get_sync FAILED); + goto err0; + } So you are doing a get_sync at init and you cut the clocks at exit only. Any particular reason for that. This is a platform glue layer and main io paths are in musb_core.c/musb_host.c and musb_gadget.c. Host controller driver's bus_suspend/resume are in musb_host.c. What specific pm implementation are you looking for? The current patch calls pm_runtime_get_sync() for musb device and that is anyway getting done in common core file musb_core.c. Glue layer file need to call pm_runtime_get_sync() and x_put() for glue device during init and exit. I will resubmit this patch with above change. Ajay Ajay that's a very good question. We're just adding a brand new file, which is clean and stuff... so we might as well start with good PM capabilities. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3 v5] usb: musb: Add support for ti81xx platform
Hi, On Wed, Feb 08, 2012 at 11:46:00AM +0530, Shubhrajyoti wrote: On Wednesday 08 February 2012 09:56 AM, Ajay Kumar Gupta wrote: +static int dsps_musb_init(struct musb *musb) { + struct device *dev = musb-controller; + struct musb_hdrc_platform_data *plat = dev-platform_data; + struct platform_device *pdev = to_platform_device(dev-parent); + struct dsps_glue *glue = platform_get_drvdata(pdev); + const struct dsps_musb_wrapper *wrp = glue-wrp; + struct omap_musb_board_data *data = plat-board_data; + void __iomem *reg_base = musb-ctrl_base; + u32 rev, val; + int status; + + /* mentor core register starts at offset of 0x400 from musb base */ + musb-mregs += wrp-musb_core_offset; + + /* NOP driver needs change if supporting dual instance */ + usb_nop_xceiv_register(); + musb-xceiv = otg_get_transceiver(); + if (!musb-xceiv) + return -ENODEV; + + status = pm_runtime_get_sync(dev); + if (status 0) { + dev_err(dev, pm_runtime_get_sync FAILED); + goto err0; + } So you are doing a get_sync at init and you cut the clocks at exit only. Any particular reason for that. This is a platform glue layer and main io paths are in musb_core.c/musb_host.c and musb_gadget.c. Host controller driver's bus_suspend/resume are in musb_host.c. What specific pm implementation are you looking for? Ajay that's a very good question. We're just adding a brand new file, which is clean and stuff... so we might as well start with good PM capabilities. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx
Hi, On Mon, Oct 10, 2011 at 10:40:55AM +0530, Gupta, Ajay Kumar wrote: Hi, On Mon, Sep 26, 2011 at 03:21:07PM +0530, Gupta, Ajay Kumar wrote: To: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org; Balbi, Felipe; t...@atomide.com; B, Ravi; Cousson, Benoit; Gupta, Ajay Kumar Subject: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx Felipe, I am planning to send patch 1/6 along with next revision of ti81xx hwmod baseport patch from Hemant so that all hwmod data gets reviewed at one place. So now only rest 5 (2/6 to 6/6) patches needs to be queued for usb. I hope this will be fine with you. Let me know if you want me to post Same 5 patches separately. I guess it's better to queue them separately. I will only take whatever lies in drivers/usb/musb Ok fine so usb hwmod patch will come from Hemant in his hwmod baseport Series. I will resubmit the one patch fixing your comment. ok cool, tks. For you platform_data stuff (if you have any), you can add a header to include/linux/platform_data/musb-dsps.h I need to access struct omap_musb_board_data and so would add #include plat/usb.h in new file drivers/usb/musb/musb_dsps.h. Any comment ? Ajay Or something similar. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx
Hi, On Tue, Oct 11, 2011 at 02:13:07PM +0530, Gupta, Ajay Kumar wrote: Hi, On Mon, Oct 10, 2011 at 10:40:55AM +0530, Gupta, Ajay Kumar wrote: Hi, On Mon, Sep 26, 2011 at 03:21:07PM +0530, Gupta, Ajay Kumar wrote: To: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org; Balbi, Felipe; t...@atomide.com; B, Ravi; Cousson, Benoit; Gupta, Ajay Kumar Subject: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx Felipe, I am planning to send patch 1/6 along with next revision of ti81xx hwmod baseport patch from Hemant so that all hwmod data gets reviewed at one place. So now only rest 5 (2/6 to 6/6) patches needs to be queued for usb. I hope this will be fine with you. Let me know if you want me to post Same 5 patches separately. I guess it's better to queue them separately. I will only take whatever lies in drivers/usb/musb Ok fine so usb hwmod patch will come from Hemant in his hwmod baseport Series. I will resubmit the one patch fixing your comment. ok cool, tks. For you platform_data stuff (if you have any), you can add a header to include/linux/platform_data/musb-dsps.h I need to access struct omap_musb_board_data and so would add #include plat/usb.h in new file drivers/usb/musb/musb_dsps.h. Any comment ? It would be nice to move that platform_data from plat to linux/platform_data Yes, correct. But that would need change at below files. arch/arm/mach-omap2/board-4430sdp.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-am3517evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap3evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-rx51.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap4panda.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-ti8168evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap4pcm049.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/usb-musb.c:static void usb_musb_mux_init(struct omap_musb_board_data *board_data) arch/arm/mach-omap2/board-ti8148evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-am335xevm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-ti8148evm.c.orig:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap3evm.c.orig:static struct omap_musb_board_data musb_board_data = { drivers/usb/musb/am35x.c: drivers/usb/musb/musb_dsps.c drivers/usb/musb/omap2430.c: I think this can be done in a separate patch and let the ti81xx 6/6 Patch to remain same. The separate patch will replace #include plat/usb.h to #include platform_data/usb.h Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx
Hi, On Tue, Oct 11, 2011 at 02:13:07PM +0530, Gupta, Ajay Kumar wrote: Hi, On Mon, Oct 10, 2011 at 10:40:55AM +0530, Gupta, Ajay Kumar wrote: Hi, On Mon, Sep 26, 2011 at 03:21:07PM +0530, Gupta, Ajay Kumar wrote: To: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org; Balbi, Felipe; t...@atomide.com; B, Ravi; Cousson, Benoit; Gupta, Ajay Kumar Subject: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx Felipe, I am planning to send patch 1/6 along with next revision of ti81xx hwmod baseport patch from Hemant so that all hwmod data gets reviewed at one place. So now only rest 5 (2/6 to 6/6) patches needs to be queued for usb. I hope this will be fine with you. Let me know if you want me to post Same 5 patches separately. I guess it's better to queue them separately. I will only take whatever lies in drivers/usb/musb Ok fine so usb hwmod patch will come from Hemant in his hwmod baseport Series. I will resubmit the one patch fixing your comment. ok cool, tks. For you platform_data stuff (if you have any), you can add a header to include/linux/platform_data/musb-dsps.h I need to access struct omap_musb_board_data and so would add #include plat/usb.h in new file drivers/usb/musb/musb_dsps.h. Any comment ? It would be nice to move that platform_data from plat to linux/platform_data Yes, correct. But that would need change at below files. arch/arm/mach-omap2/board-4430sdp.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-am3517evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap3evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-rx51.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap4panda.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-ti8168evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap4pcm049.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/usb-musb.c:static void usb_musb_mux_init(struct omap_musb_board_data *board_data) arch/arm/mach-omap2/board-ti8148evm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-am335xevm.c:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-ti8148evm.c.orig:static struct omap_musb_board_data musb_board_data = { arch/arm/mach-omap2/board-omap3evm.c.orig:static struct omap_musb_board_data musb_board_data = { drivers/usb/musb/am35x.c: drivers/usb/musb/musb_dsps.c drivers/usb/musb/omap2430.c: I think this can be done in a separate patch and let the ti81xx 6/6 Patch to remain same. The separate patch will replace #include plat/usb.h to #include platform_data/usb.h cool, that can be done with a sed script ;-) Ok fine then I will send the refreshed 5 patch set for ti81xx and 3 patch set for am335x correcting your comments. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] ti814evm: Add support for musb interface
Hi Igor, On 10/11/11 13:26, Ajay Kumar Gupta wrote: From: Ravi Babu ravib...@ti.com Adding musb support in ti814 EVM board file. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Ravi Babu ravib...@ti.com Acked-by: Felipe Balbi ba...@ti.com --- arch/arm/mach-omap2/board-ti8148evm.c |9 + This file does not exist and probably will not exist in the future, see [1] and [2]. So this patch can be dropped. Your 3/8 patch provides the support for both boards. Thanks for this. Felipe/Tony, Please drop this 4/8 patch. Ajay [1] - http://www.spinics.net/lists/linux-omap/msg58348.html [2] - http://www.spinics.net/lists/arm-kernel/msg143915.html -- Regards, Igor. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/8] ti816evm: Add support for musb interface
Hi, On 10/11/11 13:26, Ajay Kumar Gupta wrote: From: Ravi Babu ravib...@ti.com Adding musb support in ti816 EVM board file. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Ravi Babu ravib...@ti.com Acked-by: Felipe Balbi ba...@ti.com This patch can result in a merge conflict, therefore I think this one should be taken through Tony's tree. Also you should work together with Hemant Pedanekar and someone of you should rebase on top of the other otherwise one of the patches (yours or Hemant's) possibly will not apply. Ok sure. I will submit the patches (v5) after creating it on top of Hemant's and Vaibhav's patches. Thanks, Ajay --- arch/arm/mach-omap2/board-ti8168evm.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach- omap2/board-ti8168evm.c index 2c243df..1f3d423 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -23,6 +23,14 @@ #include plat/irqs.h #include plat/board.h #include plat/common.h +#include plat/usb.h + +static struct omap_musb_board_data musb_board_data = { + .set_phy_power = ti81xx_musb_phy_power, + .interface_type = MUSB_INTERFACE_ULPI, + .mode = MUSB_OTG, + .power = 500, +}; static struct omap_board_config_kernel ti8168_evm_config[] __initdata = { }; @@ -33,6 +41,7 @@ static void __init ti8168_evm_init(void) omap_sdrc_init(NULL, NULL); omap_board_config = ti8168_evm_config; omap_board_config_size = ARRAY_SIZE(ti8168_evm_config); + usb_musb_init(musb_board_data); } static void __init ti8168_evm_map_io(void) -- Regards, Igor. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/8] ti816evm: Add support for musb interface
Hi, Adding musb support in ti816 EVM board file. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Ravi Babu ravib...@ti.com Acked-by: Felipe Balbi ba...@ti.com This patch can result in a merge conflict, therefore I think this one should be taken through Tony's tree. Also you should work together with Hemant Pedanekar and someone of you should rebase on top of the other otherwise one of the patches (yours or Hemant's) possibly will not apply. Ok sure. I will submit the patches (v5) after creating it on top of Hemant's and Vaibhav's patches. Here is the v5 of this patch created against latest Hemant's patch set. From 3006845992157acc4a2b44d0be119e22bb34d1f4 Mon Sep 17 00:00:00 2001 From: Ravi Babu ravib...@ti.com Date: Wed, 17 Aug 2011 17:58:02 +0530 Subject: [PATCH 3/8 v5] ti816evm: Add support for musb interface Adding musb support in ti816 EVM board file. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Signed-off-by: Ravi Babu ravib...@ti.com Acked-by: Felipe Balbi ba...@ti.com --- arch/arm/mach-omap2/board-ti8168evm.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach-omap2/board-ti8168evm.c index b858921..cab909f 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -23,6 +23,14 @@ #include plat/irqs.h #include plat/board.h #include plat/common.h +#include plat/usb.h + +static struct omap_musb_board_data musb_board_data = { + .set_phy_power = ti81xx_musb_phy_power, + .interface_type = MUSB_INTERFACE_ULPI, + .mode = MUSB_OTG, + .power = 500, +}; static struct omap_board_config_kernel ti81xx_evm_config[] __initdata = { }; @@ -33,6 +41,7 @@ static void __init ti81xx_evm_init(void) omap_sdrc_init(NULL, NULL); omap_board_config = ti81xx_evm_config; omap_board_config_size = ARRAY_SIZE(ti81xx_evm_config); + usb_musb_init(musb_board_data); } MACHINE_START(TI8168EVM, ti8168evm) -- 1.6.2.4 Thanks, Ajay --- arch/arm/mach-omap2/board-ti8168evm.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach- omap2/board-ti8168evm.c index 2c243df..1f3d423 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -23,6 +23,14 @@ #include plat/irqs.h #include plat/board.h #include plat/common.h +#include plat/usb.h + +static struct omap_musb_board_data musb_board_data = { + .set_phy_power = ti81xx_musb_phy_power, + .interface_type = MUSB_INTERFACE_ULPI, + .mode = MUSB_OTG, + .power = 500, +}; static struct omap_board_config_kernel ti8168_evm_config[] __initdata = { }; @@ -33,6 +41,7 @@ static void __init ti8168_evm_init(void) omap_sdrc_init(NULL, NULL); omap_board_config = ti8168_evm_config; omap_board_config_size = ARRAY_SIZE(ti8168_evm_config); + usb_musb_init(musb_board_data); } static void __init ti8168_evm_map_io(void) -- Regards, Igor. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] usb: musb: enable support for am335x
Hi, On Mon, Sep 26, 2011 at 02:47:11PM +0400, Sergei Shtylyov wrote: Hello. On 26-09-2011 14:03, Ajay Kumar Gupta wrote: Enabled the flag so that musb_dsps glue file can be used for am335x Signed-off-by: Ajay Kumar Guptaajay.gu...@ti.com --- drivers/usb/musb/Kconfig |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index d6abdec..4e21aee 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -8,7 +8,7 @@ config USB_MUSB_HDRC depends on USB USB_GADGET depends on (ARM || (BF54x !BF544) || (BF52x !BF522 !BF523)) select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN) - select NOP_USB_XCEIV if SOC_OMAPTI81XX + select NOP_USB_XCEIV if (SOC_OMAPTI81XX || SOC_OMAPAM33XX) select TWL4030_USB if MACH_OMAP_3430SDP select TWL6030_USB if MACH_OMAP_4430SDP || MACH_OMAP4_PANDA select USB_OTG_UTILS @@ -57,7 +57,7 @@ config USB_MUSB_AM35X config USB_MUSB_DSPS tristate TI DSPS platforms - depends on SOC_OMAPTI81XX + depends on (SOC_OMAPTI81XX || SOC_OMAPAM33XX) Parens not needed here and above. are we getting a revised version ? Yes, I will send the revised version with earlier set fixing your comment. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx
Hi, On Mon, Sep 26, 2011 at 03:21:07PM +0530, Gupta, Ajay Kumar wrote: To: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org; Balbi, Felipe; t...@atomide.com; B, Ravi; Cousson, Benoit; Gupta, Ajay Kumar Subject: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx Felipe, I am planning to send patch 1/6 along with next revision of ti81xx hwmod baseport patch from Hemant so that all hwmod data gets reviewed at one place. So now only rest 5 (2/6 to 6/6) patches needs to be queued for usb. I hope this will be fine with you. Let me know if you want me to post Same 5 patches separately. I guess it's better to queue them separately. I will only take whatever lies in drivers/usb/musb Ok fine so usb hwmod patch will come from Hemant in his hwmod baseport Series. I will resubmit the one patch fixing your comment. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx
Hi, To: linux-omap@vger.kernel.org Cc: linux-...@vger.kernel.org; Balbi, Felipe; t...@atomide.com; B, Ravi; Cousson, Benoit; Gupta, Ajay Kumar Subject: [PATCH 1/6 v3] omap: musb: Adding hwmod data for ti81xx Felipe, I am planning to send patch 1/6 along with next revision of ti81xx hwmod baseport patch from Hemant so that all hwmod data gets reviewed at one place. So now only rest 5 (2/6 to 6/6) patches needs to be queued for usb. I hope this will be fine with you. Let me know if you want me to post Same 5 patches separately. Ajay From: Ravi Babu ravib...@ti.com [...] -- 1.6.2.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/6 v2] omap: musb: ti81xx: Add phy power function
Hi, On 07-09-2011 21:02, Ajay Kumar Gupta wrote: Adding ti81xx_musb_phy_power() which will be used by musb driver through its function pointer in board_data. Signed-off-by: Ajay Kumar Guptaajay.gu...@ti.com Signed-off-by: Ravi Baburavib...@ti.com --- arch/arm/mach-omap2/omap_phy_internal.c | 24 +++ arch/arm/plat-omap/include/plat/usb.h | 32 +++ 2 files changed, 56 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach- omap2/omap_phy_internal.c index 58775e3..352b6af 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -260,3 +260,27 @@ void am35x_set_mode(u8 musb_mode) omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); } + +void ti81xx_musb_phy_power(u8 on) +{ + u32 usbphycfg = omap_ctrl_readl(USBCTRL0); + + if (on) { + if (cpu_is_ti816x()) { + usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; + usbphycfg = ~TI816X_USBPHY_REFCLK_OSC; + } else if (cpu_is_ti814x()) { + usbphycfg= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN + | USBPHY_DPINPUT | USBPHY_DMINPUT); + usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN + | USBPHY_DPOPBUFCTL | USBPHY_DPOPBUFCTL); Same #define repeated twice? Good catch.. thanks. One of them should be USBPHY_DMOPBUFCTL, will fix in v3. Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/6 v2] omap: musb: Adding hwmod data for ti81xx
Hi, ajay.gu...@ti.com wrote: [...] -- 1.6.2.4 looks good to me. how about you reply with your Acked-by then ? -- balbi yes balbi, here it is Acked-By: Keshava Munegowda keshava_mgo...@ti.com Tony and Benoit, Please review this patch and provide your input on this. Thanks, Ajay -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/6 v2] usb: musb: Add support for ti81xx platform
Hi, +static int __devinit dsps_probe(struct platform_device *pdev) { + const struct platform_device_id *id = platform_get_device_id(pdev); + const struct dsps_musb_wrapper *wrp = + (struct dsps_musb_wrapper *)id-driver_data; + struct dsps_glue *glue; + struct resource *iomem; + int ret; + + /* allocate glue */ + glue = kzalloc(sizeof(*glue), GFP_KERNEL); + if (!glue) { + dev_err(pdev-dev, unable to allocate glue memory\n); + ret = -ENOMEM; + goto err0; + } + + /* get memory resource */ + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!iomem) { + dev_err(pdev-dev, failed to get usbss mem resourse\n); + ret = -ENODEV; + goto err1; + } + + glue-dev = pdev-dev; + glue-wrp = wrp; wrp is marked __devinitconst, so I guess you need to copy it here, instead of just pointing to it. Will fix in v3. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/6] usb: musb: Add support for ti81xx platform
Hi, diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 20a2873..07f3faf 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1014,7 +1014,9 @@ static void musb_shutdown(struct platform_device *pdev) || defined(CONFIG_USB_MUSB_OMAP2PLUS) \ || defined(CONFIG_USB_MUSB_OMAP2PLUS_MODULE)\ || defined(CONFIG_USB_MUSB_AM35X) \ - || defined(CONFIG_USB_MUSB_AM35X_MODULE) + || defined(CONFIG_USB_MUSB_AM35X_MODULE)\ + || defined(CONFIG_USB_MUSB_TI81XX) \ + || defined(CONFIG_USB_MUSB_TI81XX_MODULE) we really need to find a better way to handle this :-( We use to have 'fifo_mode' within 'struct musb_platform_ops' so shouldn't It be brought back to fix this? Thanks, Ajay diff --git a/drivers/usb/musb/ti81xx.c b/drivers/usb/musb/ti81xx.c new file mode 100644 index 000..f95774e --- /dev/null +++ b/drivers/usb/musb/ti81xx.c I still think this should be part of am35x.c It really look like they are very similar. on monday I'll read the rest of the file. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/6] usb: musb: Add support for ti81xx platform
Hi, If you compare am35x and da8x then you would find that they are mostly same and so should be merged. They both have one musb instances. Ti81xx has two musb interface and huge differences in register map as listed Above so ti81xx should be in a separate file. Apart from this, am35 and da8x is not yet runtime pm compatible but the current patches supporting ti81x is runtime pm compliant. I think da8x and am35x files should be merged in a separate patch once they are tested after the merge. Sure, let's try to merge them as soon as possible. Ok. Now, about this one. I still think the wrapper is essentially the same, but the memory map is different, right ? Yes, except that usbss related wrappers are completely new in ti81xx and They are not present in am35x or da8xx or davinci series platforms. I mean, the programming model of the wrapper and its features are the same, correct ? Yes. So, how about you abstract the memory map by passing the address offsets via driver_data ? This sounds to be a cleaner way of doing this. Let me work on this and propose something. The thing is that this is all duplicated code in a sense. Although the register map is different, the HW block is the same, isn't it ? Yes. Just a few tweaks to support multiple musb instances. Except that the complete new usbss wrapper. In that case, I'm not very keen on letting yet another very similar file to be added. Let me collect all the details and discuss further on this. How about you add a very clean musb-dsps.c file (btw, at some point, I want all glue layers to have musb- prepended to them) which, for now, only handles this new chip, but it has memory map passed in via driver_data ? Then it should be very simple to convert davinci, da8xx and am35x to the new file. Ok. You will probably need to revisit the other device's TRM to see what needs to be abstracted or flaged somehow. What is board-specific and what is silicon-specific and that sort of thing. But it will be a very nice exercise and the output will be the deletion of three very similar files. Ok. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/6] usb: musb: Add support for ti81xx platform
Hi, we really need to find a better way to handle this :-( How about passing the mode info from musb_config (musb-config- fifo_mode), same way as musb-config-fifo_cfg not sure... Ideally we wouldn't really need those fifo tables, Yes, if all the board files provide their own fifo_cfg. but I'm still thinking how to properly do that. Infact I think, we can kill musb-config-fifo_cfg and have only musb-config-mode as in this way all the fifo tables will be at one place and should help in maintenance. The idea of musb-config-fifo_cfg was to allow boards to pass their optimal fifo table. If a certain product, knows it will only support mass storage, why not letting them enable only those needed endpoints in an optimized way (with double buffering, bulk combine/split, etc). Ok fine, understood. Wrapper registers are different but I will see on this and provide you details On how the merged file would look like. I am afraid that we may have to use Either #ifdef or cpu_is_xxx(). we can use platform_device_id to cope with the differences :-) See how I'm using it on dwc3 driver (Greg's usb-next branch). Please refer the usb section in below TRMs. da8x (aka. am1x) at http://www.ti.com/lit/ug/sprufw6b/sprufw6b.pdf am35x at http://www.ti.com/lit/ug/sprugr0b/sprugr0b.pdf ti81x at http://www.ti.com/lit/ug/sprugx8/sprugx8.pdf I have compiled the differences in the register map between am35x and ti81xx and they are listed below. === Registeram35x [has one musb]ti81xx [two musb interface] === REVISION0x000x - for USBSS 0x1000 - for musb0 0x1800 - for musb1 ALL USBSS registers Not present Available from offset 0x-0x0FFC USB_CTRL_REG0x040x1014 - for musb0 0x1814 - for musb1 USB_STAT_REG0x080x1018 - for musb0 0x1818 - for musb1 USB_EMULATION_REG 0x0cNot present USB_AUTOREQ_REG 0x14Not present USB_SRP_FIX_TIME_REG0x18Not present USB_TEARDOWN_REG0x1cNot present USB_IRQ_MERGED_STATUS Present as part of 0x1020 - for musb0 EP_INTR_x or CORE_INTR_x0x1820 - for musb1 register in am35x but at different offsets USB_IRQ_EOI Same as above 0x1024 - for musb0 0x1824 - for musb1 USB_IRQ_STATUS_RAW_0Same as above 0x1028 - for musb0 0x1828 - for musb1 USB_IRQ_STATUS_RAW_1Same as above 0x102c - for musb0 0x182c - for musb1 USB_IRQ_STATUS_0Same as above 0x1030 - for musb0 0x1830 - for musb1 USB_IRQ_STATUS_1Same as above 0x1034 - for musb0 0x1834 - for musb1 USB_IRQ_ENABLE_SET_0Same as above 0x1038 - for musb0 0x1838 - for musb1 USB_IRQ_ENABLE_SET_1Same as above 0x103c - for musb0 0x183c - for musb1 USB_IRQ_ENABLE_CLR_0Same as above 0x1040 - for musb0 0x1840 - for musb1 USB_IRQ_ENABLE_CLR_1Same as above 0x1044 - for musb0 0x1844 - for musb1 EP_INTR_SRC_REG 0x20Present as part of USB_IRQ_x register in ti81xx but at different offsets EP_INTR_SRC_SET_REG 0x24Same as above EP_INTR_SRC_CLEAR_REG 0x28Same as above EP_INTR_MASK_REG0x2c
RE: [PATCH 6/6] usb: musb: Add support for ti81xx platform
Hi, On Fri, Aug 26, 2011 at 04:41:48PM +0530, Ajay Kumar Gupta wrote: @@ -57,6 +58,10 @@ config USB_MUSB_AM35X tristate AM35x depends on ARCH_OMAP +config USB_MUSB_TI81XX + bool TI81XX + depends on SOC_OMAPTI81XX this *must* be tristate. I can't emphasize enough how important this is. Ok. diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 20a2873..07f3faf 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1014,7 +1014,9 @@ static void musb_shutdown(struct platform_device *pdev) || defined(CONFIG_USB_MUSB_OMAP2PLUS) \ || defined(CONFIG_USB_MUSB_OMAP2PLUS_MODULE)\ || defined(CONFIG_USB_MUSB_AM35X) \ - || defined(CONFIG_USB_MUSB_AM35X_MODULE) + || defined(CONFIG_USB_MUSB_AM35X_MODULE)\ + || defined(CONFIG_USB_MUSB_TI81XX) \ + || defined(CONFIG_USB_MUSB_TI81XX_MODULE) we really need to find a better way to handle this :-( How about passing the mode info from musb_config (musb-config-fifo_mode), same way as musb-config-fifo_cfg Infact I think, we can kill musb-config-fifo_cfg and have only musb-config-mode as in this way all the fifo tables will be at one place and should help in maintenance. diff --git a/drivers/usb/musb/ti81xx.c b/drivers/usb/musb/ti81xx.c new file mode 100644 index 000..f95774e --- /dev/null +++ b/drivers/usb/musb/ti81xx.c I still think this should be part of am35x.c It really look like they are very similar. Wrapper registers are different but I will see on this and provide you details On how the merged file would look like. I am afraid that we may have to use Either #ifdef or cpu_is_xxx(). on monday I'll read the rest of the file. Ok. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/6] ti816evm: Add support for musb interface
Hi, On 26-08-2011 15:11, Ajay Kumar Gupta wrote: From: Ravi B ravib...@ti.com Adding musb support in ti816 EVM board file. Signed-off-by: Ajay Kumar Guptaajay.gu...@ti.com Signed-off-by: Ravi Bravib...@ti.com As far as I know, full name is required in signoff. Ok. --- arch/arm/mach-omap2/board-ti8168evm.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-ti8168evm.c b/arch/arm/mach- omap2/board-ti8168evm.c index e516a04..77200d4 100644 --- a/arch/arm/mach-omap2/board-ti8168evm.c +++ b/arch/arm/mach-omap2/board-ti8168evm.c @@ -23,6 +23,20 @@ #includeplat/irqs.h #includeplat/board.h #includeplat/common.h +#includeplat/usb.h + +static struct omap_musb_board_data musb_board_data = { + .set_phy_power = ti81xx_musb_phy_power, + .interface_type = MUSB_INTERFACE_ULPI, +#ifdef CONFIG_USB_MUSB_OTG + .mode = MUSB_OTG, +#elif defined(CONFIG_USB_MUSB_HDRC_HCD) + .mode = MUSB_HOST, +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC) + .mode = MUSB_PERIPHERAL, +#endif This #ifdef'ery is not longer necessary afetr this patch: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- 2.6.git;a=commit;h=75a14b1434a0ca409bcc8ab9b6b2e680796c487e Got it, so it should be only. + .mode = MUSB_OTG Due to the below patch: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=622859634a663c5e55d0e2a2cdbb55ac058d97b3 The .mode is still being passed to musb-board_mode which is used for below definitions. #define is_peripheral_enabled(musb) ((musb)-board_mode != MUSB_HOST) #define is_host_enabled(musb) ((musb)-board_mode != MUSB_PERIPHERAL) #define is_otg_enabled(musb)((musb)-board_mode == MUSB_OTG) Thanks, Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] OMAP3EVM: ehci: Fix EHCI support
Hi, Set the VAUX2 regulator supply to 1.8V for the HSUSB host interface. Gpio 2 of the TPS65950 has to be set to zero in order to enable the HSUBS2 clock. TPS65950 GPIO2 is not to enable HSUSB2 clock but to enable chip u131 On omap3evm (rev-G) which latches USB, camera and audio lines. Signed-off-by: Bryan DE FARIA bdefa...@adeneo-embedded.com --- arch/arm/mach-omap2/board-omap3evm.c | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- omap2/board-omap3evm.c index c452b3f..13a2b71 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -377,6 +377,10 @@ static int omap3evm_twl_gpio_setup(struct device *dev, if (r) printk(KERN_ERR failed to get/set lcd_bkl gpio\n); + /* gpio + 2 == HSUSB2 Clock Enable */ + if (get_omap3_evm_rev() = OMAP3EVM_BOARD_GEN_2) + gpio_request_one(gpio + 2, GPIOF_OUT_INIT_LOW, EN_HSUSB2_CLK); + Did you test EHCI port with this change ? Ajay /* gpio + 7 == DVI Enable */ gpio_request_one(gpio + 7, GPIOF_OUT_INIT_LOW, EN_DVI); [...] rm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] OMAP3EVM: ehci: Fix EHCI support
Hi, TPS65950 GPIO2 is to choose through u131 if we drive the lines to the evm mother board or to the expansion connector p18. Should not it be default to the mother board? It's not enabled by default and that’s why we need to enable it through Tps65950 GPIO2. Your patch looks fine but the comment and name needs to be changed as There is no clock which gets enabled using gpio2. All the HSUSB clocks Are handled inside driver at drivers/usb/host/ehci-omap.c Ajay The EHCI port on the mother board works with this patch. Bryan Hi, Set the VAUX2 regulator supply to 1.8V for the HSUSB host interface. Gpio 2 of the TPS65950 has to be set to zero in order to enable the HSUBS2 clock. TPS65950 GPIO2 is not to enable HSUSB2 clock but to enable chip u131 On omap3evm (rev-G) which latches USB, camera and audio lines. Signed-off-by: Bryan DE FARIA bdefa...@adeneo-embedded.com --- arch/arm/mach-omap2/board-omap3evm.c | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- omap2/board-omap3evm.c index c452b3f..13a2b71 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -377,6 +377,10 @@ static int omap3evm_twl_gpio_setup(struct device *dev, if (r) printk(KERN_ERR failed to get/set lcd_bkl gpio\n); + /* gpio + 2 == HSUSB2 Clock Enable */ + if (get_omap3_evm_rev() = OMAP3EVM_BOARD_GEN_2) + gpio_request_one(gpio + 2, GPIOF_OUT_INIT_LOW, EN_HSUSB2_CLK); + Did you test EHCI port with this change ? Ajay /* gpio + 7 == DVI Enable */ gpio_request_one(gpio + 7, GPIOF_OUT_INIT_LOW, EN_DVI); [...] rm-kernel
RE: [PATCH v2] OMAP3EVM: ehci: Fix EHCI support
Hi, For the comment what about: Set the VAUX2 regulator supply to 1.8V for the HSUSB host interface. Tps65950 GPIO2 has to be set to zero in order to enable the EHCI select line. Looks fine. I see this patch as a fix for the EHCI support, what would you suggest for the patch name? This is also fine. Ajay Bryan Hi, TPS65950 GPIO2 is to choose through u131 if we drive the lines to the evm mother board or to the expansion connector p18. Should not it be default to the mother board? It's not enabled by default and that’s why we need to enable it through Tps65950 GPIO2. Your patch looks fine but the comment and name needs to be changed as There is no clock which gets enabled using gpio2. All the HSUSB clocks Are handled inside driver at drivers/usb/host/ehci-omap.c Ajay The EHCI port on the mother board works with this patch. Bryan Hi, Set the VAUX2 regulator supply to 1.8V for the HSUSB host interface. Gpio 2 of the TPS65950 has to be set to zero in order to enable the HSUBS2 clock. TPS65950 GPIO2 is not to enable HSUSB2 clock but to enable chip u131 On omap3evm (rev-G) which latches USB, camera and audio lines. Signed-off-by: Bryan DE FARIA bdefa...@adeneo-embedded.com --- arch/arm/mach-omap2/board-omap3evm.c | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- omap2/board-omap3evm.c index c452b3f..13a2b71 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -377,6 +377,10 @@ static int omap3evm_twl_gpio_setup(struct device *dev, if (r) printk(KERN_ERR failed to get/set lcd_bkl gpio\n); + /* gpio + 2 == HSUSB2 Clock Enable */ + if (get_omap3_evm_rev() = OMAP3EVM_BOARD_GEN_2) + gpio_request_one(gpio + 2, GPIOF_OUT_INIT_LOW, EN_HSUSB2_CLK); + Did you test EHCI port with this change ? Ajay /* gpio + 7 == DVI Enable */ gpio_request_one(gpio + 7, GPIOF_OUT_INIT_LOW, EN_DVI); [...] rm-kernel
RE: [PATCH v3] OMAP3EVM: ehci: Fix EHCI support
Hi, [...] arch/arm/mach-omap2/board-omap3evm.c | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- omap2/board-omap3evm.c index c452b3f..13a2b71 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -377,6 +377,10 @@ static int omap3evm_twl_gpio_setup(struct device *dev, if (r) printk(KERN_ERR failed to get/set lcd_bkl gpio\n); + /* gpio + 2 == HSUSB2 Clock Enable */ This comment should be modified. gpio+2 not only enables EHCI but also camera and Audio lines. + if (get_omap3_evm_rev() = OMAP3EVM_BOARD_GEN_2) + gpio_request_one(gpio + 2, GPIOF_OUT_INIT_LOW, EN_HSUSB2_CLK); This name EN_HSUSB2_CLK should also be changed. Ajay + /* gpio + 7 == DVI Enable */ gpio_request_one(gpio + 7, GPIOF_OUT_INIT_LOW, EN_DVI); @@ -450,6 +454,25 @@ static struct regulator_init_data omap3evm_vio = { .consumer_supplies = omap3evm_vio_supply, }; +/* VAUX2 for EHCI */ +static struct regulator_consumer_supply omap3evm_vaux2_supplies[] = { + REGULATOR_SUPPLY(hsusb1, ehci-omap.0), +}; + +static struct regulator_init_data omap3evm_vaux2 = { + .constraints = { + .min_uV = 180, + .max_uV = 180, + .apply_uV = true, + .valid_modes_mask = REGULATOR_MODE_NORMAL + | REGULATOR_MODE_STANDBY, + .valid_ops_mask = REGULATOR_CHANGE_MODE + | REGULATOR_CHANGE_STATUS, + }, + .num_consumer_supplies = ARRAY_SIZE(omap3evm_vaux2_supplies), + .consumer_supplies = omap3evm_vaux2_supplies, +}; + #ifdef CONFIG_WL12XX_PLATFORM_DATA #define OMAP3EVM_WLAN_PMENA_GPIO (150) @@ -510,6 +533,8 @@ static int __init omap3_evm_i2c_init(void) omap3evm_twldata.vdac-constraints.apply_uV = true; omap3evm_twldata.vpll2-constraints.apply_uV = true; + if (get_omap3_evm_rev() = OMAP3EVM_BOARD_GEN_2) + omap3evm_twldata.vaux2 = omap3evm_vaux2; omap3_pmic_init(twl4030, omap3evm_twldata); omap_register_i2c_bus(2, 400, NULL, 0); -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 1/3] ARM: add CPPI 4.1 DMA support
Hi, Add support for Texas Instuments Communication Port Programming Interface 4.1 (CPPI 4.1) used on OMAP-L1x/DA8xx and AM35x. At this moment, only the DMA controller and queue manager are supported. Support for the buffer manager is lacking but these chips don't have it anyway. Signed-off-by: Sergei Shtylyovsshtyl...@ru.mvista.com Signed-off-by: Sekhar Norinsek...@ti.com Russell, you have recently discarded this patch from your patch system. Can we know the reason? It was recently discussed with TI, and various people raised concerns about it. I don't remember the exact details, and I wish they'd raise them with the patch author directly. It may have been that it's inventing its own API rather than using something like the DMA engine API. Adding linux-omap to try to get a response there. Sergei, This issue was discussed recently at TI and proposal was to place it to drivers/dma folder. Moreover, even Felipe also seems to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma. Regards, Ajay ___ Davinci-linux-open-source mailing list davinci-linux-open-sou...@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 1/3] ARM: add CPPI 4.1 DMA support
Hi, Add support for Texas Instuments Communication Port Programming Interface 4.1 (CPPI 4.1) used on OMAP-L1x/DA8xx and AM35x. At this moment, only the DMA controller and queue manager are supported. Support for the buffer manager is lacking but these chips don't have it anyway. Signed-off-by: Sergei Shtylyovsshtyl...@ru.mvista.com Signed-off-by: Sekhar Norinsek...@ti.com Russell, you have recently discarded this patch from your patch system. Can we know the reason? It was recently discussed with TI, and various people raised concerns about it. I don't remember the exact details, and I wish they'd raise them with the patch author directly. It may have been that it's inventing its own API rather than using something like the DMA engine API. Adding linux-omap to try to get a response there. Sergei, This issue was discussed recently at TI and proposal was to place it to drivers/dma folder. Note that I have neither time nor inclination to do this work (not do I think it's even feasible), so it will have to fall on TI's shoulders... This is fine. No issues. We will discuss this at length at linux-usb list. Moreover, even Felipe also seems to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma. Frankly speaking, I doubt that drivers/dma/ will have place for the purely MUSB specific DMA engines such as the named ones (there's no TUSB DMA BTW it uses OMAP DMA). I think we will get more clarity once we start on this activity. Thanks, Ajay Regards, Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
Hi, Is AM35x that different ? Can't you just write to MUSB_INTRRX MUSB_INTRTX and MUSB_INTRUSB ?? Writing to MUSB_INTRRX/TX/USB wouldn't help. We have to clear interrupt Bit in control register INTR_LVL_CLR. I see, thanks for the info :-) Felipe, I have recreated this patch (attached) on top of your patch set and AM35x is working. Thanks, Ajay -- balbi 0001-musb-am35x-fix-compile-error-due-to-control-apis.patch Description: 0001-musb-am35x-fix-compile-error-due-to-control-apis.patch
RE: [PATCH 1/2] AM35x: musb: fix compilation error
On Fri, Dec 03, 2010 at 07:08:53PM +0530, Gupta, Ajay Kumar wrote: We already have generic APIs so I think we can pass function pointers to musb driver via struct omap_musb_board_data, struct omap_musb_board_data { + void(*phy_on) (void) + void(*phy_off) (void) + void (*intr_clr) (void) } Reset part can be done in board file itself same as Ethernet driver is doing. Does this look fine? so those would be turn phy on, turn phy off and clear interrupt apis ? How about: int (*set_phy_power)(unsigned on); Looks good. void (*clear_phy_irq)(void); This is actually musb ip interrupt clear so I will make it like, void (*clear_irq) (void); Is AM35x that different ? Can't you just write to MUSB_INTRRX MUSB_INTRTX and MUSB_INTRUSB ?? Writing to MUSB_INTRRX/TX/USB wouldn't help. We have to clear interrupt Bit in control register INTR_LVL_CLR. -Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] musb: am35x: fix compile error due to control apis
Hi, As the control.h have been moved to new location and it's uses are not allowed to drivers directly so moving the phy control, interrupt clear and reset functionality to board files. I'm not fond of the whole approach. I'm not sure why accesses to the control registers are such a no-no, taking into account that one needs to access such regisyter to clear the interrupt... I think Tony is the right person to answer this :) Signed-off-by: Ajay Kumar Guptaajay.gu...@ti.com [...] diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb- musb.c [...] + regval= ~AM35XX_USBOTGSS_SW_RST; + omap_ctrl_writel(regval, AM35XX_CONTROL_IP_SW_RESET); + + regval = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); Why read it and ignore the result? This is due to recommendation for OMAPs as discussed at below Link, http://markmail.org/message/s3lp7xlyq7zxnjtc#query:+page:1+mid:kfia4ld4xgzek6kq+state:results Thanks, Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] musb: am35x: fix compile error due to control apis
As the control.h have been moved to new location and it's uses are not allowed to drivers directly so moving the phy control, interrupt clear and reset functionality to board files. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- Patch created against today's linus tree. I hope this is a short-term fix only; otherwise you will need to duplicate this code in all boards that use an AM3517. most of the stuff are in usb-musb.c so can be reused by all the boards. Only the musb reset part is added in board file. Ajay - Anand -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] musb: am35x: fix compile error due to control apis
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb- musb.c index 7260558..1f32fdb 100644 --- a/arch/arm/mach-omap2/usb-musb.c +++ b/arch/arm/mach-omap2/usb-musb.c @@ -33,6 +33,82 @@ #ifdef CONFIG_USB_MUSB_SOC +static void am35x_musb_phy_power(u8 on) +{ + unsigned long timeout = jiffies + msecs_to_jiffies(100); + u32 devconf2; + + if (on) { + /* +* Start the on-chip PHY and its PLL. +*/ + devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); + + devconf2 = ~(CONF2_RESET | CONF2_PHYPWRDN | CONF2_OTGPWRDN); + devconf2 |= CONF2_PHY_PLLON; + + omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); + + printk(KERN_INFO Waiting for PHY clock good...\n); pr_info(). This file already uses printk() so used it for uniformity. + while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) +CONF2_PHYCLKGD)) { + cpu_relax(); + + if (time_after(jiffies, timeout)) { + printk(KERN_ERR musb PHY clock good timed out\n); pr_err(). diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat- omap/include/plat/usb.h index 59c7fe7..4299097 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -69,6 +69,9 @@ struct omap_musb_board_data { u8 mode; u16 power; unsigned extvbus:1; + void(*set_phy_power) (u8 on); + void(*clear_irq) (void); + void(*set_mode) (u8 mode); Should be no spaces between ) and (. scripts/checkpatch.pl used to complain about such things, now it's silent though... Yes, I tried with ./scripts/checkpatch and didn't get any error/warning. @@ -364,37 +324,21 @@ eoi: int musb_platform_set_mode(struct musb *musb, u8 musb_mode) { - u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); + struct device *dev = musb-controller; + struct musb_hdrc_platform_data *plat = dev-platform_data; + struct omap_musb_board_data *data = plat-board_data; - devconf2 = ~CONF2_OTGMODE; - switch (musb_mode) { -#ifdef CONFIG_USB_MUSB_HDRC_HCD - case MUSB_HOST: /* Force VBUS valid, ID = 0 */ - devconf2 |= CONF2_FORCE_HOST; - break; -#endif -#ifdef CONFIG_USB_GADGET_MUSB_HDRC - case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */ - devconf2 |= CONF2_FORCE_DEVICE; - break; -#endif -#ifdef CONFIG_USB_MUSB_OTG - case MUSB_OTG: /* Don't override the VBUS/ID comparators */ - devconf2 |= CONF2_NO_OVERRIDE; - break; -#endif - default: - DBG(2, Trying to set unsupported mode %u\n, musb_mode); - } + if (data-set_mode) + data-set_mode(musb_mode); You should return -EIO if data-set_mode is NULL. Ok fine... Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
We already have generic APIs so I think we can pass function pointers to musb driver via struct omap_musb_board_data, struct omap_musb_board_data { +void(*phy_on) (void) +void(*phy_off) (void) +void (*intr_clr) (void) } Reset part can be done in board file itself same as Ethernet driver is doing. Does this look fine? so those would be turn phy on, turn phy off and clear interrupt apis ? How about: int (*set_phy_power)(unsigned on); Looks good. void (*clear_phy_irq)(void); This is actually musb ip interrupt clear so I will make it like, void (*clear_irq) (void); You'd need one less API for that. BTW, could you do it on top of my musb-hw branch ? Then I can push together with those for merge window. Not yet, I will send once completed. Thanks, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
why don't you add a proper otg_transceiver driver for am35x ? Is it like omap4's internal phy ? A separate block ? AM35x PHY is built inside the ip and we need to configure it through PHY control register. Additionally we also need to access IPSS reset and Intr clear register as well which would not fit inside otg_transceiver. how about passing an extra struct resource if am35x ? Then you could ioremap that base on am35x.c and use normal musb_read/write functions. We already have generic APIs so I think we can pass function pointers to musb driver via struct omap_musb_board_data, struct omap_musb_board_data { + void(*phy_on) (void) + void(*phy_off) (void) + void (*intr_clr) (void) } Reset part can be done in board file itself same as Ethernet driver is doing. Does this look fine? -Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
Tony ? Do you ack the usage of that header ? NAK. Drivers should not mess with the control registers directly. Instead, the following should be done in the platform init code: $ grep -r omap_ctrl_read drivers/usb drivers/usb/musb/am35x.c: devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) drivers/usb/musb/am35x.c: devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); drivers/usb/musb/am35x.c: u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); drivers/usb/musb/am35x.c: sw_reset = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); drivers/usb/musb/am35x.c: lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR); You can pass a function pointer like board_set_power or simila in platform_data. We would need control register apis for accessing USB PHY control , IPSS reset and interrupt clear register. This would require to add three different function pointer and that would mostly be custom to AM35x. Will that be acceptable from musb perspective ? -Ajay That was my feeling too. Anand, care to update ?!? -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] AM35x: musb: fix compilation error
We would need control register apis for accessing USB PHY control , IPSS reset and interrupt clear register. This would require to add three different function pointer and that would mostly be custom to AM35x. Will that be acceptable from musb perspective ? why don't you add a proper otg_transceiver driver for am35x ? Is it like omap4's internal phy ? A separate block ? AM35x PHY is built inside the ip and we need to configure it through PHY control register. Additionally we also need to access IPSS reset and Intr clear register as well which would not fit inside otg_transceiver. Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3 v5] AM35x: Add musb support
Hi, On Thu, Sep 30, 2010 at 12:19:07AM -0500, Gupta, Ajay Kumar wrote: AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine. It has USB phy built inside the IP itself. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com besides the one small question below: Acked-by: Felipe Balbi ba...@ti.com (note that I don't have that board to fully test this patch) +static struct omap_musb_board_data musb_board_data = { +.interface_type = MUSB_INTERFACE_ULPI, +.mode = MUSB_OTG, +.power = 500, can this board really source 500mA ?? Yes, it supports upto 500mA same as OMAP3EVM (Rev-G). -Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v5] musb: add musb support for AM35x
Hi, On Thu, Sep 30, 2010 at 12:19:08AM -0500, Gupta, Ajay Kumar wrote: AM35x has musb interface and uses CPPI4.1 DMA engine. Current patch supports only PIO mode. DMA support can be added later once basic CPPI4.1 DMA patch is accepted. Also added USB_MUSB_AM35X which is required to differentiate musb ips between OMAP3x and AM35x. This config would be used to for below purposes, - Select am35x.c instead of omap2430.c for compilation at drivers/usb/musb directory. Please note there are significant differneces in these two files as musb ip in quite different on AM35x. - Select workaround codes applicable for AM35x musb issues. one such workaround is for bytewise read issue on AM35x. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com this one doesn't apply of top of my musb-2.6.37 branch. [...] could you refresh and resend this one ?? I couldn't fetch your gitorious tree so I just applied 14 patch set Manually on my branch. Anyways I will look into it and send the refreshed patch shortly. Thanks, Ajay [...] -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v5] musb: add musb support for AM35x
Hi, [...] I couldn't fetch your gitorious tree so I just applied 14 patch set Manually on my branch. Anyways I will look into it and send the refreshed patch shortly. gitorious must be trying to make me look bad :-p Works like a charm here. Will try on a different laptop fine on another laptop too. Maybe you need to check your .git/config the correct address is git://gitorious.org/usb/usb.git I am using same address (http one) but fetch always fails while getting Index pack, 484a70e99b430800a9d93198bae32df155aecfc9 Retrying.. -Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v4] musb: add musb support for AM35x
Hi, +config USB_MUSB_AM35X + boolean AM35X MUSB support + depends on USB_MUSB_HDRC MACH_OMAP3517EVM As I've already said, depending on the board type won't scale... :-( ..and to scale it up we need to add select USB_MUSB_AM35X in arch/arm/mach-omap2/Kconfig for all the boards having AM35x. Felipe, what's your opinion on this? + select NOP_USB_XCEIV + default y + help + Select this option if your platform is based on AM35x. As + AM35x has an updated MUSB with CPPI4.1 DMA so this config + is introduced to differentiate musb ip between OMAP3x and + AM35x platforms. OK, but why it's made visible? Hmm, can be made hidden. diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c new file mode 100644 index 000..ee0c104 --- /dev/null +++ b/drivers/usb/musb/am35x.c @@ -0,0 +1,510 @@ +/* + * Texas Instruments AM35x glue layer + * + * Copyright (c) 2010, by Texas Instruments + * + * Based on the DA8xx glue layer code. + * Copyright (C) 2005-2006 by Texas Instruments There's no code by TI in the DA8xx layer -- that copyright comes from the DaVinci code. Yes, it's from DaVinci. + * Copyright (c) 2008, MontaVista Software, Inc. sou...@mvista.com I've extended it to 2008-2009 (and should have to 2010 :-). no issue, I will update it. +/* USB interrupt register bits */ +#define USB_INTR_USB_SHIFT 16 +#define USB_INTR_USB_MASK (0x1ff USB_INTR_USB_SHIFT) Don't seem like good names (USB_ repeated twice)... Ok, will try to get a better name in next version :) [...] +int __init musb_platform_init(struct musb *musb, void *board_data) +{ + void __iomem *reg_base = musb-ctrl_base; + struct clk *otg_fck; + u32 rev, lvl_intr, sw_reset; + int status; + + musb-mregs += USB_MENTOR_CORE_OFFSET; + + if (musb-set_clock) + musb-set_clock(musb-clock, 1); OMAP doesn't use plat-set_clock anymore, does it? Yes it doesn't anymore so can be removed. + else + clk_enable(musb-clock); + DBG(2, usbotg_ck=%lud\n, clk_get_rate(musb-clock)); + + otg_fck = clk_get(musb-controller, fck); + if (IS_ERR(otg_fck)) { + status = PTR_ERR(otg_fck); + otg_fck = NULL; This assignment does not seem necessary. Yes. + goto exit0; + } [...] +exit1: + clk_disable(otg_fck); +exit0: + clk_disable(musb-clock); + return status; +} + +int musb_platform_exit(struct musb *musb) +{ + struct clk *otg_fck; + + if (is_host_enabled(musb)) + del_timer_sync(otg_workaround); + + phy_off(); + + otg_put_transceiver(musb-xceiv); + usb_nop_xceiv_unregister(); + + if (musb-set_clock) + musb-set_clock(musb-clock, 0); Looks like it may be dropped... + else + clk_disable(musb-clock); + + otg_fck = clk_get(musb-controller, fck); Isn't it better to store this in some static variable? I don't think there's or there'll be multiple instances of MUSB on AM35x... AM35x has only one instance but future coming platform does have two musb interfaces and so we would face this issue. I think it's better to add another entry in struct musb itself. Felipe, any comment? Thanks, Ajay + if (IS_ERR(otg_fck)) { + DBG(2, clk_get() failed for otg_fck.\n); + } else { + clk_put(otg_fck); + clk_put(otg_fck); + clk_disable(otg_fck); + } + + return 0; +} WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/8] musb: fix compilation warning in host only mode
Hi, On Thu, Jun 24, 2010 at 01:17:54PM +0200, ext Gupta, Ajay Kumar wrote: Fixes below compilation warning when host only configuration is selected. drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq': drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase' Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com Acked-by: Felipe Balbi felipe.ba...@nokia.com You've acked a random ammount of patches recently, are you going to be resending them to me? I have a hard time going back and digging through the archives to pick out the ones that you acked vs. the ones you didn't. Greg, There are 2 musb and 1 ehci-omap bug fix patches which are acked. Another related one is on ULPI and is trivial. Apart from these, 2 musb patches for v2.6.36 window are also acked. I will prepare the set and send you after sanity testing. Thanks, Ajay thanks, greg k -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH resend 1/3] AM35x: Add musb support
Hi, If a Kconfig option is needed for optionally compiling in the support for am35x musb, it should be called USB_MUSB_AM35X or similar that gets selected if the boards using it are selected. Do you mean that we should have this option in drivers/usb/musb/Kconfig? Yeah, it could be set automatically with default y if MACH_AM35X_SOME_BOARD. Then options like this should not be mutually exclusive like they currently are for musb, that breaks using musb with multi omap. Choosing USB_MUSB_AM35X would anyways compile am35x.c and not omap2430.c File; thus musb would not work on OMAP3x boards with same binary. You should set up things so both can be compiled in, that's standard behaviour for all Linux drivers :) This is much easier said than done. Tony, As musb controllers are quite different between OMAPs and AM35x so it would be difficult in terms of software maintenance with such approach. Some of the major changes in AM35x are, - Has builtin USB PHY - It doesn't use Mentor DMA but uses CPPI4.1 DMA - Has set of wrapper register which are not present on OMAPs. - Doesn't have SYSCONFIG registers which are present on OMAPs. - Has bytewise read limitation which is not applicable to OMAPs. Musb on AM35x is actually quite similar to musb on DA8xx family of Devices. I can update the patches to use USB_MUSB_AM35X config within drivers/usb/musb/ but that would still not be able to compile in Both omapx and am35x stuff in single binary. Thanks, Ajay I will update the patches and submit for further reviews. Thanks, Tony WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH resend 2/3] musb: add musb support for AM35x
Hi, +{ + unsigned long timeout = jiffies + msecs_to_jiffies(100); + u32 devconf2; + + /* + * Start the on-chip PHY and its PLL. + */ + devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); + + devconf2 = ~(CONF2_RESET | CONF2_PHYPWRDN | CONF2_OTGPWRDN | + CONF2_PHY_GPIOMODE); BWT, what's thet GPIOMODE bit for? If GPIO Mode is set to '1' then UART3 Tx/Rx would come on DP and DM pins of USBPHY. Hm, why GPIOMODE then I wonder... :-) + devconf2 |= CONF2_PHY_PLLON | CONF2_DATPOL; So, AM35x always uses the reversed data polarity? It's getting set to '1' so its always normal polarity. Ah, I got it reversed: 1 means normal polarity indeed... I think, this would again be dependent on boards and therefore should be moved to board file. +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + void __iomem*mregs = musb-mregs; + u8 devctl; + unsigned long flags; + + /* + * We poll because AM35x's won't expose several OTG-critical + * status change events (from the transceiver) otherwise. + */ + devctl = musb_readb(mregs, MUSB_DEVCTL); + DBG(7, Poll devctl %02x (%s)\n, devctl, otg_state_string(musb)); + + spin_lock_irqsave(musb-lock, flags); + switch (musb-xceiv-state) { [...] + case OTG_STATE_A_WAIT_VFALL: So, are you sure there's no need to call mod_timer() here for RTL 1.8? What issue did you see on earlier RTLs ? I didn't test DaVinci much; I didn't even test DA8xx much in OTG mode. :-/ And in the DA8xx host mode VBUS error would never occur anyway (as the VBUS comparator is overridden on the EVM board). As such I didn't see issue In my normal testing. OK. + musb-xceiv-state = OTG_STATE_A_WAIT_VRISE; + musb_writel(musb-ctrl_base, CORE_INTR_SRC_SET_REG, + MUSB_INTR_VBUSERROR USB_INTR_USB_SHIFT); + break; + case OTG_STATE_B_IDLE: + if (!is_peripheral_enabled(musb)) + break; Hm, davinci.c sets DevCtl.Session here and I'm also doing it in da8xx.c -- can you comment why you don't do that too? I remember this was causing some issue in OTG testing. Do you remember which issue? Although I suspect that this code might be related to the comment below: DEVCTL.BDEVICE is invalid without DEVCTL.SESSION set. I will test again and then provide the details. I too feel it has something to do with the comment you pointed out. + devctl = musb_readb(mregs, MUSB_DEVCTL); + if (devctl MUSB_DEVCTL_BDEVICE) + mod_timer(otg_workaround, jiffies + POLL_SECONDS * HZ); + else + musb-xceiv-state = OTG_STATE_A_IDLE; + break; [...] + musb-a_wait_bcon = A_WAIT_BCON_TIMEOUT; This field is set by musb_core.c, so I dropped this line from da8xx.c... This has also been dropped from omap2430.c by this commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- 2.6.git;a=commit;h=f7f9d63eac12b345d6243d1d608b7944a05be921 Need to test again but I was facing a issue where OTG build image Was either working in host only mode or device only. Role were Not changing based on ID pin status. Hm, is this related? Not really but let me test again and get the details. +int musb_platform_exit(struct musb *musb) +{ + struct clk *otg_fck = clk_get(musb-controller, fck); + + if (is_host_enabled(musb)) + del_timer_sync(otg_workaround); + + /* Delay to avoid problems with module reload... */ [..] musb_platfrom_init() suggests that otg_fck can't be NULL. Also, clk_get returns error code, not NULL, so should use IS_ERR() here. Correct, would clean it. Also, you call clk_get() on this clock twice, in musb_platfrom_init() and here, but clk_put() only once. Ok fine. Thanks, Ajay Thanks, Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH resend 1/3] AM35x: Add musb support
I think Ajay has explained why it's needed. The option is necessary in one or another form. It's not needed for omaps, we can already build in support for omap2, omap3 and omap4 into the same kernel binary. Not with AM35x USB support merged -- at least you won't be able to build single kernel with monolithic MUSB support. Right. I believe musb is pretty much the only remaining driver that won't behave with multi-omap. But let's not merge code that would make fixing that even harder. If a Kconfig option is needed for optionally compiling in the support for am35x musb, it should be called USB_MUSB_AM35X or similar that gets selected if the boards using it are selected. Do you mean that we should have this option in drivers/usb/musb/Kconfig? Yeah, it could be set automatically with default y if MACH_AM35X_SOME_BOARD. Then options like this should not be mutually exclusive like they currently are for musb, that breaks using musb with multi omap. Choosing USB_MUSB_AM35X would anyways compile am35x.c and not omap2430.c File; thus musb would not work on OMAP3x boards with same binary. I will update the patches and submit for further reviews. Thanks, Ajay Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH resend 2/3] musb: add musb support for AM35x
+#define USB_SOFT_RESET_MASK1 Need a empty line here. Hmm, ok. +#define A_WAIT_BCON_TIMEOUT1100/* in ms */ I think this #define should be dropped -- see below... +{ + unsigned long timeout = jiffies + msecs_to_jiffies(100); + u32 devconf2; + + /* +* Start the on-chip PHY and its PLL. +*/ + devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); + + devconf2 = ~(CONF2_RESET | CONF2_PHYPWRDN | CONF2_OTGPWRDN | + CONF2_PHY_GPIOMODE); BWT, what's thet GPIOMODE bit for? If GPIO Mode is set to '1' then UART3 Tx/Rx would come on DP and DM pins of USBPHY. + devconf2 |= CONF2_PHY_PLLON | CONF2_DATPOL; So, AM35x always uses the reversed data polarity? It's getting set to '1' so its always normal polarity. +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + void __iomem*mregs = musb-mregs; + u8 devctl; + unsigned long flags; + + /* +* We poll because AM35x's won't expose several OTG-critical +* status change events (from the transceiver) otherwise. +*/ + devctl = musb_readb(mregs, MUSB_DEVCTL); + DBG(7, Poll devctl %02x (%s)\n, devctl, otg_state_string(musb)); + + spin_lock_irqsave(musb-lock, flags); + switch (musb-xceiv-state) { [...] + case OTG_STATE_A_WAIT_VFALL: So, are you sure there's no need to call mod_timer() here for RTL 1.8? What issue did you see on earlier RTLs ? As such I didn't see issue In my normal testing. + musb-xceiv-state = OTG_STATE_A_WAIT_VRISE; + musb_writel(musb-ctrl_base, CORE_INTR_SRC_SET_REG, + MUSB_INTR_VBUSERROR USB_INTR_USB_SHIFT); + break; + case OTG_STATE_B_IDLE: + if (!is_peripheral_enabled(musb)) + break; Hm, davinci.c sets DevCtl.Session here and I'm also doing it in da8xx.c -- can you comment why you don't do that too? I remember this was causing some issue in OTG testing. + devctl = musb_readb(mregs, MUSB_DEVCTL); + if (devctl MUSB_DEVCTL_BDEVICE) + mod_timer(otg_workaround, jiffies + POLL_SECONDS * HZ); + else + musb-xceiv-state = OTG_STATE_A_IDLE; + break; [...] +static irqreturn_t am35x_interrupt(int irq, void *hci) +{ + struct musb *musb = hci; + void __iomem *reg_base = musb-ctrl_base; + unsigned long flags; + irqreturn_t ret = IRQ_NONE; + u32 epintr, usbintr, lvl_intr; + + spin_lock_irqsave(musb-lock, flags); + + /* Get endpoint interrupts */ + epintr = musb_readl(reg_base, EP_INTR_SRC_MASKED_REG); + + if (epintr) { + musb_writel(reg_base, EP_INTR_SRC_CLEAR_REG, epintr); + + musb-int_rx = + (epintr AM35X_RX_INTR_MASK) USB_INTR_RX_SHIFT; + musb-int_tx = + (epintr AM35X_TX_INTR_MASK) USB_INTR_TX_SHIFT; + } Perhaps this should be placed after the following check... Hmm, ok. + /* Get usb core interrupts */ + usbintr = musb_readl(reg_base, CORE_INTR_SRC_MASKED_REG); + if (!usbintr !epintr) + goto eoi; + + if (usbintr) { + musb_writel(reg_base, CORE_INTR_SRC_CLEAR_REG, usbintr); + + musb-int_usb = + (usbintr USB_INTR_USB_MASK) USB_INTR_USB_SHIFT; + } [...] + if (usbintr (USB_INTR_DRVVBUS USB_INTR_USB_SHIFT)) { + int drvvbus = musb_readl(reg_base, USB_STAT_REG); + void __iomem *mregs = musb-mregs; + u8 devctl = musb_readb(mregs, MUSB_DEVCTL); + int err; [...] + } else if (is_host_enabled(musb) drvvbus) { + musb-is_active = 1; I dropped this line from da8xx.c as per this change to davinci.c: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- 2.6.git;a=commit;h=89368d3d11a5b2eff83ad8e752be67f77a372bad Would test on this. [...] +int __init musb_platform_init(struct musb *musb, void *board_data) +{ + void __iomem *reg_base = musb-ctrl_base; + struct clk *otg_fck; + u32 rev, lvl_intr, sw_reset; + + musb-mregs += USB_MENTOR_CORE_OFFSET; + + if (musb-set_clock) + musb-set_clock(musb-clock, 1); + else + clk_enable(musb-clock); + DBG(2, usbotg_ck=%lud\n, clk_get_rate(musb-clock)); + + otg_fck = clk_get(musb-controller, fck); Can't this fail? Yup, need to correct. + clk_enable(otg_fck); + DBG(2, usbotg_phy_ck=%lud\n, clk_get_rate(otg_fck)); + + /* Returns zero if e.g. not clocked */ + rev = musb_readl(reg_base, USB_REVISION_REG); + if (!rev) + return -ENODEV; You forgot to disable the clocks you just enabled above. +
RE: [PATCH 3/8] musb: fix compilation warning in host only mode
+#ifdef CONFIG_USB_MUSB_HDRC_HCD +void __iomem*mbase = musb-mregs; +#endif then you add another ifdef to this file, which is already insane. I'd rather see you either keep the local variables and just fix what needs to be fixed, or use musb-mregs directly. This one seems to be a much better one. Copying the v-3 with this fix below. -- cut here Fixes below compilation warning when host only configuration is selected. drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq': drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase' Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musb_core.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index a8b0440..ed6e1a4 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -704,7 +704,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, #ifdef CONFIG_USB_MUSB_HDRC_HCD if (int_usb MUSB_INTR_CONNECT) { struct usb_hcd *hcd = musb_to_hcd(musb); - void __iomem *mbase = musb-mregs; handled = IRQ_HANDLED; musb-is_active = 1; @@ -717,9 +716,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, if (is_peripheral_active(musb)) { /* REVISIT HNP; just force disconnect */ } - musb_writew(mbase, MUSB_INTRTXE, musb-epmask); - musb_writew(mbase, MUSB_INTRRXE, musb-epmask 0xfffe); - musb_writeb(mbase, MUSB_INTRUSBE, 0xf7); + musb_writew(musb-mregs, MUSB_INTRTXE, musb-epmask); + musb_writew(musb-mregs, MUSB_INTRRXE, musb-epmask 0xfffe); + musb_writeb(musb-mregs, MUSB_INTRUSBE, 0xf7); #endif musb-port1_status = ~(USB_PORT_STAT_LOW_SPEED |USB_PORT_STAT_HIGH_SPEED -- 1.6.2.4 Thanks, Ajay -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/8] musb: fix compilation warning in host only mode
then you add another ifdef to this file, which is already insane. I'd rather see you either keep the local variables and just fix what needs to be fixed, or use musb-mregs directly. This one seems to be a much better one. Copying the v-3 with this fix below. I don't see how it's *much* better... As it doesn't require to create another variable and moreover within ugly #ifdefs of three different flavors (HOST, DEVCIE and OTG). I think it's better to kill all 'mbase' variables and use musb-mregs directly for a better code readability. -Ajay -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/8] musb: fix compilation warning in host only mode
Fixes below compilation warning when host only configuration is selected. drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq': drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase' Also removed definition of 'mbase' from multiple places to only at function top. AFAIR, it was intentionally removed from the function top and declared in the multiple plcase instead by the former Felipe's patch [1] to fix exactly the same issue, if I don't mistake. So, it hasn't worked out? Yes, it was removed by Felipe's below patch but it introduced compilation warning issue as reported. --- commit aa4714560b4ea359bb7830188ebd06bce71bcdea usb: musb: core: declare mbase only where it's used ... and avoid a compilation if we disable host side of musb. -- Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com [...] diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index f0ff893..7cc8398 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, u8 devctl, u8 power) { irqreturn_t handled = IRQ_NONE; +#ifdef CONFIG_USB_MUSB_HDRC_HCD + void __iomem*mbase = musb-mregs; +#endif I'd rather see it declared multiple times... I was thinking it's better to have it at one place and avoid multiple #ifdefs. -Ajay @@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, if (int_usb MUSB_INTR_CONNECT) { struct usb_hcd *hcd = musb_to_hcd(musb); - void __iomem *mbase = musb-mregs; We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block. handled = IRQ_HANDLED; musb-is_active = 1; [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- 2.6.git;a=commitdiff;h=aa4714560b4ea359bb7830188ebd06bce71bcdea WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/8] musb: fix compilation warning in host only mode
AFAIR, it was intentionally removed from the function top and declared in the multiple plcase instead by the former Felipe's patch [1] to fix exactly the same issue, if I don't mistake. So, it hasn't worked out? Yes, it was removed by Felipe's below patch but it introduced compilation warning issue as reported. --- commit aa4714560b4ea359bb7830188ebd06bce71bcdea usb: musb: core: declare mbase only where it's used ... and avoid a compilation if we disable host side of musb. -- Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com [...] diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index f0ff893..7cc8398 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, u8 devctl, u8 power) { irqreturn_t handled = IRQ_NONE; +#ifdef CONFIG_USB_MUSB_HDRC_HCD + void __iomem*mbase = musb-mregs; +#endif I'd rather see it declared multiple times... I was thinking it's better to have it at one place and avoid multiple #ifdefs. You'd only need one #ifdef: in the placae where the warning was reported. And you can open a block under #ifdef CONFIG_USB_MUSB_OTG to declare it in. Other declarations are already covered by #ifdef's, aren't they? Here is the version-2 of this patch. --- cut here --- Fixes below compilation warning when host only configuration is selected. drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq': drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase' Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musb_core.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index a8b0440..cfae447 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -704,7 +704,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, #ifdef CONFIG_USB_MUSB_HDRC_HCD if (int_usb MUSB_INTR_CONNECT) { struct usb_hcd *hcd = musb_to_hcd(musb); +#ifdef CONFIG_USB_MUSB_OTG void __iomem *mbase = musb-mregs; +#endif handled = IRQ_HANDLED; musb-is_active = 1; -- 1.6.2.4 Thanks, Ajay -Ajay @@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, if (int_usb MUSB_INTR_CONNECT) { struct usb_hcd *hcd = musb_to_hcd(musb); - void __iomem *mbase = musb-mregs; We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block. Yeah, the block where 'mbase' is actually used. BTW, your patch didn't cover the declaration in #if 0'ed out SOF handler. WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [patch-2.6.35-rc3+ 0/8] musb patches
These are reviewed and non-controversial musb bugfix patches. Sanity have been tested on OMAP3EVM for Host-MSC, HID, audio, video class and device-CDC class. Sorry, I need Felipe to send these to me, or at the very least, give me an ack for them, before I will apply them. Felipe, Can you please forward these patches to Greg? -Ajay thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3 v3] AM35x: Add musb support
Hi, [..] +* We have to override VBUS/ID signals when MUSB is configured into the +* host-only mode -- ID pin will float if no cable is connected, so the +* controller won't be able to drive VBUS thinking that it's a B- device. +* Otherwise, we want to use the OTG mode and enable VBUS comparators. +*/ + devconf2 = ~CONF2_OTGMODE; +#ifdef CONFIG_USB_MUSB_HOST + devconf2 |= CONF2_FORCE_HOST; Well, this is only necessary if the board doesn't have pulldown on the USB ID signal. On DA830 EVM, rev. of the board A had it, and it got removed on the later revisions (presumably it conflicted with OTG function?), so this override became necessary. So, make sure this is indeed necessary on your board as using the overrides isn't generally desirable (they all also override VBUS sensing in addition to ID pin). Yes, it's not needed for AM3517EVM -Ajay +#else + devconf2 |= CONF2_SESENDEN | CONF2_VBDTCTEN; +#endif + + omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); + + usb_musb_init(musb_board_data); +} + static const struct ehci_hcd_omap_platform_data ehci_pdata __initconst = { .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY, #if defined(CONFIG_PANEL_SHARP_LQ043T1DG01) || \ WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3 v3] AM35x: Add musb support
AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine. It has USB phy built inside the IP itself. So it's more like DaVinci (earlier CPPI as well as integrated PHY) than OMAP3... Yes. Actually they removed original musb controller from OMAP3 and replaced it with DaVinci family musb ip. -Ajay Also added ARCH_AM35x which is required to differentiate musb ips between OMAP3x and AM35x. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: usb_nop_xceiv_register() missing when OTG built as modules
Hi, On Mon, May 24, 2010 at 09:07:37AM +0200, ext Gupta, Ajay Kumar wrote: Attempting to remove usb_nop_xceiv_register() completely will require someone with more knowledge of davinci and blackfin archs to comment on what boards need the platform_device defined. NAK.. This is an incorrect fix as it duplicates the code snippet which is already present in nop file. I don't believe you understood the purpose of the original patch: I did and that's why an alternative patch. The idea is that currently, it's impossible to build a board which uses nop transceiver and have that be a module because when you'll have an undefined function being used on a board file. I know. Removing the call to nop_xceiv_register() and using a platform_device_register() is the way to go, since it's only 4 ~ 5 lines of code anyway. 4 ~5 lines would keep on repeating for all current and future boards requiring to use NOP. ps: you really think that adding more and more ifdefs is better than Amit's patch ?? Yes, when compared to the approach of repeating those 4 ~5 line in all the board files requiring NOP. -Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: usb_nop_xceiv_register() missing when OTG built as modules
ps: you really think that adding more and more ifdefs is better than Amit's patch ?? We can avoid using #ifdefs by introducing '.neednop' flag to board_data. Here is the patch for this which can be used on top of my earlier patch. --- cut here --- diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c index 77af4c9..fb29837 100644 --- a/arch/arm/mach-omap2/board-4430sdp.c +++ b/arch/arm/mach-omap2/board-4430sdp.c @@ -137,6 +137,7 @@ static struct omap_musb_board_data musb_board_data = { .interface_type = MUSB_INTERFACE_UTMI, .mode = MUSB_PERIPHERAL, .power = 100, + .neednop= 1, }; static struct omap2_hsmmc_info mmc[] = { diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c index 83d3aa5..609f021 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -665,6 +665,7 @@ static struct omap_musb_board_data musb_board_data = { .interface_type = MUSB_INTERFACE_ULPI, .mode = MUSB_OTG, .power = 100, + .neednop= 1, }; static void __init omap3_evm_init(void) diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h index a32d3af..b53489a 100644 --- a/arch/arm/plat-omap/include/plat/usb.h +++ b/arch/arm/plat-omap/include/plat/usb.h @@ -69,6 +69,7 @@ struct omap_musb_board_data { u8 mode; u16 power; unsigned extvbus:1; + unsigned neednop:1; /* NOP transceiver */ }; enum musb_interface{MUSB_INTERFACE_ULPI, MUSB_INTERFACE_UTMI}; diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index e71049c..b774312 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1964,6 +1964,7 @@ bad_config: musb-board_set_power = plat-set_power; musb-set_clock = plat-set_clock; musb-min_power = plat-min_power; + musb-board_data = plat-board_data; /* Clock usage is chip-specific ... functional clock (DaVinci, * OMAP2430), or PHY ref (some TUSB6010 boards). All this core diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index eaabf98..b72e2e5 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -392,6 +392,7 @@ struct musb { int (*board_set_power)(int state); int (*set_clock)(struct clk *clk, int is_active); + void*board_data; u8 min_power; /* vbus for periph, in mA/2 */ diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index 58acd0c..8488a23 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -198,12 +198,11 @@ int __init musb_platform_init(struct musb *musb, void *board_data) omap_cfg_reg(AE5_2430_USB0HS_STP); #endif -#if defined(CONFIG_MACH_OMAP3EVM) || defined(CONFIG_MACH_OMAP_4430SDP) /* OMAP3EVM used ISP150x and OMAP4 SDP uses internal transceiver * so register nop transceiver */ - usb_nop_xceiv_register(); -#endif + if (data-neednop) + usb_nop_xceiv_register(); /* We require some kind of external transceiver, hooked * up through ULPI. TWL4030-family PMICs include one, @@ -330,10 +329,10 @@ static int musb_platform_resume(struct musb *musb) int musb_platform_exit(struct musb *musb) { + struct omap_musb_board_data *data = musb-board_data; musb_platform_suspend(musb); -#if defined(CONFIG_MACH_OMAP3EVM) || defined(CONFIG_MACH_OMAP_4430SDP) - usb_nop_xceiv_unregister(); -#endif + if (data-neednop) + usb_nop_xceiv_unregister(); return 0; } -Ajay -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: usb_nop_xceiv_register() missing when OTG built as modules
Hi, Here is a compile-tested patch since I haven't seen this fixed in mainline yet. It applies to the tip of Linus' tree. Attempting to remove usb_nop_xceiv_register() completely will require someone with more knowledge of davinci and blackfin archs to comment on what boards need the platform_device defined. NAK.. This is an incorrect fix as it duplicates the code snippet which is already present in nop file. Here is a better fix for this (patch attached below). The approach in this patch has already been discussed and was not agreed and I was asked to move nop registration to board files. cut here -- NOP transceiver is getting registered in board files of OAMP3EVM and OMAP4430 SDP as they use ISP150x and internal transceivers. This registration in board file forces NOP transceiver to be built into the kernel. Fixing this by removing NOP tranceiver from board files to omap platform file within musb driver. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/mach-omap2/board-4430sdp.c |3 --- arch/arm/mach-omap2/board-omap3evm.c |4 drivers/usb/musb/omap2430.c | 11 ++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c index e4a5d66..77af4c9 100644 --- a/arch/arm/mach-omap2/board-4430sdp.c +++ b/arch/arm/mach-omap2/board-4430sdp.c @@ -17,7 +17,6 @@ #include linux/platform_device.h #include linux/io.h #include linux/gpio.h -#include linux/usb/otg.h #include linux/spi/spi.h #include linux/i2c/twl.h #include linux/regulator/machine.h @@ -374,8 +373,6 @@ static void __init omap_4430sdp_init(void) platform_add_devices(sdp4430_devices, ARRAY_SIZE(sdp4430_devices)); omap_serial_init(); omap4_twl6030_hsmmc_init(mmc); - /* OMAP4 SDP uses internal transceiver so register nop transceiver */ - usb_nop_xceiv_register(); /* FIXME: allow multi-omap to boot until musb is updated for omap4 */ if (!cpu_is_omap44xx()) usb_musb_init(musb_board_data); diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c index 81bba19..83d3aa5 100644 --- a/arch/arm/mach-omap2/board-omap3evm.c +++ b/arch/arm/mach-omap2/board-omap3evm.c @@ -27,7 +27,6 @@ #include linux/spi/spi.h #include linux/spi/ads7846.h #include linux/i2c/twl.h -#include linux/usb/otg.h #include linux/smsc911x.h #include linux/regulator/machine.h @@ -682,9 +681,6 @@ static void __init omap3_evm_init(void) omap_serial_init(); - /* OMAP3EVM uses ISP1504 phy and so register nop transceiver */ - usb_nop_xceiv_register(); - if (get_omap3_evm_rev() = OMAP3EVM_BOARD_GEN_2) { /* enable EHCI VBUS using GPIO22 */ omap_mux_init_gpio(22, OMAP_PIN_INPUT_PULLUP); diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index e06d65e..58acd0c 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -198,6 +198,13 @@ int __init musb_platform_init(struct musb *musb, void *board_data) omap_cfg_reg(AE5_2430_USB0HS_STP); #endif +#if defined(CONFIG_MACH_OMAP3EVM) || defined(CONFIG_MACH_OMAP_4430SDP) + /* OMAP3EVM used ISP150x and OMAP4 SDP uses internal transceiver +* so register nop transceiver +*/ + usb_nop_xceiv_register(); +#endif + /* We require some kind of external transceiver, hooked * up through ULPI. TWL4030-family PMICs include one, * which needs a driver, drivers aren't always needed. @@ -325,6 +332,8 @@ int musb_platform_exit(struct musb *musb) { musb_platform_suspend(musb); - +#if defined(CONFIG_MACH_OMAP3EVM) || defined(CONFIG_MACH_OMAP_4430SDP) + usb_nop_xceiv_unregister(); +#endif return 0; } -- 1.6.2.4 Regards, Ajay Cheers, Amit -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, It would not scale very well - we already have multi-omap builds that select support for multiple boards. If the AM35x boards are part of such builds, then mutually exclusive config options won't work - already n8x0 MUSB is exclusive with 243x/omap3. If it is possible to detect the AM35x at runtime, then we could handle this well. Also, a similar set of changes will be needed for the DMA code as well (right now we can pick only one of the DMA engines at a time). it's time to split out platform code from musb core. I was thinking of having omap2430.c blackfin.c tusb6010.c davinci.c be a platform_device that instantiates musb_hdrc platform_device. It would also ioremap() the area and pass the gotten/enabled clock to musb. Then we can have all of the platforms enabled since the board files would pass down the platform_device for the platform code. Something like: This approach would anyway not help the original issue discussed in this thread. We need to have some config option to differentiate musb ips between OMAP3 and AM35x. Another issue: Currently almost 90% of the code is same between drivers/usb/musb/da8xx.c and drivers/usb/musb/am3517.c. any idea on how to avoid duplication? Can we merge these two files and have compilation flags based on musb capability alone (like HAS_CPPI41) rather than CONFIG_ARCH_xx? This approach would also help the original issue of discussed in this thread. -Ajay arch/arm/mach-omap2/usb-musb.c: [..] -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.
Hi, On Fri, May 21, 2010 at 07:15:37AM +0200, ext Gupta, Ajay Kumar wrote: Hi, +#ifdef CONFIG_USB_MUSB_OTG + .mode = MUSB_OTG, +#elif defined(CONFIG_USB_MUSB_HDRC_HCD) + .mode = MUSB_HOST, +#elif defined(CONFIG_USB_GADGET_MUSB_HDRC) .mode = MUSB_PERIPHERAL, +#endif = MUSB_PERIPHERAL, +#endif By the way ... the #ifdeffery should indeed vanish from all board configs except the Davinci DM6446 EVM. If we are claiming to support: A. Same kernel would work on multiple boards B. Single kernel for one single board Then I think these #ifdefferys are required in all the board files. Let's consider the OMAP3EVM which supports all the three modes and you don't support all three modes you support OTG. If you want to build for peripheral only, that's your choice, but the board is wired so that it has OTG support. I meant OTG with ID pin based role. If these #ifdefferys are not present and .mode is set to OTG. Then if I choose to compile the Kernel only for peripheral mode (Case-B above) then I would get below error from musb_core.c. incompatible Kconfig role setting there's a patch making that a warning instead of an #error if I'm not wrong. If that patch ([1] below) is taken then we can avoid all these #ifdefs. [1] http://marc.info/?l=linux-usbm=127367875600618w=2 But I see comment from David on above patch [1] to fix board data Instead which means using #ifdefs again in board files... With all these discussion it's best to accept the above [1] patch and Avoid having ugly #ifdefs from board files. -Ajay -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Re: omap_musb_board_data -- trouble specifying 500mA supply
Hi, We are designing a custom OMAP board that will have a 500mA supply on the OTG port. It looks like the power is set with: struct omap_musb_board_data { u8 interface_type; u8 mode; u8 power; }; However, with a u8, the max value is 255. Should power be changed to a u16? No. The units are 2mA not 1mA, matching the USB specs. So to specify 500mA (the max), say 250 ... David, Actually in OMAP board files, power values have been provided With units of 1mA which gets divided to half in arch/arm/mach-omap2/usb-musb.c before sending it to musb driver. -Ajay -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- Patch created against linus'tree + all musb patches in Greg's queue Changes from v1: - removed unnecessary parens. - Removed 'memcpy' for 32 bit read loops. drivers/usb/musb/am3517.c| 30 ++ drivers/usb/musb/musb_core.c |2 ++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c index b74e664..3299c66 100644 --- a/drivers/usb/musb/am3517.c +++ b/drivers/usb/musb/am3517.c [...] + } +} diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 4093f6d..9c59a8e 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -262,6 +262,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src) } } +#if !defined(CONFIG_MACH_OMAP3517EVM) Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for OMAP3517EVM and would not scale well to other boards based on AM35x. (As commented earlier by Sergei) I just got to know of another board LIZARD based on AM35x so we really need to find a solution for this. This problem is due to the fact that AM35x is based on OMAP35x but musb ip Is updated to RTL1.8 using CPPI4.1 DMA engine thus we need to have a config option to differentiate musb ips between actual OMAP3 and AM35x platforms. I am thinking of adding new config option OMAP_MUSB_RTL18 which should be selected by all the boards based on AM35x in arch/arm/mach-omap2/Kconfig. The same config option can be used for all the workaround/fixes specific to AM35x musb platform. -- diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b72ae06..3ab1156 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM bool OMAP3517/ AM3517 EVM board depends on ARCH_OMAP3 ARCH_OMAP34XX select OMAP_PACKAGE_CBB + select OMAP_MUSB_RTL18 config PMIC_TPS65023 bool TPS65023 Power Module -- Does anyone has a better option to fix this issue or any comment on this approach? Regards, Ajay /* * Unload an endpoint's FIFO */ @@ -299,6 +300,7 @@ void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) readsb(fifo, dst, len); } } +#endif #endif /* normal PIO */ -- 1.6.2.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, Gupta, Ajay Kumar wrote: AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com [...] +#if !defined(CONFIG_MACH_OMAP3517EVM) Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for OMAP3517EVM and would not scale well to other boards based on AM35x. (As commented earlier by Sergei) I just got to know of another board LIZARD based on AM35x so we really need to find a solution for this. This problem is due to the fact that AM35x is based on OMAP35x but musb ip Is updated to RTL1.8 using CPPI4.1 DMA engine Is that really the only difference? Other difference: USB PHY is built inside the ip itself. Additionally, there is an errata which restricts byte/word (8/16 bit) data read which is due to the bus interface on AM35x. thus we need to have a config option to differentiate musb ips between actual OMAP3 and AM35x platforms. I am thinking of adding new config option OMAP_MUSB_RTL18 which should be selected by all the boards based on AM35x in arch/arm/mach- omap2/Kconfig. The same config option can be used for all the workaround/fixes specific to AM35x musb platform. -- diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b72ae06..3ab1156 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM bool OMAP3517/ AM3517 EVM board depends on ARCH_OMAP3 ARCH_OMAP34XX select OMAP_PACKAGE_CBB + select OMAP_MUSB_RTL18 config PMIC_TPS65023 bool TPS65023 Power Module -- Does anyone has a better option to fix this issue or any comment on this approach? Why not introduce CONFIG_ARCH_AM35x instead? This is fine with me but don't know if Tony would agree with it. -Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi, Fixes/workaround based on CONFIG_MACH_OMAP3517EVM will be good only for OMAP3517EVM and would not scale well to other boards based on AM35x. (As commented earlier by Sergei) I just got to know of another board LIZARD based on AM35x so we really need to find a solution for this. This problem is due to the fact that AM35x is based on OMAP35x but musb ip Is updated to RTL1.8 using CPPI4.1 DMA engine thus we need to have a config option to differentiate musb ips between actual OMAP3 and AM35x platforms. I am thinking of adding new config option OMAP_MUSB_RTL18 which should be selected by all the boards based on AM35x in arch/arm/mach- omap2/Kconfig. The same config option can be used for all the workaround/fixes specific to AM35x musb platform. -- diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b72ae06..3ab1156 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -95,6 +95,7 @@ config MACH_OMAP3517EVM bool OMAP3517/ AM3517 EVM board depends on ARCH_OMAP3 ARCH_OMAP34XX select OMAP_PACKAGE_CBB + select OMAP_MUSB_RTL18 config PMIC_TPS65023 bool TPS65023 Power Module -- Does anyone has a better option to fix this issue or any comment on this approach? It would not scale very well - we already have multi-omap builds that select support for multiple boards. If the AM35x boards are part of such builds, then mutually exclusive config options won't work - already n8x0 MUSB is exclusive with 243x/omap3. AM35x musb needs different initialization sequence which involves PHY configuration etc. being done in drivers/usb/musb/am3517.c. am3517.c also need to handle different ISR routine and all CPPi4.1 DMA related programmings. If it is possible to detect the AM35x at runtime, then we could handle this well. Also, a similar set of changes will be needed for the DMA code as well (right now we can pick only one of the DMA engines at a time). We do have such mechanism cpu_is_omap3517() but how about compilation flags for am3517.c? Do you intend to have different initialization sequence, ISR function and CPPI4.1 DMA programmings within drivers/usb/musb/omap2430.c ? -Ajay - Anand -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles.
Hi, Subject: [PATCH] ARM: OMAP: Fix board data to support device only, host only and OTG roles. Fix board data to support device only, host only and OTG roles. The board data hardcodes the mode to OTG or Peripheral. This fix will allow to use Peripheral, Host and OTG roles independently. Signed-off-by: Maulik Mankad x0082...@ti.com Same set of code snippet getting repeated across the files. Isn't it good to have them at same commom file usb-musb.c ? I think better remove .mode parameter from board_data if its not needed anyways if some board supports only some specific mode they can select those specifc config only. -Ajay-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] musb: AM35x: Workaround for fifo read issue
Hi, Ajay Kumar Gupta wrote: AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- Patch created against linus'tree + all musb patches in Greg's queue Changes from v2: - fixed multipline comment style drivers/usb/musb/am3517.c| 31 +++ drivers/usb/musb/musb_core.c |2 ++ 2 files changed, 33 insertions(+), 0 deletions(-) diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c index b74e664..c68c784 100644 --- a/drivers/usb/musb/am3517.c +++ b/drivers/usb/musb/am3517.c @@ -515,3 +515,34 @@ void musb_platform_restore_context(struct musb_context_registers phy_on(); } #endif + +/* AM35x supports only 32bit read operation */ +void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ + void __iomem *fifo = hw_ep-fifo; + u32 val; + int i; + + /* Read for 32bit-aligned destination address */ + if ((likely((0x03 (unsigned long) dst) == 0)) len = 4) { You don't need to put likely() in parens. Ok. + readsl(fifo, dst, len 2); + dst += (len ~0x03); You don't need parens here as well. Ok. + len = 0x03; + } + /* +* Now read the rest 1 to 3 bytes or complete length if +* unaligned address. +*/ + if (len 4) { + for (i = 0; i (len 2); i++) { + val = musb_readl(fifo, 0); + memcpy(dst, val, 4); Can't you do away with memcpy() here? Are you asking for change below? *(u32 *)dst = musb_readl(fifo, 0); -Ajay + dst += 4; + } + len %= 4; + } + if (len 0) { + val = musb_readl(fifo, 0); + memcpy(dst, val, len); + } +} WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/6] musb: dma: use optimal transfer element for sdma
Hi, Ajay Kumar Gupta wrote: Use optimal values of transfer element based on buffer address in system DMA programming. This would improve the performance. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musbhsdma.c | 29 ++--- 1 files changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c index d29e487..39c1801 100644 --- a/drivers/usb/musb/musbhsdma.c +++ b/drivers/usb/musb/musbhsdma.c @@ -52,11 +52,34 @@ static void musb_sdma_channel_program(struct musb *musb, struct musb_dma_channel *musb_channel, dma_addr_t dma_addr, u32 len) { + u16 frame = len; + int data_type = OMAP_DMA_DATA_TYPE_S8; + + switch (dma_addr 0x3) { + case 0: + if ((len % 4) == 0) { + data_type = OMAP_DMA_DATA_TYPE_S32; + frame = len / 4; + break; + } + case 2: + if ((len % 2) == 0) { + data_type = OMAP_DMA_DATA_TYPE_S16; + frame = len / 2; + break; + } + case 1: + case 3: + default: + data_type = OMAP_DMA_DATA_TYPE_S8; + frame = len; + break; The *break* is overindented. Ok. -Ajay WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] musb: AM35x: Workaround for fifo read issue
Hi, Ajay Kumar Gupta wrote: AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- Patch created against linus'tree + all musb patches in Greg's queue Changes from v2: - fixed multipline comment style drivers/usb/musb/am3517.c| 31 +++ drivers/usb/musb/musb_core.c |2 ++ 2 files changed, 33 insertions(+), 0 deletions(-) diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c index b74e664..c68c784 100644 --- a/drivers/usb/musb/am3517.c +++ b/drivers/usb/musb/am3517.c @@ -515,3 +515,34 @@ void musb_platform_restore_context(struct musb_context_registers phy_on(); } #endif + +/* AM35x supports only 32bit read operation */ +void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst) +{ + void __iomem *fifo = hw_ep-fifo; + u32 val; + int i; + + /* Read for 32bit-aligned destination address */ + if ((likely((0x03 (unsigned long) dst) == 0)) len = 4) { You don't need to put likely() in parens. Ok. + readsl(fifo, dst, len 2); + dst += (len ~0x03); You don't need parens here as well. Ok. + len = 0x03; + } + /* + * Now read the rest 1 to 3 bytes or complete length if + * unaligned address. + */ + if (len 4) { + for (i = 0; i (len 2); i++) { + val = musb_readl(fifo, 0); + memcpy(dst, val, 4); Can't you do away with memcpy() here? Are you asking for change below? *(u32 *)dst = musb_readl(fifo, 0); This one is safe and works but the other memcpy (below one) would not be safe to replace as the length would be 1 to 3 bytes. -Ajay + dst += 4; + } + len %= 4; + } + if (len 0) { + val = musb_readl(fifo, 0); + memcpy(dst, val, len); + } +} WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3 v2] musb: AM35x: Workaround for fifo read issue
Hi AM35x supports only 32bit read operations so we need to have workaround for 8bit and 16bit read operations. But don't we need to override musb_readb() and musb_readw() then? Yes, Correct. I do have another patch on that which I will cleanup and submit later. Anyways with currebt three patch set, basic Host and device operation has been tested. -Ajay Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP3: PM: fix AM35x musb issue with AUTOIDLE bit in CONTROL_SYSCONFIG
Hi, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar Sent: Thursday, May 13, 2010 5:15 PM To: Kevin Hilman Cc: linux-omap@vger.kernel.org Subject: RE: [PATCH] OMAP3: PM: fix AM35x musb issue with AUTOIDLE bit in CONTROL_SYSCONFIG Hi, Kumar Gupta ajay.gu...@ti.com writes: MUSB interface on AM35x stops working when we set AUTOIDLE bit (D0) in CONTROL_SYSCONFIG(0x48002010) register. why? stops working in what way? I think you need to investigate more into why this is happening. There is 'turnaround/timeout' error shown in first SETUP packet on USB bus when a device is connected to OTG port. In second retry of SETUP packet, 8 byte of GET_DESCRIPTOR went out Successfully but there is no 'IN token' to receive the descriptor Data although ReqPkt is set for endpoint zero. All this observation shows that OTG controller starts misbehaving When AUTOIDLE bit is set in CONTROL_SYSCONFIG register. This suggest to me that something in the MUSB device/driver init is not quite right for these boards. Do you have anything specific to try in init path ? I observed OTG port to be working with plane linus's tree + AM3517 musb Support patch and without AUTOIDLE bit set so please hold on this patch For now. I will do further testing and let you know. -Ajay Regards, Ajay Kevin Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- arch/arm/mach-omap2/pm34xx.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach- omap2/pm34xx.c index eab..cc9d566 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -849,7 +849,12 @@ static void __init prcm_setup_regs(void) CM_AUTOIDLE); } - omap_ctrl_writel(OMAP3430_AUTOIDLE, OMAP2_CONTROL_SYSCONFIG); + /* + * MUSB interface on AM35x stops working when we enable AUTOIDLE, + * so avoid this for AM3517 and AM3505 device. + */ + if (!cpu_is_omap3517() !cpu_is_omap3505()) + omap_ctrl_writel(OMAP3430_AUTOIDLE, OMAP2_CONTROL_SYSCONFIG); /* * Set all plls to autoidle. This is needed until autoidle is -- 1.6.2.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/5] musb: use system DMA for unaligned buffers on RTL = 1.8
Hi, -Original Message- From: Felipe Balbi [mailto:felipe.ba...@nokia.com] Sent: Friday, May 14, 2010 4:37 PM To: Gupta, Ajay Kumar Cc: m...@felipebalbi.com; linux-...@vger.kernel.org; linux- o...@vger.kernel.org Subject: Re: [PATCH 4/5] musb: use system DMA for unaligned buffers on RTL = 1.8 On Thu, May 13, 2010 at 06:13:00AM +0200, ext Gupta, Ajay Kumar wrote: How about this 'memcpy', this would affect both cpu loading and performance. That's why using system dma would be a better approach. but that's not portable. Even if you come up with nice wrappers for system dma usage (which is already ugly, more exported functions) you might end up on a board where system dma has same alignment constraints as mentor dma. It can't be an OMAP3 based board but yes, future platform could have such limitations but they should not restrict other platforms to use system dma which can handle unaligned buffers. Either fall back to pio or use a bounce buffer. That'll work always. Agreed but performance we should allow system dma approach also. Besides, the only offending gadget driver we have is g_ether and that's already 'fixed' by the alignment patch to g_ether. No, it's not fixed. It had two issues, 1. Cache coherency as we observed iperf failing and after using Cache_flush_all() things started working. 2. High cpu loading of 100% and bit rate of only 30Mbits/sec due To memmove and cache_flush.. -Ajay Any other gadget driver causing that, should be fixed too... -- balbi DefectiveByDesign.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v2] musb: add musb support for AM35x
Hi, Ajay Kumar Gupta ajay.gu...@ti.com wrote: AM35x is based on OMAP35x but has an updated musb interface which uses CPPI4.1 DMA engine. Current patch supports only PIO mode transfers. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- Changes from v1: (Based on Sergei's comment) - Moved PHY clock and OTGMODE settings to board files - Added clk_disable/put in exit path - Removed unwanted code related to vbus - Removed unhandled interrupt check - Added timeout in phy_on() I had to use MACH_OMAP3517EVM as there is no ARCH_xxx config specific to AM3517 platforms other than ARCH_OMAP3. drivers/usb/musb/Kconfig | 4 +- drivers/usb/musb/Makefile | 4 + drivers/usb/musb/am3517.c | 517 [..] + +static inline void phy_on(void) +{ + unsigned long timeout = jiffies + msecs_to_jiffies(100); + u32 devconf2; + + /* + * Start the on-chip PHY and its PLL. + */ + devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); + + devconf2 = ~(CONF2_RESET | CONF2_PHYPWRDN | CONF2_OTGPWRDN | + CONF2_PHY_GPIOMODE); + devconf2 |= CONF2_PHY_PLLON | CONF2_DATPOL; + + omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); + + DBG(1, Waiting for PHY clock good...\n); + while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2) + CONF2_PHYCLKGD)) { + cpu_relax(); + + if (time_after(jiffies, timeout)) { + DBG(1, musb PHY clock good timed out\n); + return; A 'break' would have been more appropriate. Why? 'break' would bring in additional instruction and finally it would return as the loop is already at the bottom of the function. + } + } +} + [..] +int musb_platform_set_mode(struct musb *musb, u8 musb_mode) +{ + u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); + + devconf2 = ~CONF2_OTGMODE; + switch (musb_mode) { +#ifdef CONFIG_USB_MUSB_HDRC_HCD + case MUSB_HOST: /* Force VBUS valid, ID = 0 */ + devconf2 |= CONF2_FORCE_HOST; + break; +#endif +#ifdef CONFIG_USB_GADGET_MUSB_HDRC + case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */ + devconf2 |= CONF2_FORCE_DEVICE; + break; +#endif +#ifdef CONFIG_USB_MUSB_OTG + case MUSB_OTG: /* Don't override the VBUS/ID comparators */ + devconf2 |= CONF2_NO_OVERRIDE; + break; +#endif + default: + DBG(2, Trying to set unsupported mode %u\n, musb_mode); + } Either the switch case or the compile time flag is redundant. No. both has to be there. 'musb_mode' is an entry from user via sysfs for changing the operating mode and if some modes are not even configured in kernel then we should not switch the mode and also warn the user for this. Ajay + + omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); + return 0; [..] -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/5] musb: use system DMA to fix Inventra DMA issue on RTL-1.4
Hi, Another approach to use PIO mode in opposite direction would increase the cpu loading and thus using system DMA is preferred workaround. Signed-off-by: Anand Gadiyar gadi...@ti.com Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com I think falling back to pio is better than this patch. At the cost of cpu, which certainly is not preferred. It will most likely be only one transfer. How about host mode with multiple devices connected and doing transfers? Falling back to PIO would kill the cpu. Another approach is to allocate dma channels on a transfer basis. Can you elaborate this? It might be good idea to allocate the dma channels on tarnsfer basis as Felipe Suggested. The musb driver allocates dma channels for the first 8 enabled endpoints and higher endpoints works in PIO mode. For system dma if there are more nummber of Rx endpoints enabled but not used for data Transfer you might end up having many sdma channles allocated biut not used which will Impact in the system of not utilizing the sdma channels effectively. Felipe, Is this what you meant? If so then I have a patch (copied below) to fix this and I can post this one along with others. -- cut here - Currently DMA channels are allocated and they remain allocated even if there is no active data transfer. Added channel_release() whenever there is no pending request. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com --- drivers/usb/musb/musb_gadget.c | 27 +-- drivers/usb/musb/musb_host.c | 14 -- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index fd842af..477a009 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -297,9 +297,13 @@ static void txstate(struct musb *musb, struct musb_request *req) csr); #ifndefCONFIG_MUSB_PIO_ONLY - if (is_dma_capable() musb_ep-dma) { + + if (is_dma_capable() musb-dma_controller) { struct dma_controller *c = musb-dma_controller; + if (!musb_ep-dma) + musb_ep-dma = c-channel_alloc(c, musb_ep-hw_ep, 1); + use_dma = (request-dma != DMA_ADDR_INVALID); /* MUSB_TXCSR_P_ISO is still set correctly */ @@ -433,6 +437,7 @@ void musb_g_tx(struct musb *musb, u8 epnum) u8 __iomem *mbase = musb-mregs; struct musb_ep *musb_ep = musb-endpoints[epnum].ep_in; void __iomem*epio = musb-endpoints[epnum].regs; + struct dma_controller *c = musb-dma_controller; struct dma_channel *dma; musb_ep_select(mbase, epnum); @@ -535,6 +540,10 @@ void musb_g_tx(struct musb *musb, u8 epnum) if (!request) { DBG(4, %s idle now\n, musb_ep-end_point.name); + if (musb_ep-dma) { + c-channel_release(musb_ep-dma); + musb_ep-dma = NULL; + } return; } } @@ -585,6 +594,7 @@ static void rxstate(struct musb *musb, struct musb_request *req) struct usb_request *request = req-request; struct musb_ep *musb_ep = musb-endpoints[epnum].ep_out; void __iomem*epio = musb-endpoints[epnum].regs; + struct dma_controller *c = musb-dma_controller; unsignedfifo_count = 0; u16 len = musb_ep-packet_sz; u16 csr = musb_readw(epio, MUSB_RXCSR); @@ -601,8 +611,10 @@ static void rxstate(struct musb *musb, struct musb_request *req) return; } + if (is_dma_capable() musb-dma_controller !musb_ep-dma) + musb_ep-dma = c-channel_alloc(c, musb_ep-hw_ep, 0); + if ((is_cppi_enabled() || is_cppi41_enabled()) musb_ep-dma) { - struct dma_controller *c = musb-dma_controller; struct dma_channel *channel = musb_ep-dma; /* NOTE: CPPI won't actually stop advancing the DMA @@ -633,11 +645,9 @@ static void rxstate(struct musb *musb, struct musb_request *req) if (request-actual request-length) { #ifdef CONFIG_USB_INVENTRA_DMA if (is_dma_capable() musb_ep-dma) { - struct dma_controller *c; struct dma_channel *channel; int use_dma = 0; - c = musb-dma_controller; channel = musb_ep-dma; /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in @@ -719,7
RE: [PATCH 4/5] musb: use system DMA for unaligned buffers on RTL = 1.8
Hi, On Thu, May 13, 2010 at 09:44:11AM +0530, Gupta, Ajay Kumar wrote: Hi, -Original Message- From: Felipe Balbi [mailto:m...@felipebalbi.com] Sent: Wednesday, May 12, 2010 11:26 PM To: Sergei Shtylyov Cc: Gupta, Ajay Kumar; linux-...@vger.kernel.org; linux- o...@vger.kernel.org Subject: Re: [PATCH 4/5] musb: use system DMA for unaligned buffers on RTL = 1.8 On Wed, May 12, 2010 at 06:59:52PM +0400, Sergei Shtylyov wrote: Better wrap your stuff into #ifdef OMAP, I think... please don't, better to use the bounce buffer... It would work on blackfin, davinci and omap... Just working is not enough. We need to think of performance and cpu loading as well. allocating a bounce buffer doesn't take long and you can still use mentor DMA to transfer the data. But there is a memcpy involved right? It will use cpu and thus would be hitting on performance, cpu loading and power consumption. Instead why not use other available system resources (system dma)to do this job. I understand, some platform may not support/have system dma then they can surely use bounce buffer or use PIO mode. There was another workaround proposed earlier which was using 'memmove' (similar to memcpy) that was badly affecting cpu loading. I was hardly getting 50Mbits/sec of throughput with 100% loading. Instead when I tried system DMA, numbers went up to 90Mbits/sec with a cpu loading of 50%. I think we must use system DMA for this issue and also have bounce buffer Or PIO fallback mechanism in place for platforms not supporting system DMA. Regards, Ajay -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/5] musb: use system DMA for unaligned buffers on RTL = 1.8
Hi, On Wed, May 12, 2010 at 05:19:38PM +0530, Ajay Kumar Gupta wrote: MUSB RTL version 1.8 onward (OMAP3630, AM/DM37x, OMAP4) DMA requires the buffers to be aligned on a four byte boundary. This affects USB CDC/RNDIS class application where buffers are always unaligned. Use system DMA for unaligned buffers as a workaround of this issue. Current patch supports device side CDC/RNDIS. Host side would require change in Tx programming path for mode-0 operation when transfer length is more than packet size. Also fixed an issue in device Tx completion path where 'is_dma' is getting set unconditionally. This would fail the io if Tx transfer is done in mode-0. Fixed it by updating it based on 'request-actual' length. Signed-off-by: Ajay Kumar Gupta ajay.gu...@ti.com [..] @@ -166,6 +166,12 @@ config MUSB_USE_SYSTEM_DMA_WORKAROUND DMA channels are simultaneously enabled. To work around this issue, you can choose to use System DMA for RX channels. + Also on Mentor DMA in MUSB RTL version 1.8 (OMAP3630, AM/DM37x) + requires buffers to be aligned on a four byte boundary. This affects + USB CDC/RNDIS class application where buffers are always unaligned. + To work around this issue, you can choose to use System DMA for + unaligned buffers. instead of this patch, it's a whole lot easier to simply use a bounce buffer: if (unaligned) { bounce = dma_alloc_coherent(..); memcpy(request-buf, bounce, request-length); } How about this 'memcpy', this would affect both cpu loading and performance. That's why using system dma would be a better approach. -Ajay and use that buffer on channel_program(); -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html