Re: [PATCH v5 2/4] usb: phy: samsung: Add host phy support to samsung-phy driver
Hi Doug,
On Mon, Jan 14, 2013 at 11:15 AM, Vivek Gautam
gautamvivek1...@gmail.com wrote:
Hi Doug,
On Sat, Jan 12, 2013 at 6:20 AM, Doug Anderson diand...@chromium.org wrote:
Vivek,
On Fri, Jan 11, 2013 at 4:40 AM, Vivek Gautam gautamvivek1...@gmail.com
wrote:
+#define HOST_CTRL0_REFCLKSEL_MASK (0x3)
+#define HOST_CTRL0_REFCLKSEL_XTAL (0x0 19)
+#define HOST_CTRL0_REFCLKSEL_EXTL (0x1 19)
+#define HOST_CTRL0_REFCLKSEL_CLKCORE (0x2 19)
+
+#define HOST_CTRL0_FSEL_MASK (0x7 16)
+#define HOST_CTRL0_FSEL(_x)((_x) 16)
+#define HOST_CTRL0_FSEL_CLKSEL_50M (0x7)
+#define HOST_CTRL0_FSEL_CLKSEL_24M (0x5)
+#define HOST_CTRL0_FSEL_CLKSEL_20M (0x4)
+#define HOST_CTRL0_FSEL_CLKSEL_19200K (0x3)
+#define HOST_CTRL0_FSEL_CLKSEL_12M (0x2)
+#define HOST_CTRL0_FSEL_CLKSEL_10M (0x1)
+#define HOST_CTRL0_FSEL_CLKSEL_9600K (0x0)
Add the shifts to the #defines and remove HOST_CTRL0_FSEL(_x). That
makes it match the older phy more closely.
Wouldn't it hamper the readability when shifts are used ??
I mean if we have something like this -
phyhost |= HOST_CTRL0_FSEL(phyclk)
so, if we are using the shifts then
phyhost |= (HOST_CTRL0_FSEL_CLKSEL_24M HOST_CTRL0_FSEL_SHIFT)
I was actually suggesting modifying the #defines like this:
#define HOST_CTRL0_FSEL_SHIFT 16
#define HOST_CTRL0_FSEL_MASK (0x7 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_50M (0x7 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_24M (0x5 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_20M (0x4 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_19200K (0x3 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_12M (0x2 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_10M (0x1 HOST_CTRL0_FSEL_SHIFT)
#define HOST_CTRL0_FSEL_CLKSEL_9600K (0x0 HOST_CTRL0_FSEL_SHIFT)
...then the code doesn't need to think about shifts, right?
right right, sorry i din't get your point earlier. :-(
this way things will be similar in samsung_usbphy_get_refclk_freq()
across exynos 5 and older SoCs.
Is it fine if we don't use macro for SHIFT, earlier code also doesn't use it.
Can we just do like this ..
#define HOST_CTRL0_FSEL_MASK (0x7 16)
#define HOST_CTRL0_FSEL_CLKSEL_50M(0x7 16)
#define HOST_CTRL0_FSEL_CLKSEL_24M(0x5 16)
#define HOST_CTRL0_FSEL_CLKSEL_20M(0x4 16)
#define HOST_CTRL0_FSEL_CLKSEL_19200K (0x3 16)
#define HOST_CTRL0_FSEL_CLKSEL_12M(0x2 16)
#define HOST_CTRL0_FSEL_CLKSEL_10M(0x1 16)
#define HOST_CTRL0_FSEL_CLKSEL_9600K (0x0 16)
Actually missed one thing here, this HOST_CTRL0_FSEL_CLKSEL_XX is
being used by
HOST/OTG blocks to prepare reference clock, that's the reason we kept
#define HOST_CTRL0_FSEL(_x) ((_x) 16)
and #define OTG_SYS_FSEL(_x) ((_x) 4)
where (_x) is the reference clock returned from
samsung_usbphy_get_refclk_freq().
So in order to avoid confusion we can change the macro names only and
keep something like
#define HOST_CTRL0_FSEL_MASK (0x7 16)
#define HOST_CTRL0_FSEL(_x) ((_x) 16)
#define FSEL_CLKSEL_50M (0x7)
#define FSEL_CLKSEL_24M (0x5)
#define FSEL_CLKSEL_20M (0x4)
#define FSEL_CLKSEL_19200K (0x3)
#define FSEL_CLKSEL_12M (0x2)
#define FSEL_CLKSEL_10M (0x1)
#define FSEL_CLKSEL_9600K(0x0)
...
#define OTG_SYS_FSEL_MASK (0x7 4)
#define OTG_SYS_FSEL(_x) ((_x) 4)
if (IS_ERR(ref_clk)) {
dev_err(sphy-dev, Failed to get reference clock\n);
return PTR_ERR(ref_clk);
}
- switch (clk_get_rate(ref_clk)) {
- case 12 * MHZ:
- refclk_freq = PHYCLK_CLKSEL_12M;
- break;
- case 24 * MHZ:
- refclk_freq = PHYCLK_CLKSEL_24M;
- break;
- case 48 * MHZ:
- refclk_freq = PHYCLK_CLKSEL_48M;
- break;
- default:
- if (sphy-cpu_type == TYPE_S3C64XX)
- refclk_freq = PHYCLK_CLKSEL_48M;
- else
+ if (sphy-cpu_type == TYPE_EXYNOS5250) {
+ /* set clock frequency for PLL */
+ switch (clk_get_rate(ref_clk)) {
+ case 96 * 10:
nit: change to 9600 * KHZ to match; below too.
sure.
+ refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_9600K;
Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h
On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote: It changes the driver to use platform_device_id rather than cpu_is_xxx to determine the SoC type, and updates the platform code accordingly. Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable. Tested at mx51 bbg board, it works ok after enable phy clock (Need another patch to fix this problem) Signed-off-by: Peter Chen peter.c...@freescale.com not good for -rc. You have to break this down as you're solving at least three different problems with this patch. Felipe, all my changes are for this problem, these are fix build error and let it work. arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- As we change the connection-id, we need to change clock file or the devm_clk_get will be failed. arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++--- We need to differentiate SoCs, so I use platform_device_id to instead of cpu_ix_xxx(), this is for machine file change drivers/usb/gadget/fsl_mxc_udc.c | 23 + drivers/usb/gadget/fsl_udc_core.c | 52 +--- drivers/usb/gadget/fsl_usb2_udc.h | 13 -- include/linux/fsl_devices.h |8 +++ Need to get platform_device_id at driver, and replace the cpu_is_xxx to platform_device_id. Meanwhile, needs a solution for replace MX35_IO_ADDRESS. Besides there are parts which are not related to the build break. I can split this patch to two patches, one is driver part, the other is machine part. It can also fix build break, but it needs two patches to let the udc work. -- balbi -- Best Regards, Peter Chen -- 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
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Monday 14 January 2013 11:47:57 Ming Lei wrote: [ 181.175323] echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. [ 181.183624] modprobeD c04f1920 0 2462 2461 0x [ 181.183685] [c04f1920] (__schedule+0x5fc/0x6d4) from [c005eba4] (async_synchronize_cookie_domain+0xdc/0x 168) [ 181.183715] [c005eba4] (async_synchronize_cookie_domain+0xdc/0x168) from [c005ed04] (async_synchronize_f ull+0x3c/0x60) [ 181.183776] [c005ed04] (async_synchronize_full+0x3c/0x60) from [c0085610] (load_module+0x1aac/0x1cdc) [ 181.183807] [c0085610] (load_module+0x1aac/0x1cdc) from [c0085944] (sys_init_module+0x104/0x110) [ 181.183837] [c0085944] (sys_init_module+0x104/0x110) from [c000dfe0] (ret_fast_syscall+0x0/0x48) [ 271.175506] INFO: task modprobe:2462 blocked for more than 90 seconds. [ 271.182373] echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. [ 271.190826] modprobeD c04f1920 0 2462 2461 0x [ 271.190887] [c04f1920] (__schedule+0x5fc/0x6d4) from [c005eba4] (async_synchronize_cookie_domain+0xdc/0x 168) [ 271.190917] [c005eba4] (async_synchronize_cookie_domain+0xdc/0x168) from [c005ed04] (async_synchronize_f ull+0x3c/0x60) [ 271.190948] [c005ed04] (async_synchronize_full+0x3c/0x60) from [c0085610] (load_module+0x1aac/0x1cdc) [ 271.190948] [c0085610] (load_module+0x1aac/0x1cdc) from [c0085944] (sys_init_module+0x104/0x110) [ 271.190979] [c0085944] (sys_init_module+0x104/0x110) from [c000dfe0] (ret_fast_syscall+0x0/0x48) OK, your trace is totally different. If your hangs are related, as is likely, my explanation goes out of the window. Regards Oliver -- 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
Re: [RFC PATCH 1/6] Core files for the DWC2 driver
Hi,
On Fri, Jan 11, 2013 at 10:22:08PM +, Paul Zimmerman wrote:
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: Friday, January 11, 2013 1:44 AM
Hi,
Hi Felipe, thanks for the review.
On Mon, Jan 07, 2013 at 11:51:36AM -0800, Paul Zimmerman wrote:
Signed-off-by: Paul Zimmerman pa...@synopsys.com
no commit log == no commit ;-)
Will fix next time.
Also, you shouldn't have reset the version counter. 2 years ago this
same driver was already on v17, then someone else tried and did a few
extra versions...
I meant to just remove the version number, will do that in the next
revision.
... There are a few comments below, probably should be
more but all your files are so big that I just got bored, sorry.
Sorry. I know it's a freaking huge driver, but this core has a lot of
optional features, three DMA modes, yadda yadda. I actually
removed code for supporting several more recent features before
submitting this! Would it help if I did one patch per file?
Actually no since it's all part of the same driver anyway. I just need
to force myself into going over it all :-)
By the way, I probably should have mentioned this originally. I did not
really write this driver. I took our vendor driver that was written by
another team, and modified it so it would at least have a chance of
being acceptable for the kernel. So I replaced the proprietary license
with BSD/GPL, removed the portability abstraction layer, and fixed up
the coding and comment style. I did fix some logic problems along
that was very clear from the looks of the driver :-) I imagined that was
the case.
the way. And I did become somewhat familiar with how the driver
works while fixing the bugs I introduced while modifying it. But I am
far from being an expert on how it all works.
fair enough :-)
I would like it much better if you would follow what we did for dwc3 and
have core IP in a separate driver with pci/exynos/whatever glue in a
parent device driver. Still, if you really want to make core IP driver a
library, then at a minimum do it right, don't expose ALL functions, make
sure you have a small set of APIs for the glue to call. Encapsulate
implementation details from your users, they really don't need to call
each function separately. Ideally pci glue would call dwc2_initialize()
on probe() and that's it.
Once we get to the point of integrating another interface, like for a
platform driver, I plan to make the core code a separate module, so
the code can be shared instead of being duplicated in each driver. But
we're not quite there yet.
that's fair, but I wonder if it wouldn't be better to save the extra
commits later ?
But I disagree with the small set of APIs thing. It would be a HUGE
amount of effort to rewrite the code like that. The reason the code
is in separate files is to give it a somewhat logical arrangement, and to
avoid having one gargantuan source file. There is no intention to have
hehe, I didn't mean to put it all in one file, I meant that whatever has
to be exposed to other source files, shouldn't get close to the amount
of functions implemented.
Current implementation exposes almost every single function and I think
that's overkill. The small set of APIs are the ones which really have
to be exposed, but implementation details can be hidde.
I mean, instead of exposing (hypothetical situation) event_buff_alloc(),
event_buff_setup(), enable_irq_events(), udc_setup(), etc etc, you can
expose a single dwc3_initialize() and that is already calling the
others properly.
individual APIs for each of the files. I disagree that there are any
external users like you mention above, it is all one driver.
right right, see above.
As a global response to your complaints about all the debug
statements: I said in the first email in the series that I would like to
keep those for the time being, until we have the s3c-hsotg driver
integrated and there are no major issues remaining, and then
strip them all out. I found them _very_ useful for catching my
mistakes after doing the initial conversion from the vendor driver,
and I am sure they will be needed once we start integrating the
s3c-hsotg code.
Yeah, but some of those aren't adding any real information to a debug
session. Those are the ones I complain about, besides you can very
easily add them while debugging if you really want to be that verbose.
Anyway, more comments below.
+void dwc2_enable_global_interrupts(struct dwc2_hcd *hcd)
+{
+ u32 ahbcfg = readl(hcd-regs + GAHBCFG);
+
+ ahbcfg |= GAHBCFG_GlblIntrEn;
+ writel(ahbcfg, hcd-regs + GAHBCFG);
personally, I would add private static inline wrappers around writel()
and readl(), but not a big deal if you want to use directly.
I think I will leave it as is, unless you have a concrete reason for having
a wrapper?
fair enough.
+static void enable_common_interrupts(struct dwc2_hcd *hcd)
even though it's
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Mon, Jan 14, 2013 at 4:22 PM, Oliver Neukum oli...@neukum.org wrote: OK, your trace is totally different. If your hangs are related, as is likely, my explanation goes out of the window. If I run 'shutdown' after unplugging usb storage device, another hang trace same with Alex's can be triggered too, so it should be one same problem. Thanks, -- Ming Lei -- 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
Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h
Hi, On Mon, Jan 14, 2013 at 04:17:35PM +0800, Peter Chen wrote: On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote: It changes the driver to use platform_device_id rather than cpu_is_xxx to determine the SoC type, and updates the platform code accordingly. Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable. Tested at mx51 bbg board, it works ok after enable phy clock (Need another patch to fix this problem) Signed-off-by: Peter Chen peter.c...@freescale.com not good for -rc. You have to break this down as you're solving at least three different problems with this patch. Felipe, all my changes are for this problem, these are fix build error and let it work. arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- As we change the connection-id, we need to change clock file or the devm_clk_get will be failed. right right, that's ok. arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++--- We need to differentiate SoCs, so I use platform_device_id to instead of cpu_ix_xxx(), this is for machine file change fair enough. drivers/usb/gadget/fsl_mxc_udc.c | 23 + drivers/usb/gadget/fsl_udc_core.c | 52 +--- drivers/usb/gadget/fsl_usb2_udc.h | 13 -- include/linux/fsl_devices.h |8 +++ Need to get platform_device_id at driver, and replace the cpu_is_xxx to platform_device_id. Meanwhile, needs a solution for replace MX35_IO_ADDRESS. ok, here we go: You just listed to me three different fixes and each fix should be on a separate patch. Meaning that you should have one single patch to convert MX35_IO_ADDRESS() into ioremap(), another patch should be removing cpu_is_xxx() and the third one fixing connection-id. As you might remember, we want patches to be self-contained logical units and your patch contains 3 of such logical units. Besides there are parts which are not related to the build break. I can split this patch to two patches, one is driver part, the other is machine part. It can also fix build break, but it needs two patches to let the udc work. you will need as many patches as necessary, but they need to the broken correclty, the way you suggested above is wrong. -- balbi signature.asc Description: Digital signature
Re: [PATCH RFC] usb: dwc3: Remove dwc3 dependency on gadget.
Hi Felipe,
On Mon, Jan 14, 2013 at 1:27 PM, Felipe Balbi ba...@ti.com wrote:
On Fri, Jan 11, 2013 at 07:58:23PM +0530, Vivek Gautam wrote:
Hi,
On Fri, Jan 11, 2013 at 7:29 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Fri, Jan 11, 2013 at 07:13:55PM +0530, Vivek Gautam wrote:
On Thu, Jan 10, 2013 at 6:32 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Mon, Dec 24, 2012 at 07:28:33PM +0530, Vivek Gautam wrote:
DWC3 controller curretly depends on CONFIG_USB and CONFIG_USB_GADGET.
Some hardware may like to use only host feature on dwc3.
So, removing the dependency of USB_DWC3 on USB_GADGET
and further modulating the dwc3 core to enable gadget features
only with USB_GADGET.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
CC: Doug Anderson diand...@chromium.org
right, right... Eventually we need to do it, but you're only making
gadget side optional. Host side should be optional too, but then you
need to make sure we don't build dwc3 without gadget and host.
Yes, true we need to make host side also optional, build dwc3 only when
either of host or gadget are built.
btw, make the default Dual-Role, if user/defconfig doesn't select
anything we want to build with all features.
Yes we can try something like this ?
if (USB || USB_GADGET)
no need for this if, actually...
menuconfig USB_DWC3
tristate DesignWare USB3 DRD Core Support
make it a depends on (USB || USB_GAGDGET) here
Yeah, sure.
other than that, it looks correct. Just make sure to compile test in all
options.
Yes ofcourse, i shall compile test all possible options.
if USB_DWC3
choice
default USB_DWC3_DUAL_ROLE_MODE
...
config USB_DWC3_HOST_MODE
this one should depend on USB
yes true.
...
config USB_DWC3_DEVICE_MODE
this one should depend on USB_GADGET
Ok,
...
config USB_DWC3_DUAL_ROLE_MODE
should depend on USB USB_GADGET
Ok
int dwc3_gadget_init(struct dwc3 *dwc);
void dwc3_gadget_exit(struct dwc3 *dwc);
+#else
+static inline int dwc3_gadget_init(struct dwc3 *dwc)
+{ return -EINVAL; }
+static inline void dwc3_gadget_exit(struct dwc3 *dwc)
+{ }
+#endif
#endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index d4a30f1..553bbaa 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -535,7 +535,8 @@ static ssize_t dwc3_testmode_write(struct file
*file,
testmode = 0;
spin_lock_irqsave(dwc-lock, flags);
- dwc3_gadget_set_test_mode(dwc, testmode);
+ if (dwc3_gadget_set_test_mode(dwc, testmode))
+ dev_dbg(dwc-dev, host: Invalid request\n);
spin_unlock_irqrestore(dwc-lock, flags);
return count;
wrong, if you don't have gadget mode, you just don't create this file.
dwc3-core makes call to dwc3_debugfs_init() invariably depending on
DEBUG_FS.
Will this not go ahead and create this file ?
I think i am missing here something. :-(
right, you can change dwc3_debugfs_init() to take into account the fact
that you're a gadget/drd or host-only.
In case of host only, you stil want regdump to be available :-)
So in dwc3_debugfs_init() we shall actually create just 'regdump' file
in case of host only mode, otherwise keep the dwc3_debugfs_init() happy
with creating other files too.
Right ?
right.
Ok so we shall change dwc3_debugfs_init().
Thanks for reviewing and suggesting changes.
--
Thanks Regards
Vivek
--
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
Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h
On Mon, Jan 14, 2013 at 10:50:27AM +0200, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 04:17:35PM +0800, Peter Chen wrote: On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote: It changes the driver to use platform_device_id rather than cpu_is_xxx to determine the SoC type, and updates the platform code accordingly. Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable. Tested at mx51 bbg board, it works ok after enable phy clock (Need another patch to fix this problem) Signed-off-by: Peter Chen peter.c...@freescale.com not good for -rc. You have to break this down as you're solving at least three different problems with this patch. Felipe, all my changes are for this problem, these are fix build error and let it work. arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- As we change the connection-id, we need to change clock file or the devm_clk_get will be failed. right right, that's ok. arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++--- We need to differentiate SoCs, so I use platform_device_id to instead of cpu_ix_xxx(), this is for machine file change fair enough. drivers/usb/gadget/fsl_mxc_udc.c | 23 + drivers/usb/gadget/fsl_udc_core.c | 52 +--- drivers/usb/gadget/fsl_usb2_udc.h | 13 -- include/linux/fsl_devices.h |8 +++ Need to get platform_device_id at driver, and replace the cpu_is_xxx to platform_device_id. Meanwhile, needs a solution for replace MX35_IO_ADDRESS. ok, here we go: You just listed to me three different fixes and each fix should be on a separate patch. Meaning that you should have one single patch to convert MX35_IO_ADDRESS() into ioremap(), another patch should be removing cpu_is_xxx() and the third one fixing connection-id. As you might remember, we want patches to be self-contained logical units and your patch contains 3 of such logical units. OK, three patches. Thanks. The reason I thought two patches is this build break contains two problems, (MX35_IO_ADDRESS cpu_is_xxx()). I would like fix it together, in that case, the git bisect will only show one build error at most. -- Best Regards, Peter Chen -- 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
Re: [PATCH v2 1/1] usb: fsl-mxc-udc: fix build error due to mach/hardware.h
On Mon, Jan 14, 2013 at 05:13:44PM +0800, Peter Chen wrote: On Mon, Jan 14, 2013 at 10:50:27AM +0200, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 04:17:35PM +0800, Peter Chen wrote: On Mon, Jan 14, 2013 at 09:48:58AM +0200, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 03:18:17PM +0800, Peter Chen wrote: It changes the driver to use platform_device_id rather than cpu_is_xxx to determine the SoC type, and updates the platform code accordingly. Compile ok at imx_v6_v7_defconfig with CONFIG_USB_FSL_USB2 enable. Tested at mx51 bbg board, it works ok after enable phy clock (Need another patch to fix this problem) Signed-off-by: Peter Chen peter.c...@freescale.com not good for -rc. You have to break this down as you're solving at least three different problems with this patch. Felipe, all my changes are for this problem, these are fix build error and let it work. arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- As we change the connection-id, we need to change clock file or the devm_clk_get will be failed. right right, that's ok. arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++--- We need to differentiate SoCs, so I use platform_device_id to instead of cpu_ix_xxx(), this is for machine file change fair enough. drivers/usb/gadget/fsl_mxc_udc.c | 23 + drivers/usb/gadget/fsl_udc_core.c | 52 +--- drivers/usb/gadget/fsl_usb2_udc.h | 13 -- include/linux/fsl_devices.h |8 +++ Need to get platform_device_id at driver, and replace the cpu_is_xxx to platform_device_id. Meanwhile, needs a solution for replace MX35_IO_ADDRESS. ok, here we go: You just listed to me three different fixes and each fix should be on a separate patch. Meaning that you should have one single patch to convert MX35_IO_ADDRESS() into ioremap(), another patch should be removing cpu_is_xxx() and the third one fixing connection-id. As you might remember, we want patches to be self-contained logical units and your patch contains 3 of such logical units. OK, three patches. Thanks. The reason I thought two patches is this build break contains two problems, (MX35_IO_ADDRESS cpu_is_xxx()). I would like fix it together, in that case, the git bisect will only show one build error at most. right, still those are two different problems, so they should be fixed in two separate patches, I'm sorry. -- balbi signature.asc Description: Digital signature
Re: [PATCH 07/14] usb: ehci-omap: Instantiate PHY devices if required
On 01/11/2013 06:32 PM, Russell King - ARM Linux wrote:
On Fri, Jan 11, 2013 at 06:03:21PM +0200, Roger Quadros wrote:
diff --git a/include/linux/platform_data/usb-omap.h
b/include/linux/platform_data/usb-omap.h
index d63eb7d..927b8a1 100644
--- a/include/linux/platform_data/usb-omap.h
+++ b/include/linux/platform_data/usb-omap.h
@@ -38,6 +38,12 @@ enum usbhs_omap_port_mode {
OMAP_OHCI_PORT_MODE_TLL_2PIN_DPDM
};
+struct usbhs_phy_config {
+char *name; /* binds to device driver */
You may wish to consider making this const.
OK.
--
cheers,
-roger
--
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
Re: [PATCH 04/14] usb: phy: nop: Handle power supply regulator for the PHY
On 01/11/2013 07:17 PM, Russell King - ARM Linux wrote:
On Thu, Jan 10, 2013 at 06:51:24PM +0200, Roger Quadros wrote:
We use vcc as the supply name for the PHY's power supply.
The power supply will be enabled during .init() and disabled
during .shutdown()
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/otg/nop-usb-xceiv.c | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/otg/nop-usb-xceiv.c
b/drivers/usb/otg/nop-usb-xceiv.c
index 163f972..1c6db10 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -33,11 +33,13 @@
#include linux/usb/nop-usb-xceiv.h
#include linux/slab.h
#include linux/clk.h
+#include linux/regulator/consumer.h
struct nop_usb_xceiv {
struct usb_phy phy;
struct device *dev;
struct clk *clk;
+struct regulator*vcc;
};
static struct platform_device *pd;
@@ -70,6 +72,11 @@ static int nop_init(struct usb_phy *phy)
{
struct nop_usb_xceiv *nop = dev_get_drvdata(phy-dev);
+if (nop-vcc) {
+if (regulator_enable(nop-vcc))
+dev_err(phy-dev, Failed to enable power\n);
+}
+
if (nop-clk)
clk_enable(nop-clk);
@@ -82,6 +89,11 @@ static void nop_shutdown(struct usb_phy *phy)
if (nop-clk)
clk_disable(nop-clk);
+
+if (nop-vcc) {
+if (regulator_disable(nop-vcc))
+dev_err(phy-dev, Failed to disable power\n);
+}
}
static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget
*gadget)
@@ -157,6 +169,12 @@ static int nop_usb_xceiv_probe(struct platform_device
*pdev)
}
}
+nop-vcc = devm_regulator_get(pdev-dev, vcc);
+if (IS_ERR(nop-vcc)) {
+dev_dbg(pdev-dev, Error getting vcc regulator\n);
+nop-vcc = NULL;
+}
Is it really appropriate for drivers to do this kind of thing with
pointer-returning functions (I mean, setting the pointer to NULL on
error, rather than just using a test for IS_ERR() in the above
locations). You are imposing driver-local assumptions on an API.
Practically it probably doesn't make much difference but given the
amount of mistakes that we have with IS_ERR_OR_NULL()...
Makes sense. I'll convert it to use IS_ERR_OR_NULL() throughout.
--
cheers,
-roger
--
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
[PATCH v3 0/3] Fix the Build error for fsl_mxc_udc.c
Changes for v3: - Split the one big patch into three patches Changes for v2: - Add const for fsl_udc_devtype - Do ioremap for phy address at fsl-mxc-udc Peter Chen (3): usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap ARM: i.MX clock: Change the connection-id for fsl-usb2-udc arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++--- drivers/usb/gadget/fsl_mxc_udc.c | 23 + drivers/usb/gadget/fsl_udc_core.c | 52 +--- drivers/usb/gadget/fsl_usb2_udc.h | 13 -- include/linux/fsl_devices.h |8 +++ 11 files changed, 87 insertions(+), 55 deletions(-) -- 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
[PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs.
Besides we update the platform code accordingly.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
arch/arm/mach-imx/devices/devices-common.h|1 +
arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++---
drivers/usb/gadget/fsl_mxc_udc.c | 11 ++--
drivers/usb/gadget/fsl_udc_core.c | 52 +---
drivers/usb/gadget/fsl_usb2_udc.h | 13 --
include/linux/fsl_devices.h |8 +++
6 files changed, 65 insertions(+), 35 deletions(-)
diff --git a/arch/arm/mach-imx/devices/devices-common.h
b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
#include linux/fsl_devices.h
struct imx_fsl_usb2_udc_data {
+ const char *devid;
resource_size_t iobase;
resource_size_t irq;
};
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..fb527c7 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
#include ../hardware.h
#include devices-common.h
-#define imx_fsl_usb2_udc_data_entry_single(soc)
\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)
\
{ \
+ .devid = _devid,\
.iobase = soc ## _USB_OTG_BASE_ADDR,\
.irq = soc ## _INT_USB_OTG, \
}
#ifdef CONFIG_SOC_IMX25
const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX25);
+ imx_fsl_usb2_udc_data_entry_single(MX25, imx-udc-mx25);
#endif /* ifdef CONFIG_SOC_IMX25 */
#ifdef CONFIG_SOC_IMX27
const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX27);
+ imx_fsl_usb2_udc_data_entry_single(MX27, imx-udc-mx27);
#endif /* ifdef CONFIG_SOC_IMX27 */
#ifdef CONFIG_SOC_IMX31
const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX31);
+ imx_fsl_usb2_udc_data_entry_single(MX31, imx-udc-mx31);
#endif /* ifdef CONFIG_SOC_IMX31 */
#ifdef CONFIG_SOC_IMX35
const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX35);
+ imx_fsl_usb2_udc_data_entry_single(MX35, imx-udc-mx35);
#endif /* ifdef CONFIG_SOC_IMX35 */
#ifdef CONFIG_SOC_IMX51
const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX51);
+ imx_fsl_usb2_udc_data_entry_single(MX51, imx-udc-mx51);
#endif
struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
.flags = IORESOURCE_IRQ,
},
};
- return imx_add_platform_device_dmamask(fsl-usb2-udc, -1,
+ return imx_add_platform_device_dmamask(data-devid, -1,
res, ARRAY_SIZE(res),
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
}
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..6df45f7 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
#include linux/platform_device.h
#include linux/io.h
-#include mach/hardware.h
-
static struct clk *mxc_ahb_clk;
static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
@@ -28,7 +26,7 @@ static struct clk *mxc_ipg_clk;
#define USBPHYCTRL_OTGBASE_OFFSET 0x608
#define USBPHYCTRL_EVDO(1 23)
-int fsl_udc_clk_init(struct platform_device *pdev)
+int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata;
unsigned long freq;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
clk_prepare_enable(mxc_per_clk);
/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
- if (!cpu_is_mx51()) {
+ if (!(devtype == IMX51_UDC)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata-phy_mode != FSL_USB2_PHY_ULPI
(freq 5000 || freq 60001000)) {
@@ -79,10 +77,11 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata =
[PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/fsl_mxc_udc.c | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..0e858e6 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
/* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET 0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET 0x8
#define USBPHYCTRL_EVDO(1 23)
int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+ void __iomem *phy_regs = ioremap((unsigned long)pdata-regs +
+ MX35_USBPHYCTRL_OFFSET, 512);
/* workaround ENGcm09152 for i.MX35 */
if (pdata-workaround FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
writel(v | USBPHYCTRL_EVDO,
- MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
}
+ iounmap(phy_regs);
}
/* ULPI transceivers don't need usbpll */
--
1.7.0.4
--
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
[PATCH v3 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
As we use platform_device_id for fsl-usb2-udc driver, it needs to change clk connection-id, or the related devm_clk_get will be failed. Signed-off-by: Peter Chen peter.c...@freescale.com --- arch/arm/mach-imx/clk-imx25.c |6 +++--- arch/arm/mach-imx/clk-imx27.c |6 +++--- arch/arm/mach-imx/clk-imx31.c |6 +++--- arch/arm/mach-imx/clk-imx35.c |6 +++--- arch/arm/mach-imx/clk-imx51-imx53.c |6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c index b197aa7..67e353d 100644 --- a/arch/arm/mach-imx/clk-imx25.c +++ b/arch/arm/mach-imx/clk-imx25.c @@ -254,9 +254,9 @@ int __init mx25_clocks_init(void) clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); clk_register_clkdev(clk[usbotg_ahb], ahb, mxc-ehci.2); clk_register_clkdev(clk[usb_div], per, mxc-ehci.2); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usbotg_ahb], ahb, fsl-usb2-udc); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx25); + clk_register_clkdev(clk[usbotg_ahb], ahb, imx-udc-mx25); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx25); clk_register_clkdev(clk[nfc_ipg_per], NULL, imx25-nand.0); /* i.mx25 has the i.mx35 type cspi */ clk_register_clkdev(clk[cspi1_ipg], NULL, imx35-cspi.0); diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c index 4c1d1e4..1ffe3b5 100644 --- a/arch/arm/mach-imx/clk-imx27.c +++ b/arch/arm/mach-imx/clk-imx27.c @@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref) clk_register_clkdev(clk[lcdc_ahb_gate], ahb, imx21-fb.0); clk_register_clkdev(clk[csi_ahb_gate], ahb, imx27-camera.0); clk_register_clkdev(clk[per4_gate], per, imx27-camera.0); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); - clk_register_clkdev(clk[usb_ipg_gate], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usb_ahb_gate], ahb, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx27); + clk_register_clkdev(clk[usb_ipg_gate], ipg, imx-udc-mx27); + clk_register_clkdev(clk[usb_ahb_gate], ahb, imx-udc-mx27); clk_register_clkdev(clk[usb_div], per, mxc-ehci.0); clk_register_clkdev(clk[usb_ipg_gate], ipg, mxc-ehci.0); clk_register_clkdev(clk[usb_ahb_gate], ahb, mxc-ehci.0); diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c index 8be64e0..ef66eaf 100644 --- a/arch/arm/mach-imx/clk-imx31.c +++ b/arch/arm/mach-imx/clk-imx31.c @@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref) clk_register_clkdev(clk[usb_div_post], per, mxc-ehci.2); clk_register_clkdev(clk[usb_gate], ahb, mxc-ehci.2); clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); - clk_register_clkdev(clk[usb_div_post], per, fsl-usb2-udc); - clk_register_clkdev(clk[usb_gate], ahb, fsl-usb2-udc); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div_post], per, imx-udc-mx31); + clk_register_clkdev(clk[usb_gate], ahb, imx-udc-mx31); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx31); clk_register_clkdev(clk[csi_gate], NULL, mx3-camera.0); /* i.mx31 has the i.mx21 type uart */ clk_register_clkdev(clk[uart1_gate], per, imx21-uart.0); diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c index 66f3d65..69fe9c8 100644 --- a/arch/arm/mach-imx/clk-imx35.c +++ b/arch/arm/mach-imx/clk-imx35.c @@ -251,9 +251,9 @@ int __init mx35_clocks_init() clk_register_clkdev(clk[usb_div], per, mxc-ehci.2); clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); clk_register_clkdev(clk[usbotg_gate], ahb, mxc-ehci.2); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usbotg_gate], ahb, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx35); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx35); + clk_register_clkdev(clk[usbotg_gate], ahb, imx-udc-mx35); clk_register_clkdev(clk[wdog_gate], NULL, imx2-wdt.0); clk_register_clkdev(clk[nfc_div], NULL, imx25-nand.0); clk_register_clkdev(clk[csi_gate], NULL, mx3-camera.0); diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c index 579023f..fb7cb84 100644 --- a/arch/arm/mach-imx/clk-imx51-imx53.c +++ b/arch/arm/mach-imx/clk-imx51-imx53.c @@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil, clk_register_clkdev(clk[usboh3_per_gate], per, mxc-ehci.2); clk_register_clkdev(clk[usboh3_gate], ipg, mxc-ehci.2); clk_register_clkdev(clk[usboh3_gate], ahb, mxc-ehci.2); - clk_register_clkdev(clk[usboh3_per_gate], per, fsl-usb2-udc); -
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+ {
+ .name = imx-udc-mx25,
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = imx-udc-mx27,
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = imx-udc-mx31,
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = imx-udc-mx35,
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = imx-udc-mx51,
+ .driver_data = IMX51_UDC,
+ }
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/fsl_mxc_udc.c | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c
b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..0e858e6 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
/* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET0x8
#define USBPHYCTRL_EVDO (1 23)
int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+ void __iomem *phy_regs = ioremap((unsigned long)pdata-regs +
+ MX35_USBPHYCTRL_OFFSET, 512);
as I said before, this should be passed via struct resource, not pdata.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+{
+.name = imx-udc-mx25,
+.driver_data = IMX25_UDC,
+}, {
+.name = imx-udc-mx27,
+.driver_data = IMX27_UDC,
+}, {
+.name = imx-udc-mx31,
+.driver_data = IMX31_UDC,
+}, {
+.name = imx-udc-mx35,
+.driver_data = IMX35_UDC,
+}, {
+.name = imx-udc-mx51,
+.driver_data = IMX51_UDC,
+}
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+ {
+ .name = imx-udc-mx25,
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = imx-udc-mx27,
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = imx-udc-mx31,
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = imx-udc-mx35,
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = imx-udc-mx51,
+ .driver_data = IMX51_UDC,
+ }
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
--
balbi
signature.asc
Description: Digital signature
[PATCH V2 0/8] usb/dwc3: misc patches rebased at dwc3-for-v3.8
Most of these patches have alreday been posted in past but still they have not been applied to tree. These are rebased on tag dwc3-for-v3.8. Changes since v1: -- Commit log updated for patch usb/dwc3: Fix missed isoc. -- unnecessary parenthesis around list_empty() in patch usb/dwc3: Fix skip LINK-TRB on ISOC removed. Pratyush Anand (8): usb/dwc3: Enable usb2 LPM only when connected as usb2.0 usb/dwc3: Fix missed isoc usb/dwc3: Correct Return from ep_queue usb/dwc3: fix isoc END TRANSFER Condition usb/dwc3: Fix skip LINK-TRB on ISOC usb/dwc3: No need to pass params in case of UPDATE TRANSFER usb/dwc3: Fix scatter gather implementation usb/dwc3: req-queued must be forced to false in cleanup drivers/usb/dwc3/core.h |3 +- drivers/usb/dwc3/gadget.c | 289 +++-- 2 files changed, 177 insertions(+), 115 deletions(-) -- 1.7.5.4 -- 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
[PATCH V2 1/8] usb/dwc3: Enable usb2 LPM only when connected as usb2.0
Synopsys says:
The HIRD Threshold field must be set to ‘0’ when the device core is
operating in super speed mode.
This patch implements above statement.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/gadget.c | 31 ++-
1 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c9c56a1..74d2496 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2156,6 +2156,23 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3
*dwc)
break;
}
+ /* Enable USB2 LPM Capability */
+
+ if ((dwc-revision DWC3_REVISION_194A)
+(speed != DWC3_DCFG_SUPERSPEED)) {
+ reg = dwc3_readl(dwc-regs, DWC3_DCFG);
+ reg |= DWC3_DCFG_LPM_CAP;
+ dwc3_writel(dwc-regs, DWC3_DCFG, reg);
+
+ reg = dwc3_readl(dwc-regs, DWC3_DCTL);
+ reg = ~(DWC3_DCTL_HIRD_THRES_MASK | DWC3_DCTL_L1_HIBER_EN);
+
+ /* TODO: This should be configurable */
+ reg |= DWC3_DCTL_HIRD_THRES(28);
+
+ dwc3_writel(dwc-regs, DWC3_DCTL, reg);
+ }
+
/* Recent versions support automatic phy suspend and don't need this */
if (dwc-revision DWC3_REVISION_194A) {
/* Suspend unneeded PHY */
@@ -2462,20 +2479,8 @@ int __devinit dwc3_gadget_init(struct dwc3 *dwc)
DWC3_DEVTEN_DISCONNEVTEN);
dwc3_writel(dwc-regs, DWC3_DEVTEN, reg);
- /* Enable USB2 LPM and automatic phy suspend only on recent versions */
+ /* automatic phy suspend only on recent versions */
if (dwc-revision = DWC3_REVISION_194A) {
- reg = dwc3_readl(dwc-regs, DWC3_DCFG);
- reg |= DWC3_DCFG_LPM_CAP;
- dwc3_writel(dwc-regs, DWC3_DCFG, reg);
-
- reg = dwc3_readl(dwc-regs, DWC3_DCTL);
- reg = ~(DWC3_DCTL_HIRD_THRES_MASK | DWC3_DCTL_L1_HIBER_EN);
-
- /* TODO: This should be configurable */
- reg |= DWC3_DCTL_HIRD_THRES(28);
-
- dwc3_writel(dwc-regs, DWC3_DCTL, reg);
-
dwc3_gadget_usb2_phy_suspend(dwc, true);
dwc3_gadget_usb3_phy_suspend(dwc, true);
}
--
1.7.5.4
--
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
[PATCH V2 2/8] usb/dwc3: Fix missed isoc
There are two reasons to generate missed isoc.
1. when the host does not poll for all the data.
2. because of application-side delays that prevent all the data from
being transferred in programmed microframe.
Current code was able to handle first case only. This patch handles
scenario 2 as well.Scenario 2 sometime may occur with complex gadget
application, however it can be easily reproduced for testing purpose as
follows:
a. use isoc binterval as 1 in f_sourcesink.
b. use pattern=0
c. introduce a delay of 150us deliberately in source_sink_complete, so
that after few frames it lands into scenario 2.
d. now run testusb 16 (isoc in test). You will notice that if this
patch is not applied then isoc transfer is not able to recover after
first missed.
Current patch's approach is as under:
If missed isoc occurs and there is no request queued then issue END
TRANSFER, so that core generates next xfernotready and we will issue a
fresh START TRANSFER.
If there are still queued request then wait, do not issue either END or
UPDATE TRANSFER, just attach next request in request_list during giveback.
If any future queued request is successfully transferred then we will issue
UPDATE TRANSFER for all request in the request_list.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/core.h |2 --
drivers/usb/dwc3/gadget.c | 36
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4999563..1dae91d 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -405,7 +405,6 @@ struct dwc3_event_buffer {
* @number: endpoint number (1 - 15)
* @type: set to bmAttributes USB_ENDPOINT_XFERTYPE_MASK
* @resource_index: Resource transfer index
- * @current_uf: Current uf received through last event parameter
* @interval: the intervall on which the ISOC transfer is started
* @name: a human readable name e.g. ep1out-bulk
* @direction: true for TX, false for RX
@@ -439,7 +438,6 @@ struct dwc3_ep {
u8 number;
u8 type;
u8 resource_index;
- u16 current_uf;
u32 interval;
charname[20];
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 74d2496..bbbcd2e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1117,16 +1117,6 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
dep-name);
}
- /*
-* 3. Missed ISOC Handling. We need to start isoc transfer on the saved
-* uframe number.
-*/
- if (usb_endpoint_xfer_isoc(dep-endpoint.desc)
- (dep-flags DWC3_EP_MISSED_ISOC)) {
- __dwc3_gadget_start_isoc(dwc, dep, dep-current_uf);
- dep-flags = ~DWC3_EP_MISSED_ISOC;
- }
-
return 0;
}
@@ -1688,14 +1678,29 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc,
struct dwc3_ep *dep,
if (trb_status == DWC3_TRBSTS_MISSED_ISOC) {
dev_dbg(dwc-dev, incomplete IN
transfer %s\n,
dep-name);
- dep-current_uf = event-parameters
- ~(dep-interval - 1);
+ /*
+* If missed isoc occurred and there is
+* no request queued then issue END
+* TRANSFER, so that core generates
+* next xfernotready and we will issue
+* a fresh START TRANSFER.
+* If there are still queued request
+* then wait, do not issue either END
+* or UPDATE TRANSFER, just attach next
+* request in request_list during
+* giveback.If any future queued request
+* is successfully transferred then we
+* will issue UPDATE TRANSFER for all
+* request in the request_list.
+*/
dep-flags |= DWC3_EP_MISSED_ISOC;
} else {
dev_err(dwc-dev, incomplete IN
transfer %s\n,
dep-name);
status = -ECONNRESET;
}
+ }
[PATCH V2 3/8] usb/dwc3: Correct Return from ep_queue
Its better to return from each if condition as they are mutually
exclusive.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/gadget.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index bbbcd2e..4d24711 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1082,8 +1082,6 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
*
*/
if (dep-flags DWC3_EP_PENDING_REQUEST) {
- int ret;
-
/*
* If xfernotready is already elapsed and it is a case
* of isoc transfer, then issue END TRANSFER, so that
@@ -1099,6 +1097,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
if (ret ret != -EBUSY)
dev_dbg(dwc-dev, %s: failed to kick transfers\n,
dep-name);
+ return ret;
}
/*
@@ -1115,6 +1114,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
if (ret ret != -EBUSY)
dev_dbg(dwc-dev, %s: failed to kick transfers\n,
dep-name);
+ return ret;
}
return 0;
--
1.7.5.4
--
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
[PATCH V2 5/8] usb/dwc3: Fix skip LINK-TRB on ISOC
When we reach to link trb, we just need to increase free_slot and then
calculate TRB. Return is not correct, as it will cause wrong TRB DMA
address to fetch in case of update transfer.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/gadget.c | 13 +
1 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index d14bd8b..c7f1cdb 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -754,21 +754,18 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
struct dwc3 *dwc = dep-dwc;
struct dwc3_trb *trb;
- unsigned intcur_slot;
-
dev_vdbg(dwc-dev, %s: req %p dma %08llx length %d%s%s\n,
dep-name, req, (unsigned long long) dma,
length, last ? last : ,
chain ? chain : );
- trb = dep-trb_pool[dep-free_slot DWC3_TRB_MASK];
- cur_slot = dep-free_slot;
- dep-free_slot++;
-
/* Skip the LINK-TRB on ISOC */
- if (((cur_slot DWC3_TRB_MASK) == DWC3_TRB_NUM - 1)
+ if (((dep-free_slot DWC3_TRB_MASK) == DWC3_TRB_NUM - 1)
usb_endpoint_xfer_isoc(dep-endpoint.desc))
- return;
+ dep-free_slot++;
+
+ trb = dep-trb_pool[dep-free_slot DWC3_TRB_MASK];
+ dep-free_slot++;
if (!req-trb) {
dwc3_gadget_move_request_queued(req);
--
1.7.5.4
--
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
[PATCH V2 4/8] usb/dwc3: fix isoc END TRANSFER Condition
There were still some corner cases where isoc transfer was not able to
restart, specially when missed isoc does not happen , and in fact gadget does
not queue any new request during giveback.
Cleanup function calls giveback first, which provides a way to queue
another request to gadget. But gadget did not had any data. So , it did
not call ep_queue. To twist it further, gadget did not queue till
cleanup for last queued TRB is called. If we ever reach this scenario,
we must call END TRANSFER, so that we receive a new xfernotready with
information about current microframe number.
Also insure that there is no request submitted to core when issuing END
TRANSFER.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/gadget.c | 23 ++-
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4d24711..d14bd8b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1089,7 +1089,10 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep,
struct dwc3_request *req)
* notion of current microframe.
*/
if (usb_endpoint_xfer_isoc(dep-endpoint.desc)) {
- dwc3_stop_active_transfer(dwc, dep-number);
+ if (list_empty(dep-req_queued)) {
+ dwc3_stop_active_transfer(dwc, dep-number);
+ dep-flags = DWC3_EP_ENABLED;
+ }
return 0;
}
@@ -1727,10 +1730,20 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc,
struct dwc3_ep *dep,
break;
} while (1);
- if (list_empty(dep-req_queued)
- (dep-flags DWC3_EP_MISSED_ISOC)) {
- dwc3_stop_active_transfer(dwc, dep-number);
- dep-flags = ~DWC3_EP_MISSED_ISOC;
+ if (usb_endpoint_xfer_isoc(dep-endpoint.desc)
+ list_empty(dep-req_queued)) {
+ if (list_empty(dep-request_list)) {
+ /*
+* If there is no entry in request list then do
+* not issue END TRANSFER now. Just set PENDING
+* flag, so that END TRANSFER is issued when an
+* entry is added into request list.
+*/
+ dep-flags = DWC3_EP_PENDING_REQUEST;
+ } else {
+ dwc3_stop_active_transfer(dwc, dep-number);
+ dep-flags = DWC3_EP_ENABLED;
+ }
return 1;
}
--
1.7.5.4
--
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
[PATCH V2 6/8] usb/dwc3: No need to pass params in case of UPDATE TRANSFER
UPDATE transfer does not need any parameters. So, no need to prepare it.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/gadget.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c7f1cdb..0ac2ec3 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -974,13 +974,14 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep
*dep, u16 cmd_param,
}
memset(params, 0, sizeof(params));
- params.param0 = upper_32_bits(req-trb_dma);
- params.param1 = lower_32_bits(req-trb_dma);
- if (start_new)
+ if (start_new) {
+ params.param0 = upper_32_bits(req-trb_dma);
+ params.param1 = lower_32_bits(req-trb_dma);
cmd = DWC3_DEPCMD_STARTTRANSFER;
- else
+ } else {
cmd = DWC3_DEPCMD_UPDATETRANSFER;
+ }
cmd |= DWC3_DEPCMD_PARAM(cmd_param);
ret = dwc3_send_gadget_ep_cmd(dwc, dep-number, cmd, params);
--
1.7.5.4
--
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
[PATCH V2 7/8] usb/dwc3: Fix scatter gather implementation
To work with scatter gather properly, fixes have been done in number of
functions. I will explain requirement of each fixes one by one.
start_slot: used to retrieve all request of SG during cleanup
dwc3_gadget_giveback: We need to skip link TRB if it was one of the
intermediate TRB of SG.
dwc3_prepare_one_trb: We need to track all submitted TRBs during
cleanup. Since, all TRBs would be serially allocated, so we can just
keep starting slot info and we can always find rest of them. We need to
pass sg node number, so that we cab appropriately program ISOC_FIRST/ISOC,
Chain etc.
dwc3_prepare_trbs: last_one should be set when it is last node
of SG as well as last node of request_list.
__dwc3_cleanup_done_trbs: It has been prepared after re-factorization of
dwc3_cleanup_done_reqs. It is called for each TRB of SG.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/usb/dwc3/core.h |1 +
drivers/usb/dwc3/gadget.c | 218 ++---
2 files changed, 127 insertions(+), 92 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1dae91d..9925f29 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -579,6 +579,7 @@ struct dwc3_request {
struct usb_request request;
struct list_headlist;
struct dwc3_ep *dep;
+ u32 start_slot;
u8 epnum;
struct dwc3_trb *trb;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0ac2ec3..4e29e9b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -241,21 +241,22 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
dwc3_request *req,
int status)
{
struct dwc3 *dwc = dep-dwc;
+ int i;
if (req-queued) {
- if (req-request.num_mapped_sgs)
- dep-busy_slot += req-request.num_mapped_sgs;
- else
+ i = 0;
+ do {
dep-busy_slot++;
-
- /*
-* Skip LINK TRB. We can't use req-trb and check for
-* DWC3_TRBCTL_LINK_TRB because it points the TRB we just
-* completed (not the LINK TRB).
-*/
- if (((dep-busy_slot DWC3_TRB_MASK) == DWC3_TRB_NUM - 1)
+ /*
+* Skip LINK TRB. We can't use req-trb and check for
+* DWC3_TRBCTL_LINK_TRB because it points the TRB we
+* just completed (not the LINK TRB).
+*/
+ if (((dep-busy_slot DWC3_TRB_MASK) ==
+ DWC3_TRB_NUM- 1)
usb_endpoint_xfer_isoc(dep-endpoint.desc))
- dep-busy_slot++;
+ dep-busy_slot++;
+ } while(++i req-request.num_mapped_sgs);
}
list_del(req-list);
req-trb = NULL;
@@ -749,7 +750,7 @@ static void dwc3_gadget_ep_free_request(struct usb_ep *ep,
*/
static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
struct dwc3_request *req, dma_addr_t dma,
- unsigned length, unsigned last, unsigned chain)
+ unsigned length, unsigned last, unsigned chain, unsigned node)
{
struct dwc3 *dwc = dep-dwc;
struct dwc3_trb *trb;
@@ -765,14 +766,16 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
dep-free_slot++;
trb = dep-trb_pool[dep-free_slot DWC3_TRB_MASK];
- dep-free_slot++;
if (!req-trb) {
dwc3_gadget_move_request_queued(req);
req-trb = trb;
req-trb_dma = dwc3_trb_dma_offset(dep, trb);
+ req-start_slot = dep-free_slot DWC3_TRB_MASK;
}
+ dep-free_slot++;
+
trb-size = DWC3_TRB_SIZE_LENGTH(length);
trb-bpl = lower_32_bits(dma);
trb-bph = upper_32_bits(dma);
@@ -783,9 +786,12 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
break;
case USB_ENDPOINT_XFER_ISOC:
- trb-ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST;
+ if (!node)
+ trb-ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST;
+ else
+ trb-ctrl = DWC3_TRBCTL_ISOCHRONOUS;
- if (!req-request.no_interrupt)
+ if (!req-request.no_interrupt !chain)
trb-ctrl |= DWC3_TRB_CTRL_IOC;
break;
@@ -804,14 +810,13 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
if (usb_endpoint_xfer_isoc(dep-endpoint.desc)) {
trb-ctrl |= DWC3_TRB_CTRL_ISP_IMI;
trb-ctrl |= DWC3_TRB_CTRL_CSP;
- } else {
- if (chain)
- trb-ctrl |=
[PATCH V2 8/8] usb/dwc3: req-queued must be forced to false in cleanup
I am not sure, why I found it during SG debugging. But, I noticed that even when req_queued list was empty, there were some request in request_list having queued flag true. If I run test second time, it first removes all request from request_list and hence busy_slot was wrongly incremented. Signed-off-by: Pratyush Anand pratyush.an...@st.com --- drivers/usb/dwc3/gadget.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 4e29e9b..b6bd21f 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -257,6 +257,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req, usb_endpoint_xfer_isoc(dep-endpoint.desc)) dep-busy_slot++; } while(++i req-request.num_mapped_sgs); + req-queued = false; } list_del(req-list); req-trb = NULL; -- 1.7.5.4 -- 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
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+ {
+ .name = imx-udc-mx25,
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = imx-udc-mx27,
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = imx-udc-mx31,
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = imx-udc-mx35,
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = imx-udc-mx51,
+ .driver_data = IMX51_UDC,
+ }
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH V2 1/8] usb/dwc3: Enable usb2 LPM only when connected as usb2.0
Hi, On Mon, Jan 14, 2013 at 03:59:31PM +0530, Pratyush Anand wrote: Synopsys says: The HIRD Threshold field must be set to ‘0’ when the device core is operating in super speed mode. This patch implements above statement. why move HIRD settings to another part of the code ? Patch otherwise looks fine, just had this doubt in my mind. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+{
+.name = imx-udc-mx25,
+.driver_data = IMX25_UDC,
+}, {
+.name = imx-udc-mx27,
+.driver_data = IMX27_UDC,
+}, {
+.name = imx-udc-mx31,
+.driver_data = IMX31_UDC,
+}, {
+.name = imx-udc-mx35,
+.driver_data = IMX35_UDC,
+}, {
+.name = imx-udc-mx51,
+.driver_data = IMX51_UDC,
+}
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+{
+.name = imx-udc-mx25,
+.driver_data = IMX25_UDC,
+}, {
+.name = imx-udc-mx27,
+.driver_data = IMX27_UDC,
+}, {
+.name = imx-udc-mx31,
+.driver_data = IMX31_UDC,
+}, {
+.name = imx-udc-mx35,
+.driver_data = IMX35_UDC,
+}, {
+.name = imx-udc-mx51,
+.driver_data = IMX51_UDC,
+}
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
Hi,
On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device
*dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+ {
+ .name = imx-udc-mx25,
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = imx-udc-mx27,
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = imx-udc-mx31,
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = imx-udc-mx35,
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = imx-udc-mx51,
+ .driver_data = IMX51_UDC,
+ }
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 11:53 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device
*dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+ {
+ .name = imx-udc-mx25,
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = imx-udc-mx27,
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = imx-udc-mx31,
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = imx-udc-mx35,
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = imx-udc-mx51,
+ .driver_data = IMX51_UDC,
+ }
+};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.
Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.
just my 2¢
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH V2 1/8] usb/dwc3: Enable usb2 LPM only when connected as usb2.0
I think, Paul can comment better. What I got feedback from SNPS that if HIRD settings are programmed in SS mode, it might result in some weird issue. However, our issue was resolved with some other fix in application and this patch did not had any effect. Since SNPS suggests to follow it so I believe we should include it. Regards Pratyush On 1/14/2013 4:06 PM, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 03:59:31PM +0530, Pratyush Anand wrote: Synopsys says: The HIRD Threshold field must be set to ‘0’ when the device core is operating in super speed mode. This patch implements above statement. why move HIRD settings to another part of the code ? Patch otherwise looks fine, just had this doubt in my mind. -- 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
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:53 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct device
*dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+{
+.name = imx-udc-mx25,
+.driver_data = IMX25_UDC,
+}, {
+.name = imx-udc-mx27,
+.driver_data = IMX27_UDC,
+}, {
+.name = imx-udc-mx31,
+.driver_data = IMX31_UDC,
+}, {
+.name = imx-udc-mx35,
+.driver_data = IMX35_UDC,
+}, {
+.name = imx-udc-mx51,
+.driver_data = IMX51_UDC,
+}
+};
I wonder if your driver-data is actually needed since you can use
string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does
this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.
Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.
just my 2¢
$SUBJECT is trying to differentiate a single feature (or maybe two) to
replace cpu_is_xxx(), then expose that on driver_data without creating
one enum value for each release from fsl.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH V2 1/8] usb/dwc3: Enable usb2 LPM only when connected as usb2.0
Hi, On Mon, Jan 14, 2013 at 04:36:11PM +0530, Pratyush Anand wrote: I think, Paul can comment better. What I got feedback from SNPS that if HIRD settings are programmed in SS mode, it might result in some weird issue. However, our issue was resolved with some other fix in application and this patch did not had any effect. I mean that you're moving it from gadget_init() to conndone irq and I didn't understand why. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH V2 1/8] usb/dwc3: Enable usb2 LPM only when connected as usb2.0
Hi, On 1/14/2013 4:42 PM, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 04:36:11PM +0530, Pratyush Anand wrote: I think, Paul can comment better. What I got feedback from SNPS that if HIRD settings are programmed in SS mode, it might result in some weird issue. However, our issue was resolved with some other fix in application and this patch did not had any effect. I mean that you're moving it from gadget_init() to conndone irq and I didn't understand why. How can we know that core is working in non-SS mode in gadget_init? We can only know about negotiated speed in conndone_irq. Is n't it? Or am I missing something. Regards Pratyush cheers -- 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
Re: [PATCH 04/14] usb: phy: nop: Handle power supply regulator for the PHY
On Mon, Jan 14, 2013 at 11:54:42AM +0200, Roger Quadros wrote:
On 01/11/2013 07:17 PM, Russell King - ARM Linux wrote:
On Thu, Jan 10, 2013 at 06:51:24PM +0200, Roger Quadros wrote:
We use vcc as the supply name for the PHY's power supply.
The power supply will be enabled during .init() and disabled
during .shutdown()
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/otg/nop-usb-xceiv.c | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/otg/nop-usb-xceiv.c
b/drivers/usb/otg/nop-usb-xceiv.c
index 163f972..1c6db10 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -33,11 +33,13 @@
#include linux/usb/nop-usb-xceiv.h
#include linux/slab.h
#include linux/clk.h
+#include linux/regulator/consumer.h
struct nop_usb_xceiv {
struct usb_phy phy;
struct device *dev;
struct clk *clk;
+ struct regulator*vcc;
};
static struct platform_device *pd;
@@ -70,6 +72,11 @@ static int nop_init(struct usb_phy *phy)
{
struct nop_usb_xceiv *nop = dev_get_drvdata(phy-dev);
+ if (nop-vcc) {
+ if (regulator_enable(nop-vcc))
+ dev_err(phy-dev, Failed to enable power\n);
+ }
+
if (nop-clk)
clk_enable(nop-clk);
@@ -82,6 +89,11 @@ static void nop_shutdown(struct usb_phy *phy)
if (nop-clk)
clk_disable(nop-clk);
+
+ if (nop-vcc) {
+ if (regulator_disable(nop-vcc))
+ dev_err(phy-dev, Failed to disable power\n);
+ }
}
static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget
*gadget)
@@ -157,6 +169,12 @@ static int nop_usb_xceiv_probe(struct platform_device
*pdev)
}
}
+ nop-vcc = devm_regulator_get(pdev-dev, vcc);
+ if (IS_ERR(nop-vcc)) {
+ dev_dbg(pdev-dev, Error getting vcc regulator\n);
+ nop-vcc = NULL;
+ }
Is it really appropriate for drivers to do this kind of thing with
pointer-returning functions (I mean, setting the pointer to NULL on
error, rather than just using a test for IS_ERR() in the above
locations). You are imposing driver-local assumptions on an API.
Practically it probably doesn't make much difference but given the
amount of mistakes that we have with IS_ERR_OR_NULL()...
Makes sense. I'll convert it to use IS_ERR_OR_NULL() throughout.
IS_ERR_OR_NULL() is going to be deprecated and removed. Please take a
look at how these APIs work.
If regulator support is not enabled, then devm_regulator_get() returns
NULL, and all the other regulator functions become no-ops. This is not
an error. Do you need the driver to error out if CONFIG_REGULATOR is
disabled?
If regulator support is enabled, then it can return a pointer-error
value (as defined when IS_ERR() is true, and _in that case only_ can it
be decoded by PTR_ERR() - PTR_ERR() is _only_ valid when IS_ERR()
returns true - it is _not_ valid for all the cases that IS_ERR_OR_NULL()
returns true.) Otherwise, it returns a cookie as far as the driver is
concerned for the regulator suitable for passing into the other regulator
APIs.
By having the driver do something different, and make use of NULL as its
own special sentinel, it's placing additional interpretations on the API,
which can lead to bugs. Don't do this.
If you can start making use of 'nop' vcc/clk members before they've been
got then initialize them to ERR_PTR(-EINVAL) first.
Also consider that when using the devm interfaces, you can do:
nop-clk = devm_clk_get(pdev-dev, ...);
nop-vcc = devm_regulator_get(pdev-dev, vcc);
if (IS_ERR(nop-clk))
dev_dbg(pdev-dev, unable to get clock: %d\n,
PTR_ERR(nop-clk));
if (IS_ERR(nop-vcc))
dev_dbg(pdev-dev, unable to get vcc regulator: %d\n,
PTR_ERR(nop-vcc));
...
if (!IS_ERR(nop-clk))
clk_enable(nop-clk);
You may also consider that if you're going to print a warning for
regulator_enable(), that you should print the error code as well, so that
you know the reason why the failure happened.
Also consider... is dev_err() appropriate for an error, for which you
print a message and continue as if nothing went wrong. To me that sounds
more like a warning than an error, so maybe dev_warn() would be more
appropriate?
--
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
Re: [PATCH 01/14] mfd: omap-usb-host: Consolidate OMAP USB-HS platform data
On 01/11/2013 08:13 PM, Tony Lindgren wrote: * Roger Quadros rog...@ti.com [130111 01:43]: Tony, On 01/11/2013 01:45 AM, Tony Lindgren wrote: * Roger Quadros rog...@ti.com [130110 08:54]: Let's have a single platform data structure for the OMAP's High-Speed USB host subsystem instead of having 3 separate ones i.e. one for board data, one for USB Host (UHH) module and one for USB-TLL module. This makes the code much simpler and avoids creating multiple copies of platform data. I can apply just this patch alone into an immutable branch that we all can merge in as needed as long as we have acks for the USB and MFD parts. Or does this one need to be changed based on Alan's comments on the EHCI lib related changes? This does not depend on EHCI lib based changes but it depends on the OMAP USB Host cleanup series posted earlier. Can we first apply just the minimal platform_data + board file + clock changes? We could, but I'll then have to make changes to the patches in the first series and re-post them. Do you want me to do that? That way I can apply those to some immutable tree for everybody to use, and we cut off the dependency to the driver changes for the rest of the patches. And then I'm off the hook for the rest of the patches :) Or you could just ack this patch ;). The platform data is specific to USB host only :) -- cheers, -roger -- 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
Re: [PATCH 04/14] usb: phy: nop: Handle power supply regulator for the PHY
On 01/14/2013 01:25 PM, Russell King - ARM Linux wrote:
On Mon, Jan 14, 2013 at 11:54:42AM +0200, Roger Quadros wrote:
On 01/11/2013 07:17 PM, Russell King - ARM Linux wrote:
On Thu, Jan 10, 2013 at 06:51:24PM +0200, Roger Quadros wrote:
We use vcc as the supply name for the PHY's power supply.
The power supply will be enabled during .init() and disabled
during .shutdown()
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/otg/nop-usb-xceiv.c | 18 ++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/otg/nop-usb-xceiv.c
b/drivers/usb/otg/nop-usb-xceiv.c
index 163f972..1c6db10 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -33,11 +33,13 @@
#include linux/usb/nop-usb-xceiv.h
#include linux/slab.h
#include linux/clk.h
+#include linux/regulator/consumer.h
struct nop_usb_xceiv {
struct usb_phy phy;
struct device *dev;
struct clk *clk;
+ struct regulator*vcc;
};
static struct platform_device *pd;
@@ -70,6 +72,11 @@ static int nop_init(struct usb_phy *phy)
{
struct nop_usb_xceiv *nop = dev_get_drvdata(phy-dev);
+ if (nop-vcc) {
+ if (regulator_enable(nop-vcc))
+ dev_err(phy-dev, Failed to enable power\n);
+ }
+
if (nop-clk)
clk_enable(nop-clk);
@@ -82,6 +89,11 @@ static void nop_shutdown(struct usb_phy *phy)
if (nop-clk)
clk_disable(nop-clk);
+
+ if (nop-vcc) {
+ if (regulator_disable(nop-vcc))
+ dev_err(phy-dev, Failed to disable power\n);
+ }
}
static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget
*gadget)
@@ -157,6 +169,12 @@ static int nop_usb_xceiv_probe(struct platform_device
*pdev)
}
}
+ nop-vcc = devm_regulator_get(pdev-dev, vcc);
+ if (IS_ERR(nop-vcc)) {
+ dev_dbg(pdev-dev, Error getting vcc regulator\n);
+ nop-vcc = NULL;
+ }
Is it really appropriate for drivers to do this kind of thing with
pointer-returning functions (I mean, setting the pointer to NULL on
error, rather than just using a test for IS_ERR() in the above
locations). You are imposing driver-local assumptions on an API.
Practically it probably doesn't make much difference but given the
amount of mistakes that we have with IS_ERR_OR_NULL()...
Makes sense. I'll convert it to use IS_ERR_OR_NULL() throughout.
IS_ERR_OR_NULL() is going to be deprecated and removed. Please take a
look at how these APIs work.
OK. I misunderstood what you meant earlier.
If regulator support is not enabled, then devm_regulator_get() returns
NULL, and all the other regulator functions become no-ops. This is not
an error. Do you need the driver to error out if CONFIG_REGULATOR is
disabled?
No, in fact even if CONFIG_REGULATOR is enabled and regulator_get()
fails, we just print a debug message and continue as usual. So
CONFIG_REGULATOR disabled _and_ regulator_get() error are handled in the
same way.
If regulator support is enabled, then it can return a pointer-error
value (as defined when IS_ERR() is true, and _in that case only_ can it
be decoded by PTR_ERR() - PTR_ERR() is _only_ valid when IS_ERR()
returns true - it is _not_ valid for all the cases that IS_ERR_OR_NULL()
returns true.) Otherwise, it returns a cookie as far as the driver is
concerned for the regulator suitable for passing into the other regulator
APIs.
By having the driver do something different, and make use of NULL as its
own special sentinel, it's placing additional interpretations on the API,
which can lead to bugs. Don't do this.
Agreed.
If you can start making use of 'nop' vcc/clk members before they've been
got then initialize them to ERR_PTR(-EINVAL) first.
Also consider that when using the devm interfaces, you can do:
nop-clk = devm_clk_get(pdev-dev, ...);
nop-vcc = devm_regulator_get(pdev-dev, vcc);
if (IS_ERR(nop-clk))
dev_dbg(pdev-dev, unable to get clock: %d\n,
PTR_ERR(nop-clk));
if (IS_ERR(nop-vcc))
dev_dbg(pdev-dev, unable to get vcc regulator: %d\n,
PTR_ERR(nop-vcc));
...
if (!IS_ERR(nop-clk))
clk_enable(nop-clk);
OK I get it now. The only case where nop-clk or nop-vcc will be NULL
is if the framework is not enabled. It is fine to call the
regulator/clock APIs with NULL pointers in that case.
You may also consider that if you're going to print a warning for
regulator_enable(), that you should print the error code as well, so that
you know the reason why the failure happened.
OK.
Also consider... is dev_err() appropriate for an error, for which you
print a message and continue as if nothing went wrong. To me that sounds
more like a warning than an error, so maybe dev_warn() would be more
appropriate?
I used dev_dbg(),
[PATCH v7 2/4] usb: phy: samsung: Add host phy support to samsung-phy driver
This patch adds host phy support to samsung-usbphy driver and further adds support for samsung's exynos5250 usb-phy. Signed-off-by: Praveen Paneri p.pan...@samsung.com Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- Changes from v6: - Changing macro names from 'HOST_CTRL0_FSEL_CLKSEL_XX' to 'FSEL_CLKSEL_XX' since it's being used by HOST and OTG block to prepare reference clock. - Directly Assigning 'FSEL_CLKSEL_XX' to refclk_freq in samsung_usbphy_get_refclk_freq() instead of ORing them since we are anyways using macros: HOST_CTRL0_FSEL(_x)((_x) 16) OTG_SYS_FSEL(_x) ((_x) 4) .../devicetree/bindings/usb/samsung-usbphy.txt | 12 +- drivers/usb/phy/Kconfig|2 +- drivers/usb/phy/samsung-usbphy.c | 513 ++-- 3 files changed, 496 insertions(+), 31 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt index 22d06cf..0331949 100644 --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt @@ -1,15 +1,23 @@ * Samsung's usb phy transceiver -The Samsung's phy transceiver is used for controlling usb otg phy for -s3c-hsotg usb device controller. +The Samsung's phy transceiver is used for controlling usb phy for +s3c-hsotg as well as ehci-s5p and ohci-exynos usb controllers +across Samsung SOCs. TODO: Adding the PHY binding with controller(s) according to the under developement generic PHY driver. Required properties: + +Exynos4210: - compatible : should be samsung,exynos4210-usbphy - reg : base physical address of the phy registers and length of memory mapped region. +Exynos5250: +- compatible : should be samsung,exynos5250-usbphy +- reg : base physical address of the phy registers and length of memory mapped + region. + Optional properties: - #address-cells: should be '1' when usbphy node has a child node with 'reg' property. diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 36a85b6..fae4d08 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -48,7 +48,7 @@ config USB_RCAR_PHY config SAMSUNG_USBPHY bool Samsung USB PHY controller Driver - depends on USB_S3C_HSOTG + depends on USB_S3C_HSOTG || USB_EHCI_S5P || USB_OHCI_EXYNOS select USB_OTG_UTILS help Enable this to support Samsung USB phy controller for samsung diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c index 7eec7c3..355c6b2 100644 --- a/drivers/usb/phy/samsung-usbphy.c +++ b/drivers/usb/phy/samsung-usbphy.c @@ -5,7 +5,8 @@ * * Author: Praveen Paneri p.pan...@samsung.com * - * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller + * Samsung USB2.0 PHY transceiver; talks to S3C HS OTG controller, EHCI-S5P and + * OHCI-EXYNOS controllers. * * 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 @@ -21,11 +22,13 @@ #include linux/platform_device.h #include linux/clk.h #include linux/delay.h +#include linux/device.h #include linux/err.h #include linux/io.h #include linux/of.h #include linux/of_address.h #include linux/usb/otg.h +#include linux/usb/samsung_usb_phy.h #include linux/platform_data/samsung-usbphy.h /* Register definitions */ @@ -57,24 +60,132 @@ #define RSTCON_HLINK_SWRST (0x1 1) #define RSTCON_SWRST (0x1 0) +/* EXYNOS5 */ +#define EXYNOS5_PHY_HOST_CTRL0 (0x00) + +#define HOST_CTRL0_PHYSWRSTALL (0x1 31) + +#define HOST_CTRL0_REFCLKSEL_MASK (0x3 19) +#define HOST_CTRL0_REFCLKSEL_XTAL (0x0 19) +#define HOST_CTRL0_REFCLKSEL_EXTL (0x1 19) +#define HOST_CTRL0_REFCLKSEL_CLKCORE (0x2 19) + +#define HOST_CTRL0_FSEL_MASK (0x7 16) +#define HOST_CTRL0_FSEL(_x)((_x) 16) + +#define FSEL_CLKSEL_50M(0x7) +#define FSEL_CLKSEL_24M(0x5) +#define FSEL_CLKSEL_20M(0x4) +#define FSEL_CLKSEL_19200K (0x3) +#define FSEL_CLKSEL_12M(0x2) +#define FSEL_CLKSEL_10M(0x1) +#define FSEL_CLKSEL_9600K (0x0) + +#define HOST_CTRL0_TESTBURNIN (0x1 11) +#define HOST_CTRL0_RETENABLE (0x1 10) +#define HOST_CTRL0_COMMONON_N (0x1 9) +#define HOST_CTRL0_SIDDQ (0x1 6) +#define HOST_CTRL0_FORCESLEEP (0x1 5) +#define HOST_CTRL0_FORCESUSPEND(0x1 4) +#define HOST_CTRL0_WORDINTERFACE (0x1 3) +#define
Re: [PATCH 04/14] usb: phy: nop: Handle power supply regulator for the PHY
On Mon, Jan 14, 2013 at 01:51:07PM +0200, Roger Quadros wrote: On 01/14/2013 01:25 PM, Russell King - ARM Linux wrote: Also consider... is dev_err() appropriate for an error, for which you print a message and continue as if nothing went wrong. To me that sounds more like a warning than an error, so maybe dev_warn() would be more appropriate? I used dev_dbg(), because we don't treat not getting the power supply regulator as that serious. This comment is about what you do when regulator_enable() and the like returns non-zero. -- 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
[PATCH v3 0/2] Adding USB 3.0 DRD-phy support for exynos5250
Changes from v2: - Renaming 'samsung-usbphy.c' driver to 'samsung-usb2.c' indicating usb 2.0 phy controller's driver for Samsung's SoCs. - Moving the register definitions and strcuture definitions to common header file 'samsung-usbphy.h' to be used across usb 2.0 and usb 3.0 phy. - Keeping common exported function definitions in samsung-usbphy.c which can be used across usb 2.0 and usb 3.0 phy. - Writting separate driver file for Samsung's USB 3.0 phy controller. and making it dependent on USB_DWC3. Rebased on top of usb-next followed by following patches/patch-threads: -- [PATCH v9 1/2] usb: phy: samsung: Introducing usb phy driver for hsotg -- [PATCH] usb: phy: samsung: Add support to set pmu isolation (version 6) -- [PATCH v6 0/4] Adding usb2.0 host-phy support for exynos5250 Changes form v1: - Moved architecture related patch out of this patch-set. - Replaced unnecessary multi-line macro definitions by single line definitions. - Creating new data structure for USB 3.0 phy type and embedding it in 'samsung_usbphy' structure. - Adding a flag in 'samsung_usbphy' structure to check if device has usb 3.0 type phy or not. - Restructuring probe sequence for USB 3.0 phy, such that we are initializing only when device has usb3.0 type phy. Vivek Gautam (2): usb: phy: samsung: Common out the generic stuff usb: phy: samsung: Add PHY support for USB 3.0 controller drivers/usb/phy/Kconfig |8 + drivers/usb/phy/Makefile |3 +- drivers/usb/phy/samsung-usb2.c | 511 +++ drivers/usb/phy/samsung-usb3.c | 349 +++ drivers/usb/phy/samsung-usbphy.c | 713 +- drivers/usb/phy/samsung-usbphy.h | 328 + 6 files changed, 1205 insertions(+), 707 deletions(-) create mode 100644 drivers/usb/phy/samsung-usb2.c create mode 100644 drivers/usb/phy/samsung-usb3.c create mode 100644 drivers/usb/phy/samsung-usbphy.h -- 1.7.6.5 -- 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
[PATCH v3 1/2] usb: phy: samsung: Common out the generic stuff
Moving register and structure definitions to header file,
and keeping the generic functions to be used across
multiple PHYs in common file samsung-usbphy.c.
Also renaming the usb 2.0 phy driver to samsung-usb2.c
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
drivers/usb/phy/Makefile |2 +-
drivers/usb/phy/samsung-usb2.c | 511 +++
drivers/usb/phy/samsung-usbphy.c | 713 +-
drivers/usb/phy/samsung-usbphy.h | 247 +
4 files changed, 766 insertions(+), 707 deletions(-)
create mode 100644 drivers/usb/phy/samsung-usb2.c
create mode 100644 drivers/usb/phy/samsung-usbphy.h
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index ec304f6..0f4fd4f 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -9,4 +9,4 @@ obj-$(CONFIG_USB_ISP1301) += isp1301.o
obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
-obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o
+obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o samsung-usb2.o
diff --git a/drivers/usb/phy/samsung-usb2.c b/drivers/usb/phy/samsung-usb2.c
new file mode 100644
index 000..ffde341
--- /dev/null
+++ b/drivers/usb/phy/samsung-usb2.c
@@ -0,0 +1,511 @@
+/* linux/drivers/usb/phy/samsung-usbphy.c
+ *
+ * Copyright (c) 2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Author: Praveen Paneri p.pan...@samsung.com
+ *
+ * Samsung USB2.0 PHY transceiver; talks to S3C HS OTG controller, EHCI-S5P and
+ * OHCI-EXYNOS controllers.
+ *
+ * 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.
+ */
+
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/clk.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/err.h
+#include linux/io.h
+#include linux/of.h
+#include linux/usb/otg.h
+#include linux/usb/samsung_usb_phy.h
+#include linux/platform_data/samsung-usbphy.h
+
+#include samsung-usbphy.h
+
+int samsung_usbphy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+ if (!otg)
+ return -ENODEV;
+
+ if (!otg-host)
+ otg-host = host;
+
+ return 0;
+}
+
+static bool exynos5_phyhost_is_on(void *regs)
+{
+ u32 reg;
+
+ reg = readl(regs + EXYNOS5_PHY_HOST_CTRL0);
+
+ return !(reg HOST_CTRL0_SIDDQ);
+}
+
+static void samsung_exynos5_usbphy_enable(struct samsung_usbphy *sphy)
+{
+ void __iomem *regs = sphy-regs;
+ u32 phyclk = sphy-ref_clk_freq;
+ u32 phyhost;
+ u32 phyotg;
+ u32 phyhsic;
+ u32 ehcictrl;
+ u32 ohcictrl;
+
+ /*
+* phy_usage helps in keeping usage count for phy
+* so that the first consumer enabling the phy is also
+* the last consumer to disable it.
+*/
+
+ atomic_inc(sphy-phy_usage);
+
+ if (exynos5_phyhost_is_on(regs)) {
+ dev_info(sphy-dev, Already power on PHY\n);
+ return;
+ }
+
+ /* Host configuration */
+ phyhost = readl(regs + EXYNOS5_PHY_HOST_CTRL0);
+
+ /* phy reference clock configuration */
+ phyhost = ~HOST_CTRL0_FSEL_MASK;
+ phyhost |= HOST_CTRL0_FSEL(phyclk);
+
+ /* host phy reset */
+ phyhost = ~(HOST_CTRL0_PHYSWRST |
+ HOST_CTRL0_PHYSWRSTALL |
+ HOST_CTRL0_SIDDQ |
+ /* Enable normal mode of operation */
+ HOST_CTRL0_FORCESUSPEND |
+ HOST_CTRL0_FORCESLEEP);
+
+ /* Link reset */
+ phyhost |= (HOST_CTRL0_LINKSWRST |
+ HOST_CTRL0_UTMISWRST |
+ /* COMMON Block configuration during suspend */
+ HOST_CTRL0_COMMONON_N);
+ writel(phyhost, regs + EXYNOS5_PHY_HOST_CTRL0);
+ udelay(10);
+ phyhost = ~(HOST_CTRL0_LINKSWRST |
+ HOST_CTRL0_UTMISWRST);
+ writel(phyhost, regs + EXYNOS5_PHY_HOST_CTRL0);
+
+ /* OTG configuration */
+ phyotg = readl(regs + EXYNOS5_PHY_OTG_SYS);
+
+ /* phy reference clock configuration */
+ phyotg = ~OTG_SYS_FSEL_MASK;
+ phyotg |= OTG_SYS_FSEL(phyclk);
+
+ /* Enable normal mode of operation */
+ phyotg = ~(OTG_SYS_FORCESUSPEND |
+ OTG_SYS_SIDDQ_UOTG |
+ OTG_SYS_FORCESLEEP |
+ OTG_SYS_REFCLKSEL_MASK |
+ /* COMMON Block configuration
[PATCH v3 2/2] usb: phy: samsung: Add PHY support for USB 3.0 controller
Adding PHY driver support for USB 3.0 controller for Samsung's
SoCs.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
drivers/usb/phy/Kconfig |8 +
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/samsung-usb3.c | 349 ++
drivers/usb/phy/samsung-usbphy.h | 81 +
4 files changed, 439 insertions(+), 0 deletions(-)
create mode 100644 drivers/usb/phy/samsung-usb3.c
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index fae4d08..a7b0536 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -53,3 +53,11 @@ config SAMSUNG_USBPHY
help
Enable this to support Samsung USB phy controller for samsung
SoCs.
+
+config SAMSUNG_USB3PHY
+ bool Samsung USB 3.0 PHY controller Driver
+ depends on USB_DWC3
+ select USB_OTG_UTILS
+ help
+ Enable this to support Samsung USB 3.0 phy controller for samsung
+ SoCs.
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 0f4fd4f..ed28e89 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o samsung-usb2.o
+obj-$(CONFIG_SAMSUNG_USB3PHY) += samsung-usb3.o
diff --git a/drivers/usb/phy/samsung-usb3.c b/drivers/usb/phy/samsung-usb3.c
new file mode 100644
index 000..8029c1b
--- /dev/null
+++ b/drivers/usb/phy/samsung-usb3.c
@@ -0,0 +1,349 @@
+/* linux/drivers/usb/phy/samsung-usb3.c
+ *
+ * Copyright (c) 2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Author: Vivek Gautam gautam.vi...@samsung.com
+ *
+ * Samsung USB 3.0 PHY transceiver; talks to DWC3 controller.
+ *
+ * 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.
+ */
+
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/clk.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/io.h
+#include linux/of.h
+#include linux/usb/samsung_usb_phy.h
+#include linux/platform_data/samsung-usbphy.h
+
+#include samsung-usbphy.h
+
+/*
+ * Sets the phy clk as EXTREFCLK (XXTI) which is internal clock from clock
core.
+ */
+static u32 samsung_usb3_phy_set_refclk(struct samsung_usbphy *sphy)
+{
+ u32 reg;
+ u32 refclk;
+
+ refclk = sphy-ref_clk_freq;
+
+ reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
+ PHYCLKRST_FSEL(refclk);
+
+ switch (refclk) {
+ case FSEL_CLKSEL_50M:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x00));
+ break;
+ case FSEL_CLKSEL_20M:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x00));
+ break;
+ case FSEL_CLKSEL_19200K:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x88));
+ break;
+ case FSEL_CLKSEL_24M:
+ default:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x88));
+ break;
+ }
+
+ return reg;
+}
+
+static int samsung_exynos5_usb3_phy_enable(struct samsung_usbphy *sphy)
+{
+ void __iomem *regs = sphy-regs;
+ u32 phyparam0;
+ u32 phyparam1;
+ u32 linksystem;
+ u32 phybatchg;
+ u32 phytest;
+ u32 phyclkrst;
+
+ /* Reset USB 3.0 PHY */
+ writel(0x0, regs + EXYNOS5_DRD_PHYREG0);
+
+ phyparam0 = readl(regs + EXYNOS5_DRD_PHYPARAM0);
+ /* Select PHY CLK source */
+ phyparam0 = ~PHYPARAM0_REF_USE_PAD;
+ /* Set Loss-of-Signal Detector sensitivity */
+ phyparam0 = ~PHYPARAM0_REF_LOSLEVEL_MASK;
+ phyparam0 |= PHYPARAM0_REF_LOSLEVEL;
+ writel(phyparam0, regs + EXYNOS5_DRD_PHYPARAM0);
+
+ writel(0x0, regs + EXYNOS5_DRD_PHYRESUME);
+
+ /*
+* Setting the Frame length Adj value[6:1] to default 0x20
+* See xHCI 1.0 spec, 5.2.4
+*/
+ linksystem = LINKSYSTEM_XHCI_VERSION_CONTROL |
+ LINKSYSTEM_FLADJ(0x20);
+ writel(linksystem, regs + EXYNOS5_DRD_LINKSYSTEM);
+
+ phyparam1 = readl(regs + EXYNOS5_DRD_PHYPARAM1);
+ /* Set Tx De-Emphasis level */
+ phyparam1 = ~PHYPARAM1_PCS_TXDEEMPH_MASK;
+ phyparam1 |= PHYPARAM1_PCS_TXDEEMPH;
+ writel(phyparam1, regs +
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On Mon, Jan 14, 2013 at 01:06:00PM +0200, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:53 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
@@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct
device *dev)
return fsl_udc_resume(NULL);
}
-
/*-
Register entry point for the peripheral controller driver
--*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+ {
+ .name = imx-udc-mx25,
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = imx-udc-mx27,
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = imx-udc-mx31,
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = imx-udc-mx35,
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = imx-udc-mx51,
+ .driver_data = IMX51_UDC,
+ }
+};
I wonder if your driver-data is actually needed since you can use
string
comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does
this
for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id-name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.
Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.
just my 2¢
$SUBJECT is trying to differentiate a single feature (or maybe two) to
replace cpu_is_xxx(), then expose that on driver_data without creating
one enum value for each release from fsl.
Felipe, every one or two SoCs may have their special operations for
integrate PHY interface, clk operation, or workaround for IC limitation.
Maybe, it will add more future or SoCs (maybe not for this driver)
in the future, using enum is easier than string comparison for expanding
something.
--
balbi
--
Best Regards,
Peter Chen
--
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
Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
On Mon, Jan 14, 2013 at 12:17:08PM +0200, Felipe Balbi wrote:
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/fsl_mxc_udc.c | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c
b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..0e858e6 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
/* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET 0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET 0x8
#define USBPHYCTRL_EVDO(1 23)
int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device
*pdev)
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+ void __iomem *phy_regs = ioremap((unsigned long)pdata-regs +
+ MX35_USBPHYCTRL_OFFSET, 512);
as I said before, this should be passed via struct resource, not pdata.
My careless, will change at next version
--
balbi
--
Best Regards,
Peter Chen
--
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
Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+ void __iomem *phy_regs = ioremap((unsigned long)pdata-regs +
+ MX35_USBPHYCTRL_OFFSET, 512);
Consider that ioremap() can fail.
--
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
Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
On Mon, Jan 14, 2013 at 01:10:56PM +, Russell King - ARM Linux wrote:
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+ void __iomem *phy_regs = ioremap((unsigned long)pdata-regs +
+ MX35_USBPHYCTRL_OFFSET, 512);
Consider that ioremap() can fail.
Thanks, will check NULL pointer.
--
Best Regards,
Peter Chen
--
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
[PATCH v4 0/3] Fix the Build error for fsl_mxc_udc.c
Changes for v4: - Using pdev's struct resource to do ioremap - Add ioremap return value check Changes for v3: - Split the one big patch into three patches Changes for v2: - Add const for fsl_udc_devtype - Do ioremap for phy address at fsl-mxc-udc Peter Chen (3): usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap ARM: i.MX clock: Change the connection-id for fsl-usb2-udc arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++--- drivers/usb/gadget/fsl_mxc_udc.c | 36 ++ drivers/usb/gadget/fsl_udc_core.c | 54 ++--- drivers/usb/gadget/fsl_usb2_udc.h | 13 -- include/linux/fsl_devices.h |8 +++ 11 files changed, 102 insertions(+), 55 deletions(-) -- 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
[PATCH v4 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs.
Besides we update the platform code accordingly.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
arch/arm/mach-imx/devices/devices-common.h|1 +
arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 +++---
drivers/usb/gadget/fsl_mxc_udc.c | 11 ++--
drivers/usb/gadget/fsl_udc_core.c | 52 +---
drivers/usb/gadget/fsl_usb2_udc.h | 13 --
include/linux/fsl_devices.h |8 +++
6 files changed, 65 insertions(+), 35 deletions(-)
diff --git a/arch/arm/mach-imx/devices/devices-common.h
b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
#include linux/fsl_devices.h
struct imx_fsl_usb2_udc_data {
+ const char *devid;
resource_size_t iobase;
resource_size_t irq;
};
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..fb527c7 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
#include ../hardware.h
#include devices-common.h
-#define imx_fsl_usb2_udc_data_entry_single(soc)
\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)
\
{ \
+ .devid = _devid,\
.iobase = soc ## _USB_OTG_BASE_ADDR,\
.irq = soc ## _INT_USB_OTG, \
}
#ifdef CONFIG_SOC_IMX25
const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX25);
+ imx_fsl_usb2_udc_data_entry_single(MX25, imx-udc-mx25);
#endif /* ifdef CONFIG_SOC_IMX25 */
#ifdef CONFIG_SOC_IMX27
const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX27);
+ imx_fsl_usb2_udc_data_entry_single(MX27, imx-udc-mx27);
#endif /* ifdef CONFIG_SOC_IMX27 */
#ifdef CONFIG_SOC_IMX31
const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX31);
+ imx_fsl_usb2_udc_data_entry_single(MX31, imx-udc-mx31);
#endif /* ifdef CONFIG_SOC_IMX31 */
#ifdef CONFIG_SOC_IMX35
const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX35);
+ imx_fsl_usb2_udc_data_entry_single(MX35, imx-udc-mx35);
#endif /* ifdef CONFIG_SOC_IMX35 */
#ifdef CONFIG_SOC_IMX51
const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX51);
+ imx_fsl_usb2_udc_data_entry_single(MX51, imx-udc-mx51);
#endif
struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
.flags = IORESOURCE_IRQ,
},
};
- return imx_add_platform_device_dmamask(fsl-usb2-udc, -1,
+ return imx_add_platform_device_dmamask(data-devid, -1,
res, ARRAY_SIZE(res),
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
}
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..6df45f7 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
#include linux/platform_device.h
#include linux/io.h
-#include mach/hardware.h
-
static struct clk *mxc_ahb_clk;
static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
@@ -28,7 +26,7 @@ static struct clk *mxc_ipg_clk;
#define USBPHYCTRL_OTGBASE_OFFSET 0x608
#define USBPHYCTRL_EVDO(1 23)
-int fsl_udc_clk_init(struct platform_device *pdev)
+int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata;
unsigned long freq;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
clk_prepare_enable(mxc_per_clk);
/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
- if (!cpu_is_mx51()) {
+ if (!(devtype == IMX51_UDC)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata-phy_mode != FSL_USB2_PHY_ULPI
(freq 5000 || freq 60001000)) {
@@ -79,10 +77,11 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata =
[PATCH v4 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/fsl_mxc_udc.c | 27 +--
drivers/usb/gadget/fsl_udc_core.c |4 +++-
drivers/usb/gadget/fsl_usb2_udc.h |4 ++--
3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..e505d60 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
/* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET 0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET 0x8
#define USBPHYCTRL_EVDO(1 23)
int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -77,28 +78,42 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
+ int ret = 0;
+
if (devtype == IMX35_UDC) {
unsigned int v;
+ struct resource *res = platform_get_resource
+ (pdev, IORESOURCE_MEM, 0);
+ void __iomem *phy_regs = ioremap(res-start +
+ MX35_USBPHYCTRL_OFFSET, 512);
+ if (!phy_regs) {
+ dev_err(pdev-dev, ioremap for phy address fails\n);
+ ret = -EINVAL;
+ goto ioremap_err;
+ }
/* workaround ENGcm09152 for i.MX35 */
if (pdata-workaround FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
writel(v | USBPHYCTRL_EVDO,
- MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
}
+
+ iounmap(phy_regs);
}
+ioremap_err:
/* ULPI transceivers don't need usbpll */
if (pdata-phy_mode == FSL_USB2_PHY_ULPI) {
clk_disable_unprepare(mxc_per_clk);
mxc_per_clk = NULL;
}
+
+ return ret;
}
void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c
b/drivers/usb/gadget/fsl_udc_core.c
index c32119b..4391d49 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2544,7 +2544,9 @@ static int __init fsl_udc_probe(struct platform_device
*pdev)
dr_controller_setup(udc_controller);
}
- fsl_udc_clk_finalize(udc_controller-devtype, pdev);
+ ret = fsl_udc_clk_finalize(udc_controller-devtype, pdev);
+ if (ret)
+ goto err_free_irq;
/* Setup gadget structure */
udc_controller-gadget.ops = fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h
b/drivers/usb/gadget/fsl_usb2_udc.h
index bc1f6d0..7ead5f7 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -594,7 +594,7 @@ static inline struct ep_queue_head *get_qh_by_ep(struct
fsl_ep *ep)
struct platform_device;
#ifdef CONFIG_ARCH_MXC
int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev);
-void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev);
void fsl_udc_clk_release(void);
#else
@@ -603,7 +603,7 @@ static inline int fsl_udc_clk_init(enum fsl_udc_type
devtype,
{
return 0;
}
-static inline void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+static inline int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev)
{
}
--
1.7.0.4
--
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
[PATCH v4 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
As we use platform_device_id for fsl-usb2-udc driver, it needs to change clk connection-id, or the related devm_clk_get will be failed. Signed-off-by: Peter Chen peter.c...@freescale.com --- arch/arm/mach-imx/clk-imx25.c |6 +++--- arch/arm/mach-imx/clk-imx27.c |6 +++--- arch/arm/mach-imx/clk-imx31.c |6 +++--- arch/arm/mach-imx/clk-imx35.c |6 +++--- arch/arm/mach-imx/clk-imx51-imx53.c |6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c index b197aa7..67e353d 100644 --- a/arch/arm/mach-imx/clk-imx25.c +++ b/arch/arm/mach-imx/clk-imx25.c @@ -254,9 +254,9 @@ int __init mx25_clocks_init(void) clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); clk_register_clkdev(clk[usbotg_ahb], ahb, mxc-ehci.2); clk_register_clkdev(clk[usb_div], per, mxc-ehci.2); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usbotg_ahb], ahb, fsl-usb2-udc); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx25); + clk_register_clkdev(clk[usbotg_ahb], ahb, imx-udc-mx25); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx25); clk_register_clkdev(clk[nfc_ipg_per], NULL, imx25-nand.0); /* i.mx25 has the i.mx35 type cspi */ clk_register_clkdev(clk[cspi1_ipg], NULL, imx35-cspi.0); diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c index 4c1d1e4..1ffe3b5 100644 --- a/arch/arm/mach-imx/clk-imx27.c +++ b/arch/arm/mach-imx/clk-imx27.c @@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref) clk_register_clkdev(clk[lcdc_ahb_gate], ahb, imx21-fb.0); clk_register_clkdev(clk[csi_ahb_gate], ahb, imx27-camera.0); clk_register_clkdev(clk[per4_gate], per, imx27-camera.0); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); - clk_register_clkdev(clk[usb_ipg_gate], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usb_ahb_gate], ahb, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx27); + clk_register_clkdev(clk[usb_ipg_gate], ipg, imx-udc-mx27); + clk_register_clkdev(clk[usb_ahb_gate], ahb, imx-udc-mx27); clk_register_clkdev(clk[usb_div], per, mxc-ehci.0); clk_register_clkdev(clk[usb_ipg_gate], ipg, mxc-ehci.0); clk_register_clkdev(clk[usb_ahb_gate], ahb, mxc-ehci.0); diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c index 8be64e0..ef66eaf 100644 --- a/arch/arm/mach-imx/clk-imx31.c +++ b/arch/arm/mach-imx/clk-imx31.c @@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref) clk_register_clkdev(clk[usb_div_post], per, mxc-ehci.2); clk_register_clkdev(clk[usb_gate], ahb, mxc-ehci.2); clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); - clk_register_clkdev(clk[usb_div_post], per, fsl-usb2-udc); - clk_register_clkdev(clk[usb_gate], ahb, fsl-usb2-udc); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div_post], per, imx-udc-mx31); + clk_register_clkdev(clk[usb_gate], ahb, imx-udc-mx31); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx31); clk_register_clkdev(clk[csi_gate], NULL, mx3-camera.0); /* i.mx31 has the i.mx21 type uart */ clk_register_clkdev(clk[uart1_gate], per, imx21-uart.0); diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c index 66f3d65..69fe9c8 100644 --- a/arch/arm/mach-imx/clk-imx35.c +++ b/arch/arm/mach-imx/clk-imx35.c @@ -251,9 +251,9 @@ int __init mx35_clocks_init() clk_register_clkdev(clk[usb_div], per, mxc-ehci.2); clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); clk_register_clkdev(clk[usbotg_gate], ahb, mxc-ehci.2); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usbotg_gate], ahb, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx35); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx35); + clk_register_clkdev(clk[usbotg_gate], ahb, imx-udc-mx35); clk_register_clkdev(clk[wdog_gate], NULL, imx2-wdt.0); clk_register_clkdev(clk[nfc_div], NULL, imx25-nand.0); clk_register_clkdev(clk[csi_gate], NULL, mx3-camera.0); diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c index 579023f..fb7cb84 100644 --- a/arch/arm/mach-imx/clk-imx51-imx53.c +++ b/arch/arm/mach-imx/clk-imx51-imx53.c @@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil, clk_register_clkdev(clk[usboh3_per_gate], per, mxc-ehci.2); clk_register_clkdev(clk[usboh3_gate], ipg, mxc-ehci.2); clk_register_clkdev(clk[usboh3_gate], ahb, mxc-ehci.2); - clk_register_clkdev(clk[usboh3_per_gate], per, fsl-usb2-udc); -
Re: [PATCH V2 1/8] usb/dwc3: Enable usb2 LPM only when connected as usb2.0
On Mon, Jan 14, 2013 at 04:52:58PM +0530, Pratyush Anand wrote: Hi, On 1/14/2013 4:42 PM, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 04:36:11PM +0530, Pratyush Anand wrote: I think, Paul can comment better. What I got feedback from SNPS that if HIRD settings are programmed in SS mode, it might result in some weird issue. However, our issue was resolved with some other fix in application and this patch did not had any effect. I mean that you're moving it from gadget_init() to conndone irq and I didn't understand why. How can we know that core is working in non-SS mode in gadget_init? We can only know about negotiated speed in conndone_irq. Is n't it? Or am I missing something. right, good point. My bad :-) -- balbi signature.asc Description: Digital signature
Re: USB issue with kernel 3.6
On Sun, 13 Jan 2013, Piergiorgio Sartor wrote: Hi Alan, Happy New Year! I hope you had a good holiday break. I removed the previous short patch to ehci-q.c and applied this second one. After some extensive testing, I could not reproduce the issue and dmesg showed the ususl async off and async on messages, the second as soon as I started to read the HDDs. Performance wise, it seems to me better than ever, I mean I saw regular transfer of above 4000 KB/sec per disk. Even with 2 processes reading the 10 HDDs, the data rate was about 3800 KB/sec. Next step? Okay, I figured out what the problem is. It really is a bug in ehci-hcd; the driver doesn't do what the spec requires when unlinking more than one QH at a time. Although the driver could be fixed, it will be easier to stick with the old approach and never unlink more than one QH. Doing two or more at once is pretty rare, so it doesn't much matter if they take a little longer. I'll send a patch that does this properly in the near future, for you to test. Alan Stern -- 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
Re: [Patch] Problem in drivers/usb/serial/io_ti - Kernel oops when disconnecting an opened device
On Thu, Jan 03, 2013 at 11:21:08PM +0100, Wolfgang Frisch wrote:
The attached patch fixes the symptom.
The NULL dereference is caused by chase_port in
drivers/usb/serial/io_ti.c as tty == NULL.
Unfortunately I'm not familiar with the usb-serial system.
I guess the tty is already cleaned up and chase_port shouldn't be called
at this point.
It works for me but I'd like to see the proper solution.
Thanks for identifying and fixing this. The tty is set to NULL when the
port is hanging up and chase_port should have checked for this.
I've prepared a patch series which removes the custom chase_port
function and replaces it with the corresponding generic implementations
instead (which does not suffer from the problem you found).
However, I think your solution is probably the best one for the stable
trees as it is less intrusive.
Care to resubmit your patch with a short description and perhaps the
stack trace from your original report? Have look at
Documentation/SubmittingPatches for details (e.g. you need to add a
Signed-off-by line and should configure you mail client to send the patch
as an inline attachment). Please see my notes on the patch below as
well.
I'll respond to this mail with my series which should also fix the
problem (and which could later be applied on top of your patch). If you
could test it on actual hardware it would be much appreciated.
Thanks,
Johan
On 03/01/13 00:44, Wolfgang Frisch wrote:
I have a problem with my Digi Edgeport USB sensor.
1. Environment:
- Digi Watchport/H USB sensor (io_ti driver)
- Linux v3.7.1 on amd64
Tested with v3.7.1 on 2 physical machines.
Further tests were done in a virtual machine.
2. Observations:
The problem was observed with Linux 3.7.1, 3.2 and 3.1.
I'm not able to find a recent kernel without this problem.
The sensor works until it is disconnected while its character device
still being used. This causes a kernel Oops.
Steps to reproduce:
- Attach Watchport sensor
- Connect, e.g.: minicom -D /dev/ttyUSB0
- Detach the sensor
- Kernel oops
The dmesg log is attached.
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 60023c2..65258c1 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -534,6 +534,11 @@ static void chase_port(struct edgeport_port *port,
unsigned long timeout,
wait_queue_t wait;
unsigned long flags;
+ // FIXME: chase_port is called with tty == NULL
You could drop this comment.
+ if (tty == NULL) {
+ return;
+ }
+
and this should simply be
if (!tty)
return;
if (!timeout)
timeout = (HZ * EDGE_CLOSING_WAIT)/100;
--
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
[PATCH 1/5] USB: io_ti: move write-fifo flushing to close
Move write-fifo flushing from chase_port to close where it belongs.
Signed-off-by: Johan Hovold jhov...@gmail.com
---
drivers/usb/serial/io_ti.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 58184f3..596f8c9 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -521,8 +521,7 @@ exit_is_tx_active:
return bytes_left;
}
-static void chase_port(struct edgeport_port *port, unsigned long timeout,
- int flush)
+static void chase_port(struct edgeport_port *port, unsigned long timeout)
{
int baud_rate;
struct tty_struct *tty = tty_port_tty_get(port-port-port);
@@ -550,8 +549,6 @@ static void chase_port(struct edgeport_port *port, unsigned
long timeout,
}
set_current_state(TASK_RUNNING);
remove_wait_queue(tty-write_wait, wait);
- if (flush)
- kfifo_reset_out(port-write_fifo);
spin_unlock_irqrestore(port-ep_lock, flags);
tty_kref_put(tty);
@@ -1956,6 +1953,7 @@ static void edge_close(struct usb_serial_port *port)
struct edgeport_serial *edge_serial;
struct edgeport_port *edge_port;
struct usb_serial *serial = port-serial;
+ unsigned long flags;
int port_number;
edge_serial = usb_get_serial_data(port-serial);
@@ -1967,12 +1965,14 @@ static void edge_close(struct usb_serial_port *port)
* this flag and dump add read data */
edge_port-close_pending = 1;
- /* chase the port close and flush */
- chase_port(edge_port, (HZ * closing_wait) / 100, 1);
+ chase_port(edge_port, (HZ * closing_wait) / 100);
usb_kill_urb(port-read_urb);
usb_kill_urb(port-write_urb);
edge_port-ep_write_urb_in_use = 0;
+ spin_lock_irqsave(edge_port-ep_lock, flags);
+ kfifo_reset_out(edge_port-write_fifo);
+ spin_unlock_irqrestore(edge_port-ep_lock, flags);
/* assuming we can still talk to the device,
* send a close port command to it */
@@ -2515,7 +2515,7 @@ static void edge_break(struct tty_struct *tty, int
break_state)
int bv = 0; /* Off */
/* chase the port close */
- chase_port(edge_port, 0, 0);
+ chase_port(edge_port, 0);
if (break_state == -1)
bv = 1; /* On */
--
1.8.1
--
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
[PATCH 0/5] USB: io-ti: remove custom closing-wait implementation
Replace custom closing-wait implementation with the corresponding generic implementations. This fixes a null-pointer deref on close as well as access to USB interface after disconnect. Greg, I asked Wolfgang to resubmit his minimal patch for the oops as it is more appropriate for the stable trees. This series could then be applied on top of that minimal fix. Johan Johan Hovold (5): USB: io_ti: move write-fifo flushing to close USB: io_ti: use tty-port drain delay USB: serial: grab disconnect mutex in chars_in_buffer USB: io_ti: query hardware-buffer status in chars_in_buffer USB: io_ti: kill custom closing_wait implementation drivers/usb/serial/io_ti.c | 86 +++-- drivers/usb/serial/usb-serial.c | 14 +-- 2 files changed, 32 insertions(+), 68 deletions(-) -- 1.8.1 -- 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
[PATCH 3/5] USB: serial: grab disconnect mutex in chars_in_buffer
Grab disconnect mutex in chars_in_buffer before checking disconnected
flag or calling driver specific function.
This allows subdrivers to query any hardware buffer status without
having to handle the locking themselves.
Signed-off-by: Johan Hovold jhov...@gmail.com
---
drivers/usb/serial/usb-serial.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 64bda13..0a17f59 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -361,15 +361,21 @@ static int serial_write_room(struct tty_struct *tty)
static int serial_chars_in_buffer(struct tty_struct *tty)
{
struct usb_serial_port *port = tty-driver_data;
+ struct usb_serial *serial = port-serial;
+ int count = 0;
dev_dbg(tty-dev, %s - port %d\n, __func__, port-number);
+ mutex_lock(serial-disc_mutex);
/* if the device was unplugged then any remaining characters
fell out of the connector ;) */
- if (port-serial-disconnected)
- return 0;
- /* pass on to the driver specific version of this function */
- return port-serial-type-chars_in_buffer(tty);
+ if (serial-disconnected)
+ count = 0;
+ else
+ count = serial-type-chars_in_buffer(tty);
+ mutex_unlock(serial-disc_mutex);
+
+ return count;
}
static void serial_throttle(struct tty_struct *tty)
--
1.8.1
--
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
[PATCH 4/5] USB: io_ti: query hardware-buffer status in chars_in_buffer
Query hardware-buffer status in chars_in_buffer should the write fifo be
empty.
This is needed to make the tty layer wait for hardware buffers to drain
on close.
Signed-off-by: Johan Hovold jhov...@gmail.com
---
drivers/usb/serial/io_ti.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 3abbdaa..590f27d 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2088,6 +2088,7 @@ static int edge_chars_in_buffer(struct tty_struct *tty)
struct edgeport_port *edge_port = usb_get_serial_port_data(port);
int chars = 0;
unsigned long flags;
+ int ret;
if (edge_port == NULL)
return 0;
@@ -2098,6 +2099,12 @@ static int edge_chars_in_buffer(struct tty_struct *tty)
chars = kfifo_len(edge_port-write_fifo);
spin_unlock_irqrestore(edge_port-ep_lock, flags);
+ if (!chars) {
+ ret = tx_active(edge_port);
+ if (ret 0)
+ chars = ret;
+ }
+
dev_dbg(port-dev, %s - returns %d\n, __func__, chars);
return chars;
}
--
1.8.1
--
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
[PATCH 2/5] USB: io_ti: use tty-port drain delay
Use tty-port drain delay rather than custom implementation in
chase_port.
Signed-off-by: Johan Hovold jhov...@gmail.com
---
drivers/usb/serial/io_ti.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 596f8c9..3abbdaa 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -523,7 +523,6 @@ exit_is_tx_active:
static void chase_port(struct edgeport_port *port, unsigned long timeout)
{
- int baud_rate;
struct tty_struct *tty = tty_port_tty_get(port-port-port);
struct usb_serial *serial = port-port-serial;
wait_queue_t wait;
@@ -561,17 +560,6 @@ static void chase_port(struct edgeport_port *port,
unsigned long timeout)
break;
msleep(10);
}
-
- /* disconnected */
- if (serial-disconnected)
- return;
-
- /* wait one more character time, based on baud rate */
- /* (tx_active doesn't seem to wait for the last byte) */
- baud_rate = port-baud_rate;
- if (baud_rate == 0)
- baud_rate = 50;
- msleep(max(1, DIV_ROUND_UP(1, baud_rate)));
}
static int choose_config(struct usb_device *dev)
@@ -1938,6 +1926,8 @@ static int edge_open(struct tty_struct *tty, struct
usb_serial_port *port)
++edge_serial-num_ports_open;
+ port-port.drain_delay = 1;
+
goto release_es_lock;
unlink_int_urb:
--
1.8.1
--
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
[PATCH 5/5] USB: io_ti: kill custom closing_wait implementation
Kill custom closing_wait implementation and let the tty layer handle it
instead.
Signed-off-by: Johan Hovold jhov...@gmail.com
---
drivers/usb/serial/io_ti.c | 57 --
1 file changed, 9 insertions(+), 48 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 590f27d..641ab3d 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -521,47 +521,6 @@ exit_is_tx_active:
return bytes_left;
}
-static void chase_port(struct edgeport_port *port, unsigned long timeout)
-{
- struct tty_struct *tty = tty_port_tty_get(port-port-port);
- struct usb_serial *serial = port-port-serial;
- wait_queue_t wait;
- unsigned long flags;
-
- if (!timeout)
- timeout = (HZ * EDGE_CLOSING_WAIT)/100;
-
- /* wait for data to drain from the buffer */
- spin_lock_irqsave(port-ep_lock, flags);
- init_waitqueue_entry(wait, current);
- add_wait_queue(tty-write_wait, wait);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- if (kfifo_len(port-write_fifo) == 0
- || timeout == 0 || signal_pending(current)
- || serial-disconnected)
- /* disconnect */
- break;
- spin_unlock_irqrestore(port-ep_lock, flags);
- timeout = schedule_timeout(timeout);
- spin_lock_irqsave(port-ep_lock, flags);
- }
- set_current_state(TASK_RUNNING);
- remove_wait_queue(tty-write_wait, wait);
- spin_unlock_irqrestore(port-ep_lock, flags);
- tty_kref_put(tty);
-
- /* wait for data to drain from the device */
- timeout += jiffies;
- while ((long)(jiffies - timeout) 0 !signal_pending(current)
-!serial-disconnected) {
- /* not disconnected */
- if (!tx_active(port))
- break;
- msleep(10);
- }
-}
-
static int choose_config(struct usb_device *dev)
{
/*
@@ -1955,8 +1914,6 @@ static void edge_close(struct usb_serial_port *port)
* this flag and dump add read data */
edge_port-close_pending = 1;
- chase_port(edge_port, (HZ * closing_wait) / 100);
-
usb_kill_urb(port-read_urb);
usb_kill_urb(port-write_urb);
edge_port-ep_write_urb_in_use = 0;
@@ -2092,8 +2049,6 @@ static int edge_chars_in_buffer(struct tty_struct *tty)
if (edge_port == NULL)
return 0;
- if (edge_port-close_pending == 1)
- return 0;
spin_lock_irqsave(edge_port-ep_lock, flags);
chars = kfifo_len(edge_port-write_fifo);
@@ -2442,10 +2397,15 @@ static int get_serial_info(struct edgeport_port
*edge_port,
struct serial_struct __user *retinfo)
{
struct serial_struct tmp;
+ unsigned cwait;
if (!retinfo)
return -EFAULT;
+ cwait = edge_port-port-port.closing_wait;
+ if (cwait != ASYNC_CLOSING_WAIT_NONE)
+ cwait = jiffies_to_msecs(closing_wait) / 10;
+
memset(tmp, 0, sizeof(tmp));
tmp.type= PORT_16550A;
@@ -2456,7 +2416,7 @@ static int get_serial_info(struct edgeport_port
*edge_port,
tmp.xmit_fifo_size = edge_port-port-bulk_out_size;
tmp.baud_base = 9600;
tmp.close_delay = 5*HZ;
- tmp.closing_wait= closing_wait;
+ tmp.closing_wait= cwait;
if (copy_to_user(retinfo, tmp, sizeof(*retinfo)))
return -EFAULT;
@@ -2511,8 +2471,7 @@ static void edge_break(struct tty_struct *tty, int
break_state)
int status;
int bv = 0; /* Off */
- /* chase the port close */
- chase_port(edge_port, 0);
+ tty_wait_until_sent(tty, 0);
if (break_state == -1)
bv = 1; /* On */
@@ -2585,6 +2544,8 @@ static int edge_port_probe(struct usb_serial_port *port)
return ret;
}
+ port-port.closing_wait = msecs_to_jiffies(closing_wait * 10);
+
return 0;
}
--
1.8.1
--
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
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Mon, Jan 14, 2013 at 3:39 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sun, 13 Jan 2013, Oliver Neukum wrote: This is not a USB problem. You need to involve the SCSI people. khubd just stops working because disconnects are processed in its context and the removal deadlocks. The why whould building the deadline elevator as a module make any difference? Or does it make a difference? Building elevator as module does make a difference: the system is broken. Alex, if the elevator is made static instead, do you still see the same behavior when the USB drive is removed? How can I make the elevator static? Or did you mean built-in? Or did you mean to ask if khubd hangs if the deadline is built in? In that case - no. The behavior is normal. Nothing hangs. Also, are there any mounted filesystems on the drive when you unplug it? No, no auto-mount. The whole of userspace init is attached, and I'm reasonably sure nothing of it mounts anything automatically. Nothing of udev, too. linuxrc-t Description: Binary data
Re: [PATCH] CDC_NCM adding support IFF_NOARP for infineon modem platform
On Sat, 2013-01-12 at 15:35 -0800, David Miller wrote: From: Wei Shuai cpuw...@gmail.com Date: Sat, 12 Jan 2013 19:34:39 +0800 Infineon(now Intel) HSPA Modem platform NCM cannot support ARP. so I introduce a flag CDC_NCM_DRIVER_DATA_NOARP which is defined in driver_info:data. so later on, if more such buggy devices are found, they could use same flag to handle. Is it no able to do ARP or, the more likely case, does broadcast not work at all? If it's the latter, IFF_NOARP is just making over the real problem. I'm not applying this, no hardware device should set IFF_NOARP. You probably really want IFF_POINTOPOINT or similar. IFF_NOARP is already done for other WWAN devices (sierra_net, hso, cdc-ether, cdc-phonet, lg-vl600, etc) so there is some precedent. Some drivers (phonet, hso) set *both* POINTTOPOINT and NOARP. Is that redundant, and should all WWAN drivers be moved to only POINTTOPOINT? (aside: usbnet has FLAG_POINTTOPOINT, but that's nothing to do with IFF_POINTTOPOINT, it only controls whether the interface is named usbX or ethX. Confusing.) Dan -- 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
Re: [PATCH 01/14] mfd: omap-usb-host: Consolidate OMAP USB-HS platform data
* Roger Quadros rog...@ti.com [130114 03:31]: On 01/11/2013 08:13 PM, Tony Lindgren wrote: * Roger Quadros rog...@ti.com [130111 01:43]: Tony, On 01/11/2013 01:45 AM, Tony Lindgren wrote: * Roger Quadros rog...@ti.com [130110 08:54]: Let's have a single platform data structure for the OMAP's High-Speed USB host subsystem instead of having 3 separate ones i.e. one for board data, one for USB Host (UHH) module and one for USB-TLL module. This makes the code much simpler and avoids creating multiple copies of platform data. I can apply just this patch alone into an immutable branch that we all can merge in as needed as long as we have acks for the USB and MFD parts. Or does this one need to be changed based on Alan's comments on the EHCI lib related changes? This does not depend on EHCI lib based changes but it depends on the OMAP USB Host cleanup series posted earlier. Can we first apply just the minimal platform_data + board file + clock changes? We could, but I'll then have to make changes to the patches in the first series and re-post them. Do you want me to do that? Yes please. Otherwise we'll unnecessarily complicate the dependencies between arch/arm/*omap* code and the drivers. And we've certainly had enough of self-inflicted merge conflicts with the omap usb code already :) That way I can apply those to some immutable tree for everybody to use, and we cut off the dependency to the driver changes for the rest of the patches. And then I'm off the hook for the rest of the patches :) Or you could just ack this patch ;). The platform data is specific to USB host only :) Well it's not just this patch. It's the clock related patches in your earlier seriers that will conflict with any attempts to move the clock data to live under drivers/clk/omap where it needs to go. And the three patches at the end of this series to add platform data (which look fine), but will likely conflict with something else. Let's try do do these changes in a way where the dependencies are cut to minimum where possible. Regards, Tony -- 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
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Sun, Jan 13, 2013 at 11:15 PM, Ming Lei ming@canonical.com wrote: The deadlock problem is caused by calling request_module() inside async function of do_scan_async(), and it was introduced by Linus's below commit: commit d6de2c80e9d758d2e36c21699117db6178c0f517 Author: Linus Torvalds torva...@linux-foundation.org Date: Fri Apr 10 12:17:41 2009 -0700 async: Fix module loading async-work regression IMO, maybe the commit isn't a proper fix, considered the below fact: - it isn't good to allow async function to be marked as __init Immaterial. For modules, __init is a non-issue. For non-modules, the synchronization elsewhere is sufficient. - any user mode shouldn't expect that the device is ready just after completing of 'insmod' Bullshit. That expectation is just a fact. People insmod a device driver, and mount the device immediately in scripts. We do not say user mode shouldn't. Seriously. EVER. User mode *does*, and we deal with it. Learn it now, and stop ever saying that again. This is really starting to annoy me. Kernel developers who say user mode should be fixes to not do that should go somewhere else. The whole and *only* point of a kernel is to hide these kinds of issues from user mode, and make things just work in user mode. User mode should not ever worry about oh, doing X can trigger a module load, so now the device might not be available immediately, so I should delay and re-try until it is. That's just f*cking crazy talk. We have a very simple rule in the kernel: we don't break user space. EVER. Learn that rule. I don't ever want to hear any user mode shouldn't expect again. User mode *does* expect. End of discussion. - from view of driver, introducing async_synchronize_full() after do_one_initcall() inside do_init_module() is like a sync probe for drivers built as module, and cause this kind of deadlock easily. So could we revert the commit and fix the previous problems just case by case? or other better fix? There's no way in hell we take a fix things one by one approach. It's not going to work. And your suggestion seems to not do async discovery of devices in general, which is a *much* worse fix than anything else. It's just crazy. But there are other approaches we might take. We might move the call to async_synchronize_full(); to other places. For example, maybe we're better off doing it at block/char device open instead? Linus -- 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
Re: USB issue with kernel 3.6
Hi Alan, On Mon, Jan 14, 2013 at 10:17:52AM -0500, Alan Stern wrote: On Sun, 13 Jan 2013, Piergiorgio Sartor wrote: Hi Alan, Happy New Year! I hope you had a good holiday break. thanks, holiday was fine, I trust you had some rest too. [...] Next step? Okay, I figured out what the problem is. It really is a bug in ehci-hcd; the driver doesn't do what the spec requires when unlinking more than one QH at a time. Well, I'm happy to read this. Good that you find it! It puzzles me I'm the only one, it seems, affected by this! Although the driver could be fixed, it will be easier to stick with the old approach and never unlink more than one QH. Doing two or more at once is pretty rare, so it doesn't much matter if they take a little longer. I'll send a patch that does this properly in the near future, for you to test. Cool! I'm looking forward to it. Thanks for the support, bye, -- piergiorgio -- 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
Re: close blocks, if FIFO full on gagdet serial
On Sun, 2013-01-13 at 20:09 +0100, mas...@georadis.com wrote:
Hello guys,
I've found a following problems on BeagleBoard and Kernel 3.0.8 with
gadget serial driver.
Attached is a small program, which opens gadget serial tty at
/dev/ttyGS0. USB is NOT connected to host. Then it fills it's output
fifo up. Before closing, it flushes the output fifo. But despite that,
call to close function on last line blocks for 15seconds!
To me it looks like fifo flushing doesn't work. If I do this on regular
serial port on PC and if I remove output fifo flushing, close also
blocks (as I have flow control set). But if the flushing is present,
close returns immediately on PC.
On gadget serial, it always block for 15s.
So it looks to me like a bug in gadget serial driver. Or is there any
other cleanup, which I should do? Or at least some option how to lower
the timeout value?
Sounds like the same issue; you want to look at the serial port
attribute closing_wait. If the driver correctly implements that
attribute (which they all should if they don't already), then in
userspace if you do not want this behavior, you can set closing_wait to
0.
But gadget doesn't implement this; so you'll want to modify
drivers/usb/gadget/u_serial.c to add an ioctl hook to the gs_tty_ops
structure. It should probably just do whatever usb_wwan_ioctl() does.
See:
https://patchwork.kernel.org/patch/1880941/
http://kerneltrap.org/mailarchive/linux-usb/2010/11/19/6266802
http://cgit.freedesktop.org/ModemManager/ModemManager/tree/src/mm-serial-port.c#n901
If the gadget
Dan
thank you and best regards
Petr Masek
#include stdio.h
#include string.h
#include stdio.h
#include termios.h
#include fcntl.h
#include errno.h
int main(void)
{
int fd = open(/dev/ttyGS0, O_NOCTTY | O_RDWR | O_NONBLOCK);
if (fd 0) {
printf(Serial device open error!\n);
return -1;
}
struct termios newtio;
bzero(newtio,sizeof(struct termios));
newtio.c_cflag = B9600 | CS8 | CRTSCTS | CLOCAL | CREAD |
NOFLSH;
newtio.c_iflag = IGNPAR;
newtio.c_oflag = 0;
newtio.c_lflag = 0;
newtio.c_cc[VMIN] = 0;
newtio.c_cc[VTIME] = 0;
tcsetattr(fd,TCSANOW,newtio);
int ret = 0;
while (ret = 0) {
const char buffer[32] = Hello World!\n;
ret = write(fd, buffer, strlen(buffer));
if ((ret 0) (errno == EWOULDBLOCK)) printf(FIFO Full!\n);
}
tcflush(fd, TCOFLUSH);
close(fd);
--
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-usb 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 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
Hi, On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote: snip Usually there isn't any Changelog between IP cores used in the different fsl processors (at least available outside of fsl), that makes it quite difficult to say if something found on one imx is really the same as on the other one. And they (usually) don't provide any versioning information in a register or the documentation. just my 2¢ $SUBJECT is trying to differentiate a single feature (or maybe two) to replace cpu_is_xxx(), then expose that on driver_data without creating one enum value for each release from fsl. Felipe, every one or two SoCs may have their special operations for integrate PHY interface, clk operation, or workaround for IC limitation. the particular PHY and clk used should be hidden by phy layer and clk API respectively. Workarounds, fair enough, we need to handle them; but ideally those should be based on runtime revision detection, not some hackery using driver_data. Maybe, it will add more future or SoCs (maybe not for this driver) in the future, using enum is easier than string comparison for expanding something. a) I never told you to *not* use enum. I said that creating DEVICE_A, DEVICE_B, DEVICE_C, DEVICE_D and DEVICE_E values when DEVICE_B, DEVICE_C and DEVICE_E behave exactly the same is unnecessary. b) you can't be expecting to add future SoCs support to fsl udc, I have already said and will repeat for the last time: move to chipidea ASAP. New SoCs cannot be added to fsl udc, you *must* use chipidea for anything new and move the legacy to chipidea eventually. I will wait for a full year for you to do that, but after that I will have to start deleting drivers for the sake of avoid duplication of effort. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/2] usbnet: allow status interrupt URB to always be active
On Sat, 2013-01-05 at 12:01 +0100, Oliver Neukum wrote: On Friday 04 January 2013 19:26:33 Dan Williams wrote: On Fri, 2013-01-04 at 23:16 +0100, Oliver Neukum wrote: On Friday 04 January 2013 10:48:16 Dan Williams wrote: Some drivers (ex sierra_net) need the status interrupt URB active even when the device is closed, because they receive custom indications from firmware. Allow sub-drivers to set a flag that submits the status interrupt URB on probe and keeps the URB alive over device open/close. The URB is still killed/re-submitted for suspend/resume, as before. Signed-off-by: Dan Williams d...@redhat.com --- Oliver: alternatively, is there a problem with *always* submitting the interrupt URB, and then simply not calling the subdriver's .status function when the netdev is closed? That would be a much simpler patch. That is quite radical. We have no idea what a device does when we do not react to a status update. I would much prefer to not take the risk. Besides, we don't use bandwidth if we don't have to. Ok, so scratch the alternative. Thus, does the posted patch look like the right course of action? In principle yes. If I wasn't clear enough before, sierra_net needs to listen to the status interrupt URB to receive the custom Restart indication as part of the driver's device setup. Thus for sierra_net at least, tying the status interrupt URB submission to device open/close isn't right. So, there seems to be an inevitable race before probe() is called. Have you looked at FLAG_AVOID_UNLINK_URBS ? So that looks like it only applies to the bulk URBs, what was your suggestion here? Sierra would want the same behavior as it currently has (kill data urbs on stop/start) but only the interrupt urb needs to be kept alive over stop/start. Dan -- 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
Re: [PATCH 1/2] usbnet: allow status interrupt URB to always be active
On Fri, 2013-01-11 at 11:06 +0800, Ming Lei wrote: On Sat, Jan 5, 2013 at 9:26 AM, Dan Williams d...@redhat.com wrote: On Fri, 2013-01-04 at 23:16 +0100, Oliver Neukum wrote: On Friday 04 January 2013 10:48:16 Dan Williams wrote: Some drivers (ex sierra_net) need the status interrupt URB active even when the device is closed, because they receive custom indications from firmware. Allow sub-drivers to set a flag that submits the status interrupt URB on probe and keeps the URB alive over device open/close. The URB is still killed/re-submitted for suspend/resume, as before. Signed-off-by: Dan Williams d...@redhat.com --- Oliver: alternatively, is there a problem with *always* submitting the interrupt URB, and then simply not calling the subdriver's .status function when the netdev is closed? That would be a much simpler patch. That is quite radical. We have no idea what a device does when we do not react to a status update. I would much prefer to not take the risk. Besides, we don't use bandwidth if we don't have to. Ok, so scratch the alternative. Thus, does the posted patch look like the right course of action? If I wasn't clear enough before, sierra_net needs to listen to the status interrupt URB to receive the custom Restart indication as part of the driver's device setup. Thus for sierra_net at least, tying the I am curious who are interested in the 'custom Restart indication' information after the interface is closed. It's actually before the interface is even opened. It's really just a sync signal that's part of the driver's setup/initialization of the device. If sierra_net provides ways(such as read registers) to query the indication event, you can just query the information and setup the device in driver_info-reset() during device open, so you can avoid submitting interrupt URB always. As far as I know, it does not, or at least Sierra hasn't released such information about the firmware API of their devices. Dan status interrupt URB submission to device open/close isn't right. In theory, drivers should support to report its link status even it is closed, but looks no much actual usage, so guys opt to submit the interrupt URB only after it is opened. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe netdev 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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: close blocks, if FIFO full on gagdet serial
Hi,
On Sun, Jan 13, 2013 at 08:09:14PM +0100, mas...@georadis.com wrote:
Hello guys,
I've found a following problems on BeagleBoard and Kernel 3.0.8 with
gadget serial driver.
Attached is a small program, which opens gadget serial tty at
/dev/ttyGS0. USB is NOT connected to host. Then it fills it's output
fifo up. Before closing, it flushes the output fifo. But despite
that, call to close function on last line blocks for 15seconds!
To me it looks like fifo flushing doesn't work. If I do this on
regular serial port on PC and if I remove output fifo flushing, close
also blocks (as I have flow control set). But if the flushing is
present, close returns immediately on PC.
On gadget serial, it always block for 15s.
So it looks to me like a bug in gadget serial driver. Or is there any
other cleanup, which I should do? Or at least some option how to
lower the timeout value?
thank you and best regards
I haven't even compile tested this, but something like below is probably
what you need:
diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
index 598dcc1..adba385 100644
--- a/drivers/usb/gadget/u_serial.c
+++ b/drivers/usb/gadget/u_serial.c
@@ -936,8 +936,10 @@ static void gs_flush_chars(struct tty_struct *tty)
pr_vdebug(gs_flush_chars: (%d,%p)\n, port-port_num, tty);
spin_lock_irqsave(port-port_lock, flags);
- if (port-port_usb)
+ if (port-port_usb) {
+ gs_buf_clear(port-port_write_buf);
gs_start_tx(port);
+ }
spin_unlock_irqrestore(port-port_lock, flags);
}
needs proper testing however. If you look at gs_start_tx(), you'll see
that it waits up to 15 seconds for port_write_buf to be cleared. In fact
I think gs_start_tx() shouldn't even be called from gs_flush_chars(),
IIUC, gs_flush_chars() should call usb_ep_dequeue() on every single
pending request and then call gs_buf_clear(), but that'll be a much more
involved work which I'll try to do later this week.
Meanwhile, can you see if patch above breaks anything ?
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On 01/14/2013 06:40 PM, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote: snip Usually there isn't any Changelog between IP cores used in the different fsl processors (at least available outside of fsl), that makes it quite difficult to say if something found on one imx is really the same as on the other one. And they (usually) don't provide any versioning information in a register or the documentation. just my 2¢ $SUBJECT is trying to differentiate a single feature (or maybe two) to replace cpu_is_xxx(), then expose that on driver_data without creating one enum value for each release from fsl. Felipe, every one or two SoCs may have their special operations for integrate PHY interface, clk operation, or workaround for IC limitation. the particular PHY and clk used should be hidden by phy layer and clk API respectively. Workarounds, fair enough, we need to handle them; but ideally those should be based on runtime revision detection, not some hackery using driver_data. If this is actually possible, I'd love to do this. But IP vendor don't include a version register in their cores. :( Maybe, it will add more future or SoCs (maybe not for this driver) in the future, using enum is easier than string comparison for expanding something. a) I never told you to *not* use enum. I said that creating DEVICE_A, DEVICE_B, DEVICE_C, DEVICE_D and DEVICE_E values when DEVICE_B, DEVICE_C and DEVICE_E behave exactly the same is unnecessary. b) you can't be expecting to add future SoCs support to fsl udc, I have already said and will repeat for the last time: move to chipidea ASAP. New SoCs cannot be added to fsl udc, you *must* use chipidea for anything new and move the legacy to chipidea eventually. I will wait for a full year for you to do that, but after that I will have to start deleting drivers for the sake of avoid duplication of effort. +1 Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On Mon, Jan 14, 2013 at 06:54:22PM +0100, Marc Kleine-Budde wrote: On 01/14/2013 06:40 PM, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote: snip Usually there isn't any Changelog between IP cores used in the different fsl processors (at least available outside of fsl), that makes it quite difficult to say if something found on one imx is really the same as on the other one. And they (usually) don't provide any versioning information in a register or the documentation. just my 2¢ $SUBJECT is trying to differentiate a single feature (or maybe two) to replace cpu_is_xxx(), then expose that on driver_data without creating one enum value for each release from fsl. Felipe, every one or two SoCs may have their special operations for integrate PHY interface, clk operation, or workaround for IC limitation. the particular PHY and clk used should be hidden by phy layer and clk API respectively. Workarounds, fair enough, we need to handle them; but ideally those should be based on runtime revision detection, not some hackery using driver_data. If this is actually possible, I'd love to do this. But IP vendor don't include a version register in their cores. :( then fair enough, driver_data or platform_data is the way to go, still my point (a) below is valid. -- balbi signature.asc Description: Digital signature
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Mon, 14 Jan 2013, Linus Torvalds wrote: - from view of driver, introducing async_synchronize_full() after do_one_initcall() inside do_init_module() is like a sync probe for drivers built as module, and cause this kind of deadlock easily. So could we revert the commit and fix the previous problems just case by case? or other better fix? There's no way in hell we take a fix things one by one approach. It's not going to work. And your suggestion seems to not do async discovery of devices in general, which is a *much* worse fix than anything else. It's just crazy. But there are other approaches we might take. We might move the call to async_synchronize_full(); to other places. For example, maybe we're better off doing it at block/char device open instead? How about skipping that call if the current thread is one of the async helpers? Is it possible to detect when that happens? Or maybe such a check should go inside async_synchronize_full() itself. Alan Stern -- 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
Re: [PATCH] CDC_NCM adding support IFF_NOARP for infineon modem platform
From: Dan Williams d...@redhat.com Date: Mon, 14 Jan 2013 11:19:13 -0600 On Sat, 2013-01-12 at 15:35 -0800, David Miller wrote: From: Wei Shuai cpuw...@gmail.com Date: Sat, 12 Jan 2013 19:34:39 +0800 Infineon(now Intel) HSPA Modem platform NCM cannot support ARP. so I introduce a flag CDC_NCM_DRIVER_DATA_NOARP which is defined in driver_info:data. so later on, if more such buggy devices are found, they could use same flag to handle. Is it no able to do ARP or, the more likely case, does broadcast not work at all? If it's the latter, IFF_NOARP is just making over the real problem. I'm not applying this, no hardware device should set IFF_NOARP. You probably really want IFF_POINTOPOINT or similar. IFF_NOARP is already done for other WWAN devices (sierra_net, hso, cdc-ether, cdc-phonet, lg-vl600, etc) so there is some precedent. Some drivers (phonet, hso) set *both* POINTTOPOINT and NOARP. Is that redundant, and should all WWAN drivers be moved to only POINTTOPOINT? (aside: usbnet has FLAG_POINTTOPOINT, but that's nothing to do with IFF_POINTTOPOINT, it only controls whether the interface is named usbX or ethX. Confusing.) I can't answer any of your questions unless you tell me what the real limitation of these devices is. For the second time, is the problem that these devices cannot support broadcast packets properly? -- 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
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Mon, Jan 14, 2013 at 10:04 AM, Alan Stern st...@rowland.harvard.edu wrote: How about skipping that call if the current thread is one of the async helpers? Is it possible to detect when that happens? Or maybe such a check should go inside async_synchronize_full() itself. Do we have some idea of exactly what is waiting for what? Which async context is causing the module load to happen in the first place? I think *that* is what we should avoid - it sounds like the block layer is loading the IO scheduler at the wrong point. I realize that people like (for testing purposes) to change the IO scheduler at random, but if that means that any IO can basically result in a request_module(), then that sounds like a problem. It seems to be elevator_get(), and I presume the chain is something like load block driver async, the block driver does blk_init_allocated_queue, that does request_module() to find the elevator, the request_module() succeeds, but ends up waiting for async work, which is the block driver load, which is waiting for the request_module to finish. And my gut feel is that blk_init_allocated_queue() probably shouldn't do that request_module() at all. We migth want to do it when we *open* the device, but not while loading the module for the device. So my _feeling_ is that this is just a bug in the block layer, and that it shouldn't set up block device drivers for this kind of crazy need to load the elevator module while in the middle of scanning devices. I think *that* is what we should aim to change. Hmm? That said, I think it might indeed be a good idea to make this problem much easier to see, and that detect when it happens would be a good thing (and then we should WARN_ON_ONCE() on people trying to do request_module() calls from async context). Linus -- 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
[PATCH] USB: prevent overlapping access by usb-storage and usbfs
Serialize usb-storage operations with usbfs and 'cat /proc/bus/usb/devices',
so that they cannot disturb storage by seemingly harmless control reads.
This patch was adapted from 2.4.28 patch by Pete Zaitcev -- which I even had to
reconstruct as I have never found it in its final form. That patch dates back
to 2004 and it unfortunately wasn't applied to 2.6 branch in the same form back
then (it was applied in another form and then immediately reverted). Despite 8+
years passing from that moment, the vendors didn't stop producing USB devices
that require this patch. Two recent examples are SanDisk Cruzer Slice 8GB and
Kingston DataTraveller 100 G2 32GB. In the latter case, even the enumeration
fails as the INQUIRY command takes 2.8 seconds to finish, so 'udev' also comes
into action with its control requests, with neither completing normally.
Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com
Cc: sta...@vger.kernel.org
---
This patch is atop of 'usb-linus' branch of Greg's tree...
drivers/usb/core/devices.c | 13 +++--
drivers/usb/core/devio.c|7 +++
drivers/usb/core/usb.c |2 ++
drivers/usb/storage/transport.c | 11 +++
include/linux/usb.h |4
5 files changed, 35 insertions(+), 2 deletions(-)
Index: usb/drivers/usb/core/devices.c
===
--- usb.orig/drivers/usb/core/devices.c
+++ usb/drivers/usb/core/devices.c
@@ -423,21 +423,30 @@ static char *usb_dump_desc(char *start,
if (start end)
return start;
+ /*
+* Grab device's exclusive_access mutex to prevent its driver or
+* devio from using this device while we are accessing it.
+*/
+ mutex_lock(dev-exclusive_access);
+
start = usb_dump_device_descriptor(start, end, dev-descriptor);
if (start end)
- return start;
+ goto out;
start = usb_dump_device_strings(start, end, dev);
for (i = 0; i dev-descriptor.bNumConfigurations; i++) {
if (start end)
- return start;
+ goto out;
start = usb_dump_config(dev-speed,
start, end, dev-config + i,
/* active ? */
(dev-config + i) == dev-actconfig);
}
+
+out:
+ mutex_unlock(dev-exclusive_access);
return start;
}
Index: usb/drivers/usb/core/devio.c
===
--- usb.orig/drivers/usb/core/devio.c
+++ usb/drivers/usb/core/devio.c
@@ -1983,6 +1983,12 @@ static long usbdev_do_ioctl(struct file
return -ENODEV;
}
+ /*
+* Grab device's exclusive_access mutex to prevent its driver from
+* using this device while it is being accessed by us.
+*/
+ mutex_lock(dev-exclusive_access);
+
switch (cmd) {
case USBDEVFS_CONTROL:
snoop(dev-dev, %s: CONTROL\n, __func__);
@@ -2138,6 +2144,7 @@ static long usbdev_do_ioctl(struct file
ret = proc_disconnect_claim(ps, p);
break;
}
+ mutex_unlock(dev-exclusive_access);
usb_unlock_device(dev);
if (ret = 0)
inode-i_atime = CURRENT_TIME;
Index: usb/drivers/usb/core/usb.c
===
--- usb.orig/drivers/usb/core/usb.c
+++ usb/drivers/usb/core/usb.c
@@ -442,6 +442,8 @@ struct usb_device *usb_alloc_dev(struct
dev-parent = parent;
INIT_LIST_HEAD(dev-filelist);
+ mutex_init(dev-exclusive_access);
+
#ifdef CONFIG_PM
pm_runtime_set_autosuspend_delay(dev-dev,
usb_autosuspend_delay * 1000);
Index: usb/drivers/usb/storage/transport.c
===
--- usb.orig/drivers/usb/storage/transport.c
+++ usb/drivers/usb/storage/transport.c
@@ -601,9 +601,18 @@ void usb_stor_invoke_transport(struct sc
int need_auto_sense;
int result;
+ /*
+* Grab device's exclusive_access mutex to prevent libusb/usbfs from
+* sending out a command in the middle of ours (if libusb sends a
+* get_descriptor or something on pipe 0 after our CBW and before
+* our CSW, some devices may malfunction.
+*/
+ mutex_lock(us-pusb_dev-exclusive_access);
+
/* send the command to the transport layer */
scsi_set_resid(srb, 0);
result = us-transport(srb, us);
+ mutex_unlock(us-pusb_dev-exclusive_access);
/* if the command gets aborted by the higher layers, we need to
* short-circuit all other processing
@@ -712,8 +721,10 @@ Retry_Sense:
srb-cmd_len = 12;
/* issue the auto-sense command */
+
Re: [PATCH] USB: prevent overlapping access by usb-storage and usbfs
Hello.
On 01/14/2013 11:36 PM, Sergei Shtylyov wrote:
Serialize usb-storage operations with usbfs and 'cat /proc/bus/usb/devices',
so that they cannot disturb storage by seemingly harmless control reads.
This patch was adapted from 2.4.28 patch by Pete Zaitcev -- which I even had
to
reconstruct as I have never found it in its final form. That patch dates
back
to 2004 and it unfortunately wasn't applied to 2.6 branch in the same form
back
then (it was applied in another form and then immediately reverted). Despite
8+
years passing from that moment, the vendors didn't stop producing USB devices
that require this patch. Two recent examples are SanDisk Cruzer Slice 8GB and
Kingston DataTraveller 100 G2 32GB. In the latter case, even the enumeration
fails as the INQUIRY command takes 2.8 seconds to finish, so 'udev' also comes
into action with its control requests, with neither completing normally.
Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com
Cc: sta...@vger.kernel.org
---
This patch is atop of 'usb-linus' branch of Greg's tree...
drivers/usb/core/devices.c | 13 +++--
drivers/usb/core/devio.c|7 +++
drivers/usb/core/usb.c |2 ++
drivers/usb/storage/transport.c | 11 +++
include/linux/usb.h |4
5 files changed, 35 insertions(+), 2 deletions(-)
Index: usb/drivers/usb/core/devio.c
===
--- usb.orig/drivers/usb/core/devio.c
+++ usb/drivers/usb/core/devio.c
@@ -1983,6 +1983,12 @@ static long usbdev_do_ioctl(struct file
return -ENODEV;
}
+ /*
+ * Grab device's exclusive_access mutex to prevent its driver from
+ * using this device while it is being accessed by us.
+ */
+ mutex_lock(dev-exclusive_access);
+
switch (cmd) {
case USBDEVFS_CONTROL:
snoop(dev-dev, %s: CONTROL\n, __func__);
@@ -2138,6 +2144,7 @@ static long usbdev_do_ioctl(struct file
ret = proc_disconnect_claim(ps, p);
break;
}
+ mutex_unlock(dev-exclusive_access);
usb_unlock_device(dev);
if (ret = 0)
inode-i_atime = CURRENT_TIME;
Forgot to mention the side effect of the patch: one can't submit read and
write URB simultaneously via USBDEVFS_BULK ioctl(). That has been dealt in 2.4
by later patch by Pete, which I can try to port if needed.
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
Re: [PATCH] USB: prevent overlapping access by usb-storage and usbfs
On Mon, 14 Jan 2013, Sergei Shtylyov wrote: Serialize usb-storage operations with usbfs and 'cat /proc/bus/usb/devices', so that they cannot disturb storage by seemingly harmless control reads. This patch was adapted from 2.4.28 patch by Pete Zaitcev -- which I even had to reconstruct as I have never found it in its final form. That patch dates back to 2004 and it unfortunately wasn't applied to 2.6 branch in the same form back then (it was applied in another form and then immediately reverted). Despite 8+ years passing from that moment, the vendors didn't stop producing USB devices that require this patch. Two recent examples are SanDisk Cruzer Slice 8GB and Kingston DataTraveller 100 G2 32GB. In the latter case, even the enumeration fails as the INQUIRY command takes 2.8 seconds to finish, so 'udev' also comes into action with its control requests, with neither completing normally. Without commenting on the wisdom of this patch, there are a couple of places where the comments could be improved. --- usb.orig/drivers/usb/core/devices.c +++ usb/drivers/usb/core/devices.c @@ -423,21 +423,30 @@ static char *usb_dump_desc(char *start, if (start end) return start; + /* + * Grab device's exclusive_access mutex to prevent its driver or + * devio from using this device while we are accessing it. s/devio/usbfs/ --- usb.orig/drivers/usb/storage/transport.c +++ usb/drivers/usb/storage/transport.c @@ -601,9 +601,18 @@ void usb_stor_invoke_transport(struct sc int need_auto_sense; int result; + /* + * Grab device's exclusive_access mutex to prevent libusb/usbfs from + * sending out a command in the middle of ours (if libusb sends a + * get_descriptor or something on pipe 0 after our CBW and before s/pipe 0/ep0/ Alan Stern -- 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
Re: [PATCH] USB: prevent overlapping access by usb-storage and usbfs
On Mon, Jan 14, 2013 at 11:42:09PM +0300, Sergei Shtylyov wrote: Hello. On 01/14/2013 11:36 PM, Sergei Shtylyov wrote: Serialize usb-storage operations with usbfs and 'cat /proc/bus/usb/devices', so that they cannot disturb storage by seemingly harmless control reads. This patch was adapted from 2.4.28 patch by Pete Zaitcev -- which I even had to reconstruct as I have never found it in its final form. That patch dates back to 2004 and it unfortunately wasn't applied to 2.6 branch in the same form back then (it was applied in another form and then immediately reverted). Despite 8+ years passing from that moment, the vendors didn't stop producing USB devices that require this patch. Two recent examples are SanDisk Cruzer Slice 8GB and Kingston DataTraveller 100 G2 32GB. In the latter case, even the enumeration fails as the INQUIRY command takes 2.8 seconds to finish, so 'udev' also comes into action with its control requests, with neither completing normally. Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com Cc: sta...@vger.kernel.org Forgot to mention the side effect of the patch: one can't submit read and write URB simultaneously via USBDEVFS_BULK ioctl(). That has been dealt in 2.4 by later patch by Pete, which I can try to port if needed. That's not good, it would need to be part of this patch, we don't want to break that existing functionality. greg k-h -- 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
Re: [PATCH v9] usb_8dev: Add support for USB2CAN interface from 8 devices
Hi Bernd, On 16.12.2012 08:01, Bernd Krumboeck wrote: Add device driver for USB2CAN interface from 8 devices (http://www.8devices.com). changes since v8: * remove all sysfs files changes since v7: * add sysfs documentation * fix minor styling issue * fixed can state for passive mode * changed handling for crc errors changes since v6: * changed some variable types to big endian equivalent * small cleanups changes since v5: * unlock mutex on error changes since v4: * removed FSF address * renamed struct usb_8dev * removed unused variable free_slots * replaced some _to_cpu functions with pointer equivalent * fix return value for usb_8dev_set_mode * handle can errors with separate function * fix overrun error handling * rewrite error handling for usb_8dev_start_xmit * fix urb submit in usb_8dev_start * various small fixes Signed-off-by: Bernd Krumboeck krumbo...@universalnet.at Acked-by: Wolfgang Grandegger w...@grandegger.com Tested-by: Oliver Hartkopp socket...@hartkopp.net Fortunately i got my adapter today ... [ 52.984254] usb 2-1.4: new full-speed USB device number 6 using ehci-pci [ 53.078690] usb 2-1.4: New USB device found, idVendor=0483, idProduct=1234 [ 53.078698] usb 2-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 53.078703] usb 2-1.4: Product: USB2CAN converter [ 53.078707] usb 2-1.4: Manufacturer: edevices [ 53.078711] usb 2-1.4: SerialNumber: ED000212 [ 53.111990] usb_8dev 2-1.4:1.0 can2: firmware: 1.4, hardware: 255.255 [ 53.112090] usbcore: registered new interface driver usb_8dev Looks good :-) When detaching the device from the CAN bus when sending/receiving CAN traffic i got these dmesg infos: [ 960.047130] usb_8dev 2-1.4:1.0 can2: Unknown status/error message (0) [ 976.544343] usb_8dev 2-1.4:1.0 can2: Unknown status/error message (0) Did you check these kind of 'unfriendly user' tests? E.g. pulling the USB under receive load brings this output: [ 1343.786427] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) [ 1343.786640] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) (..) another 344 of these URB aborted messages [ 1343.872620] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) [ 1343.872867] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) [ 1343.873124] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) [ 1343.873269] usb 2-1.4: USB disconnect, device number 6 [ 1343.873363] usb_8dev 2-1.4:1.0 can2: Rx URB aborted (-32) [ 1343.875259] usb_8dev 2-1.4:1.0 can2: device disconnected [ 1343.875366] usb_8dev 2-1.4:1.0 can2: sending command message failed [ 1343.875371] usb_8dev 2-1.4:1.0 can2: couldn't stop device Tomorrow i will do some more testing. The LED handling of the device is really fine: - interface down - red - interface up - green - interface error passive - green blinking Regards, Oliver -- 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
Re: xhci problem
On Sun, Jan 13, 2013 at 09:34:58AM -0500, Allan Dennis wrote: I left this problem for awhile, then finally got back to it. I upgraded the gentoo kernel to 3.6.11 and was partially successful: the 3TB drive mounted ok, but then had some serious trouble transferring files and I believe that linux force-unmounted it as a result. Then upgrading to 3.7.1 fixed the problem completely. Here is the dmesg output now: Good to hear that upgrading helped. It would be nice to know which commit fixed your issue, so that we can make sure any Linux distributions include it in their stable releases. Do you have time to git bisect the changes between 3.6 and 3.7? Sarah Sharp [4.787051] scsi 7:0:0:0: Direct-Access ST330006 51AS CC45 PQ: 0 ANSI: 0 [4.787176] sd 7:0:0:0: Attached scsi generic sg2 type 0 [4.787416] sd 7:0:0:0: [sdc] Very big device. Trying to use READ CAPACITY(16). [4.787998] sd 7:0:0:0: [sdc] 5860533168 512-byte logical blocks: (3.00 TB/2.72 TiB) [4.788405] sd 7:0:0:0: [sdc] Write Protect is off [4.788408] sd 7:0:0:0: [sdc] Mode Sense: 03 00 00 00 [4.76] sd 7:0:0:0: [sdc] No Caching mode page present [4.78] sd 7:0:0:0: [sdc] Assuming drive cache: write through [4.789246] sd 7:0:0:0: [sdc] Very big device. Trying to use READ CAPACITY(16). [4.790420] sd 7:0:0:0: [sdc] No Caching mode page present [4.790424] sd 7:0:0:0: [sdc] Assuming drive cache: write through [4.841399] sdc: sdc1 [4.841883] sd 7:0:0:0: [sdc] Very big device. Trying to use READ CAPACITY(16). [4.843331] sd 7:0:0:0: [sdc] No Caching mode page present [4.843334] sd 7:0:0:0: [sdc] Assuming drive cache: write through [4.843336] sd 7:0:0:0: [sdc] Attached SCSI disk Thanks for your help! On Wed, 2012-12-05 at 09:24 -0800, Sarah Sharp wrote: On Tue, Dec 04, 2012 at 08:28:30PM -0500, Allan Dennis wrote: I hope you won't be annoyed by my question... here goes: It's my job to answer questions, so fire away. :) I have 2TB and 3TB SATA drives, both connected to my Intense PC (Ivy Bridge tiny PC) on separate external drive enclosures on USB 3 cables. I have gentoo linux, and was running kernel 3.4.9 where both drives weren't recognized properly. Now I updated to 3.5.7 and the 2TB is good to go (yay!) but the 3TB guy is still giving me problems. Here is the dmesg output as I plugged in the 3TB (sdc) after the 2TB (sdb) was successful, with debug enabled in the kernel config. Is this a known issue? Can I help you figure out what's going wrong here by providing logs or whatnot? I can't see anything wrong on the xHCI side, so I suspect the problem might be that the SCSI layer doesn't like some response the hard drive is sending. I would suggest you take a usbmon trace, following the directions here: http://lxr.linux.no/#linux/Documentation/usb/usbmon.txt Please hit 'reply-all' when you send the trace, since I've Cced the linux-usb and usb-storage mailing lists. Sarah Sharp -- 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
Re: [PATCH] usb: phy: samsung: Add support to set pmu isolation
Vivek,
Sorry for being so absent from these reviews. I'll try to look over a
few patches today, but please don't hold up anything on account of my
reviews. I'm definitely a bit of an interested bystander in USB land.
;)
In general things look pretty good here. :) One last comment below...
On Fri, Jan 11, 2013 at 12:08 AM, Vivek Gautam
gautam.vi...@samsung.com wrote: +static int
samsung_usbphy_parse_dt(struct samsung_usbphy *sphy)
+{
+ struct device_node *usbphy_sys;
+
+ /* Getting node for system controller interface for usb-phy */
+ usbphy_sys = of_get_child_by_name(sphy-dev-of_node, usbphy-sys);
+ if (!usbphy_sys)
+ dev_warn(sphy-dev, No sys-controller interface for
usb-phy\n);
Seems like you ought to return with an error here. Calling of_iomap()
with a NULL value seems undesirable.
+
+ sphy-pmuregs = of_iomap(usbphy_sys, 0);
+
+ of_node_put(usbphy_sys);
+
+ if (sphy-pmuregs == NULL) {
+ dev_err(sphy-dev, Can't get usb-phy pmu control
register\n);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+/*
+ * Set isolation here for phy.
+ * Here 'on = true' would mean USB PHY block is isolated, hence
+ * de-activated and vice-versa.
+ */
Thank you very much for this comment. :) This explains one of the
confusions I had earlier...
Once you fix the one error condition above you can add my
Reviewed-by. Thanks!
-Doug
--
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
Re: [PATCH v5 2/4] usb: phy: samsung: Add host phy support to samsung-phy driver
Vivek, On Mon, Jan 14, 2013 at 12:06 AM, Vivek Gautam gautamvivek1...@gmail.com wrote: Is it fine if we don't use macro for SHIFT, earlier code also doesn't use it. Can we just do like this .. #define HOST_CTRL0_FSEL_MASK (0x7 16) #define HOST_CTRL0_FSEL_CLKSEL_50M(0x7 16) #define HOST_CTRL0_FSEL_CLKSEL_24M(0x5 16) #define HOST_CTRL0_FSEL_CLKSEL_20M(0x4 16) #define HOST_CTRL0_FSEL_CLKSEL_19200K (0x3 16) #define HOST_CTRL0_FSEL_CLKSEL_12M(0x2 16) #define HOST_CTRL0_FSEL_CLKSEL_10M(0x1 16) #define HOST_CTRL0_FSEL_CLKSEL_9600K (0x0 16) I don't have a problem with just putting the 16 there... Actually missed one thing here, this HOST_CTRL0_FSEL_CLKSEL_XX is being used by HOST/OTG blocks to prepare reference clock, that's the reason we kept #define HOST_CTRL0_FSEL(_x) ((_x) 16) and #define OTG_SYS_FSEL(_x) ((_x) 4) where (_x) is the reference clock returned from samsung_usbphy_get_refclk_freq(). I feel like samsung_usbphy_get_refclk_freq() is supposed to be returning an already shifted value. ...at least that's how it appears to work with existing code. Why does it feel like that? ...because with existing code we have: #define PHYCLK_CLKSEL_MASK (0x3 0) #define PHYCLK_CLKSEL_48M (0x0 0) #define PHYCLK_CLKSEL_12M (0x2 0) #define PHYCLK_CLKSEL_24M (0x3 0) Those #defines are what are returned by samsung_usbphy_get_refclk_freq(). The fact that the 0 is there implies (to me) that the existing function would return shifted values if it were applicable. For exynos you are having the same function return things like: #define FSEL_CLKSEL_50M (0x7) #define FSEL_CLKSEL_24M (0x5) #define FSEL_CLKSEL_20M (0x4) ...to me that means that the exynos code is returning unshifted values. If you say that samsung_usbphy_get_refclk_freq() is in charge of returning shifted values then you no longer need the HOST_CTRL0_FSEL() macro. In any case, I will defer to whatever Felipe thinks is best here (if he has an opinion on it). I am only pointing out that the exynos code and existing code seem to be specifying things differently. That's weird to me. so we can change this to something like case 10 * MHZ: refclk_freq = FSEL_CLKSEL_10M; break; and so on. will this be fine ? Seems good to me. -Doug -- 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
Re: [PATCH] CDC_NCM adding support IFF_NOARP for infineon modem platform
David Miller wrote: should all WWAN drivers be moved to only POINTTOPOINT? I can't answer any of your questions unless you tell me what the real limitation of these devices is. It's rather about the network than any given devices, right? //Peter -- 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
Re: [Resend PATCH V4 0/10] usb: usb port power off mechanism anc expose usb port connect type
On 2013年01月11日 20:10, Lan Tianyu wrote: Change since v1: optimize the export connect type patch and adjust the DeviceRemovalbe flag in the rh_call_control() after GetHubDescriptor request being processed. move all debounce operation to usb port's runtime resume callback(). Add did_runtime_put in the struct usb_port to call pm_runtime_get/put(portdev) paired. using pm_runtime_get/put(portdev) to allow or prohibit device to be power off inside of pm qos request in the kernel side. Change since v2: Correct some link breaks. Add did_runtime_put in the usb_disconnect() before calling pm_runtime_put(portdev). Provide two seperate functions usb_device_allow_power_off() and usb_device_prevent_power_off() instead of just one. Change since v3: Set did_runtime_put to false in usb_disconnect() when its value is true Add comment about not enable port runtime pm if fail to expose port's pm qos. and call pm_runtime_set_active(portdev) unconditionally. rename usb_device_allow_prevent_power_off with usb_device_control_power_off Modify be power off to be powered off Expose dev_pm_qos_flags() symbol in order to ensure usb core can compile as module. Resend v4: make patch PM/Qos: Expose dev_pm_qos_flags symbol as first patch to avoid compilation error during git bisect correct some comments. This Patchset is based on usb-next tree commit 102ee001912 Merge tag 'for-usb-next-2013-01-03' This patchset is to add usb port power off mechanism and merge with patchset usb: expose DeviceRemovable to user space via sysfs attribute. Patchset usb: expose DeviceRemovable to user space via sysfs attribute. http://marc.info/?l=linux-usbm=135279430410171w=2 with some link break corrects The main discussion about usb port power off mechanism is in the following link: http://marc.info/?l=linux-usbm=134818340017208w=2 PM/Qos: Expose dev_pm_qos_flags symbol usb: Add driver/usb/core/(port.c,hub.h) files USB: Set usb port's DeviceRemovable according acpi information usb: Add portX/connect_type attribute to expose usb port's connect type usb: Create link files between child device and usb port device. usb: Register usb port's acpi power resources usb: add runtime pm support for usb port device usb: add usb port auto power off mechanism usb: expose usb port's pm qos flags to user space usb: add usb_device_allow_power_off() and usb_device_prevent_power_off() function. Hi GregAlan: Do you have some more comments about this patchset? Thanks. -- Best regards Tianyu Lan -- 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
Re: [PATCH 1/2] usbnet: allow status interrupt URB to always be active
On Tue, Jan 15, 2013 at 1:23 AM, Dan Williams d...@redhat.com wrote: On Fri, 2013-01-11 at 11:06 +0800, Ming Lei wrote: I am curious who are interested in the 'custom Restart indication' information after the interface is closed. It's actually before the interface is even opened. It's really just a sync signal that's part of the driver's setup/initialization of the device. OK, got it. Considered that it is only required by Sierra, could you submit the status URB in driver-bind and cancel it in driver-reset so we can avoid touching usbnet core code(one simple change might be to move init_status() before calling driver-info() inside usbnet_probe())? If sierra_net provides ways(such as read registers) to query the indication event, you can just query the information and setup the device in driver_info-reset() during device open, so you can avoid submitting interrupt URB always. As far as I know, it does not, or at least Sierra hasn't released such information about the firmware API of their devices. You are unlucky, :-) In fact, many usbnet devices provide read command to query this kind of information. Thanks, -- Ming Lei -- 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
Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
On Mon, Jan 14, 2013 at 07:57:24PM +0200, Felipe Balbi wrote: On Mon, Jan 14, 2013 at 06:54:22PM +0100, Marc Kleine-Budde wrote: On 01/14/2013 06:40 PM, Felipe Balbi wrote: Hi, On Mon, Jan 14, 2013 at 08:56:33PM +0800, Peter Chen wrote: snip Usually there isn't any Changelog between IP cores used in the different fsl processors (at least available outside of fsl), that makes it quite difficult to say if something found on one imx is really the same as on the other one. And they (usually) don't provide any versioning information in a register or the documentation. just my 2¢ $SUBJECT is trying to differentiate a single feature (or maybe two) to replace cpu_is_xxx(), then expose that on driver_data without creating one enum value for each release from fsl. Felipe, every one or two SoCs may have their special operations for integrate PHY interface, clk operation, or workaround for IC limitation. the particular PHY and clk used should be hidden by phy layer and clk API respectively. Workarounds, fair enough, we need to handle them; but ideally those should be based on runtime revision detection, not some hackery using driver_data. If this is actually possible, I'd love to do this. But IP vendor don't include a version register in their cores. :( then fair enough, driver_data or platform_data is the way to go, still my point (a) below is valid. I will send v5 patch with your suggestion. -- balbi -- Best Regards, Peter Chen -- 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
Re: USB device cannot be reconnected and khubd blocked for more than 120 seconds
On Tue, Jan 15, 2013 at 1:30 AM, Linus Torvalds torva...@linux-foundation.org wrote: On Sun, Jan 13, 2013 at 11:15 PM, Ming Lei ming@canonical.com wrote: The deadlock problem is caused by calling request_module() inside async function of do_scan_async(), and it was introduced by Linus's below commit: commit d6de2c80e9d758d2e36c21699117db6178c0f517 Author: Linus Torvalds torva...@linux-foundation.org Date: Fri Apr 10 12:17:41 2009 -0700 async: Fix module loading async-work regression IMO, maybe the commit isn't a proper fix, considered the below fact: - it isn't good to allow async function to be marked as __init Immaterial. For modules, __init is a non-issue. For non-modules, the synchronization elsewhere is sufficient. Looks 5d38258ec026921a7b266f4047ebeaa75db358e5(ACPI battery: fix async boot oops) addresses the issue of __init for modules. - any user mode shouldn't expect that the device is ready just after completing of 'insmod' Bullshit. That expectation is just a fact. People insmod a device driver, and mount the device immediately in scripts. I mean we can let the device node populated in probe() first, but let open() wait for completion of the async probe(). Maybe my expression is not accurate, here the 'device isn't ready' just means that the async probe() isn't completed, and doesn't mean the device node doesn't come. We do not say user mode shouldn't. Seriously. EVER. User mode *does*, and we deal with it. Learn it now, and stop ever saying that again. This is really starting to annoy me. Kernel developers who say user mode should be fixes to not do that should go somewhere else. The whole and *only* point of a kernel is to hide these kinds of issues from user mode, and make things just work in user mode. User mode should not ever worry about oh, doing X can trigger a module load, so now the device might not be available immediately, so I should delay and re-try until it is. That's just f*cking crazy talk. We have a very simple rule in the kernel: we don't break user space. EVER. No, I don't mean we should break user space, see above. Learn that rule. I don't ever want to hear any user mode shouldn't expect again. User mode *does* expect. End of discussion. - from view of driver, introducing async_synchronize_full() after do_one_initcall() inside do_init_module() is like a sync probe for drivers built as module, and cause this kind of deadlock easily. So could we revert the commit and fix the previous problems just case by case? or other better fix? There's no way in hell we take a fix things one by one approach. It's not going to work. And your suggestion seems to not do async discovery of devices in general, which is a *much* worse fix than anything else. It's just crazy. I will try to figure out one patch to address the scsi block async probe issue first, and see if it can fix the problem by moving add_disk() into sd_probe() and calling async_synchronize_full_domain(scsi_sd_probe_domain) in the entry of sd_open(). But there are other approaches we might take. We might move the call to async_synchronize_full(); to other places. For example, maybe we're better off doing it at block/char device open instead? Looks it is similar with the above idea, but we have to remove the async_synchronize_full() in do_init_module() together. Thanks, -- Ming Lei -- 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
[PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c
Changes for v5: - Using strcmp to get specific SoC - Delete one cpu_is_mx35() as it has already pdata runtime check Changes for v4: - Using pdev's struct resource to do ioremap - Add ioremap return value check Changes for v3: - Split the one big patch into three patches Changes for v2: - Add const for fsl_udc_devtype - Do ioremap for phy address at fsl-mxc-udc Peter Chen (3): usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap ARM: i.MX clock: Change the connection-id for fsl-usb2-udc arch/arm/mach-imx/clk-imx25.c |6 +- arch/arm/mach-imx/clk-imx27.c |6 +- arch/arm/mach-imx/clk-imx31.c |6 +- arch/arm/mach-imx/clk-imx35.c |6 +- arch/arm/mach-imx/clk-imx51-imx53.c |6 +- arch/arm/mach-imx/devices/devices-common.h|1 + arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 --- drivers/usb/gadget/fsl_mxc_udc.c | 40 -- drivers/usb/gadget/fsl_udc_core.c | 46 + drivers/usb/gadget/fsl_usb2_udc.h |5 +- 10 files changed, 82 insertions(+), 55 deletions(-) -- 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
[PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
already used pdata to differentiate runtime
Meanwhile we update the platform code accordingly.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
arch/arm/mach-imx/devices/devices-common.h|1 +
arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 ---
drivers/usb/gadget/fsl_mxc_udc.c | 24 +---
drivers/usb/gadget/fsl_udc_core.c | 42 +
4 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/arch/arm/mach-imx/devices/devices-common.h
b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
#include linux/fsl_devices.h
struct imx_fsl_usb2_udc_data {
+ const char *devid;
resource_size_t iobase;
resource_size_t irq;
};
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..fb527c7 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
#include ../hardware.h
#include devices-common.h
-#define imx_fsl_usb2_udc_data_entry_single(soc)
\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)
\
{ \
+ .devid = _devid,\
.iobase = soc ## _USB_OTG_BASE_ADDR,\
.irq = soc ## _INT_USB_OTG, \
}
#ifdef CONFIG_SOC_IMX25
const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX25);
+ imx_fsl_usb2_udc_data_entry_single(MX25, imx-udc-mx25);
#endif /* ifdef CONFIG_SOC_IMX25 */
#ifdef CONFIG_SOC_IMX27
const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX27);
+ imx_fsl_usb2_udc_data_entry_single(MX27, imx-udc-mx27);
#endif /* ifdef CONFIG_SOC_IMX27 */
#ifdef CONFIG_SOC_IMX31
const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX31);
+ imx_fsl_usb2_udc_data_entry_single(MX31, imx-udc-mx31);
#endif /* ifdef CONFIG_SOC_IMX31 */
#ifdef CONFIG_SOC_IMX35
const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX35);
+ imx_fsl_usb2_udc_data_entry_single(MX35, imx-udc-mx35);
#endif /* ifdef CONFIG_SOC_IMX35 */
#ifdef CONFIG_SOC_IMX51
const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
- imx_fsl_usb2_udc_data_entry_single(MX51);
+ imx_fsl_usb2_udc_data_entry_single(MX51, imx-udc-mx51);
#endif
struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
.flags = IORESOURCE_IRQ,
},
};
- return imx_add_platform_device_dmamask(fsl-usb2-udc, -1,
+ return imx_add_platform_device_dmamask(data-devid, -1,
res, ARRAY_SIZE(res),
pdata, sizeof(*pdata), DMA_BIT_MASK(32));
}
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..1176bd8 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
#include linux/platform_device.h
#include linux/io.h
-#include mach/hardware.h
-
static struct clk *mxc_ahb_clk;
static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
clk_prepare_enable(mxc_per_clk);
/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
- if (!cpu_is_mx51()) {
+ if (strcmp(pdev-id_entry-name, imx-udc-mx51)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata-phy_mode != FSL_USB2_PHY_ULPI
(freq 5000 || freq 60001000)) {
@@ -82,17 +80,15 @@ eclkrate:
void fsl_udc_clk_finalize(struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
- if (cpu_is_mx35()) {
- unsigned int v;
-
- /* workaround ENGcm09152 for i.MX35 */
- if (pdata-workaround FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
- writel(v | USBPHYCTRL_EVDO,
- MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-
[PATCH v5 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/gadget/fsl_mxc_udc.c | 30 +++---
drivers/usb/gadget/fsl_udc_core.c |4 +++-
drivers/usb/gadget/fsl_usb2_udc.h |5 +++--
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1176bd8..bb65c46 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
static struct clk *mxc_ipg_clk;
/* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET 0x608
+#define MX35_USBPHYCTRL_OFFSET 0x600
+#define USBPHYCTRL_OTGBASE_OFFSET 0x8
#define USBPHYCTRL_EVDO(1 23)
int fsl_udc_clk_init(struct platform_device *pdev)
@@ -77,25 +78,40 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+int fsl_udc_clk_finalize(struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev-dev.platform_data;
- unsigned int v;
+ int ret = 0;
/* workaround ENGcm09152 for i.MX35 */
if (pdata-workaround FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ unsigned int v;
+ struct resource *res = platform_get_resource
+ (pdev, IORESOURCE_MEM, 0);
+ void __iomem *phy_regs = ioremap(res-start +
+ MX35_USBPHYCTRL_OFFSET, 512);
+ if (!phy_regs) {
+ dev_err(pdev-dev, ioremap for phy address fails\n);
+ ret = -EINVAL;
+ goto ioremap_err;
+ }
+
+ v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
writel(v | USBPHYCTRL_EVDO,
- MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
+
+ iounmap(phy_regs);
}
+
+ioremap_err:
/* ULPI transceivers don't need usbpll */
if (pdata-phy_mode == FSL_USB2_PHY_ULPI) {
clk_disable_unprepare(mxc_per_clk);
mxc_per_clk = NULL;
}
+
+ return ret;
}
void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c
b/drivers/usb/gadget/fsl_udc_core.c
index c971e84..347b1ed 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2543,7 +2543,9 @@ static int __init fsl_udc_probe(struct platform_device
*pdev)
dr_controller_setup(udc_controller);
}
- fsl_udc_clk_finalize(pdev);
+ ret = fsl_udc_clk_finalize(pdev);
+ if (ret)
+ goto err_free_irq;
/* Setup gadget structure */
udc_controller-gadget.ops = fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h
b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..c6703bb 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -592,15 +592,16 @@ static inline struct ep_queue_head *get_qh_by_ep(struct
fsl_ep *ep)
struct platform_device;
#ifdef CONFIG_ARCH_MXC
int fsl_udc_clk_init(struct platform_device *pdev);
-void fsl_udc_clk_finalize(struct platform_device *pdev);
+int fsl_udc_clk_finalize(struct platform_device *pdev);
void fsl_udc_clk_release(void);
#else
static inline int fsl_udc_clk_init(struct platform_device *pdev)
{
return 0;
}
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline int fsl_udc_clk_finalize(struct platform_device *pdev)
{
+ return 0;
}
static inline void fsl_udc_clk_release(void)
{
--
1.7.0.4
--
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
[PATCH v5 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
As we use platform_device_id for fsl-usb2-udc driver, it needs to change clk connection-id, or the related devm_clk_get will be failed. Signed-off-by: Peter Chen peter.c...@freescale.com --- arch/arm/mach-imx/clk-imx25.c |6 +++--- arch/arm/mach-imx/clk-imx27.c |6 +++--- arch/arm/mach-imx/clk-imx31.c |6 +++--- arch/arm/mach-imx/clk-imx35.c |6 +++--- arch/arm/mach-imx/clk-imx51-imx53.c |6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c index b197aa7..67e353d 100644 --- a/arch/arm/mach-imx/clk-imx25.c +++ b/arch/arm/mach-imx/clk-imx25.c @@ -254,9 +254,9 @@ int __init mx25_clocks_init(void) clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); clk_register_clkdev(clk[usbotg_ahb], ahb, mxc-ehci.2); clk_register_clkdev(clk[usb_div], per, mxc-ehci.2); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usbotg_ahb], ahb, fsl-usb2-udc); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx25); + clk_register_clkdev(clk[usbotg_ahb], ahb, imx-udc-mx25); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx25); clk_register_clkdev(clk[nfc_ipg_per], NULL, imx25-nand.0); /* i.mx25 has the i.mx35 type cspi */ clk_register_clkdev(clk[cspi1_ipg], NULL, imx35-cspi.0); diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c index 4c1d1e4..1ffe3b5 100644 --- a/arch/arm/mach-imx/clk-imx27.c +++ b/arch/arm/mach-imx/clk-imx27.c @@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref) clk_register_clkdev(clk[lcdc_ahb_gate], ahb, imx21-fb.0); clk_register_clkdev(clk[csi_ahb_gate], ahb, imx27-camera.0); clk_register_clkdev(clk[per4_gate], per, imx27-camera.0); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); - clk_register_clkdev(clk[usb_ipg_gate], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usb_ahb_gate], ahb, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx27); + clk_register_clkdev(clk[usb_ipg_gate], ipg, imx-udc-mx27); + clk_register_clkdev(clk[usb_ahb_gate], ahb, imx-udc-mx27); clk_register_clkdev(clk[usb_div], per, mxc-ehci.0); clk_register_clkdev(clk[usb_ipg_gate], ipg, mxc-ehci.0); clk_register_clkdev(clk[usb_ahb_gate], ahb, mxc-ehci.0); diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c index 8be64e0..ef66eaf 100644 --- a/arch/arm/mach-imx/clk-imx31.c +++ b/arch/arm/mach-imx/clk-imx31.c @@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref) clk_register_clkdev(clk[usb_div_post], per, mxc-ehci.2); clk_register_clkdev(clk[usb_gate], ahb, mxc-ehci.2); clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); - clk_register_clkdev(clk[usb_div_post], per, fsl-usb2-udc); - clk_register_clkdev(clk[usb_gate], ahb, fsl-usb2-udc); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div_post], per, imx-udc-mx31); + clk_register_clkdev(clk[usb_gate], ahb, imx-udc-mx31); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx31); clk_register_clkdev(clk[csi_gate], NULL, mx3-camera.0); /* i.mx31 has the i.mx21 type uart */ clk_register_clkdev(clk[uart1_gate], per, imx21-uart.0); diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c index 66f3d65..69fe9c8 100644 --- a/arch/arm/mach-imx/clk-imx35.c +++ b/arch/arm/mach-imx/clk-imx35.c @@ -251,9 +251,9 @@ int __init mx35_clocks_init() clk_register_clkdev(clk[usb_div], per, mxc-ehci.2); clk_register_clkdev(clk[ipg], ipg, mxc-ehci.2); clk_register_clkdev(clk[usbotg_gate], ahb, mxc-ehci.2); - clk_register_clkdev(clk[usb_div], per, fsl-usb2-udc); - clk_register_clkdev(clk[ipg], ipg, fsl-usb2-udc); - clk_register_clkdev(clk[usbotg_gate], ahb, fsl-usb2-udc); + clk_register_clkdev(clk[usb_div], per, imx-udc-mx35); + clk_register_clkdev(clk[ipg], ipg, imx-udc-mx35); + clk_register_clkdev(clk[usbotg_gate], ahb, imx-udc-mx35); clk_register_clkdev(clk[wdog_gate], NULL, imx2-wdt.0); clk_register_clkdev(clk[nfc_div], NULL, imx25-nand.0); clk_register_clkdev(clk[csi_gate], NULL, mx3-camera.0); diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c index 579023f..fb7cb84 100644 --- a/arch/arm/mach-imx/clk-imx51-imx53.c +++ b/arch/arm/mach-imx/clk-imx51-imx53.c @@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil, clk_register_clkdev(clk[usboh3_per_gate], per, mxc-ehci.2); clk_register_clkdev(clk[usboh3_gate], ipg, mxc-ehci.2); clk_register_clkdev(clk[usboh3_gate], ahb, mxc-ehci.2); - clk_register_clkdev(clk[usboh3_per_gate], per, fsl-usb2-udc); -
[PATCH] HID: add support for Sony RF receiver with USB product id 0x0374
Some Vaio desktop computers, among them the VGC-LN51JGB multimedia PC, have
a RF receiver, multi-interface USB device 054c:0374, that is used to connect
a wireless keyboard and a wireless mouse.
The keyboard works flawlessly, but the mouse (VGP-WMS3 in my case) does not
seem to be generating any pointer events. The problem is that the mouse pointer
is wrongly declared as a constant non-data variable in the report descriptor
(see lsusb and usbhid-dump output below), with the consenquence that it is
ignored by the HID code.
Add this device to the have-special-driver list and fix up the report
descriptor in the Sony-specific driver which happens to already have a fixup
for a similar firmware bug.
# lsusb -vd 054C:0374
Bus 003 Device 002: ID 054c:0374 Sony Corp.
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x054c Sony Corp.
idProduct 0x0374
iSerial 0
[...]
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 2 Mouse
iInterface 2 RF Receiver
[...]
Report Descriptor: (length is 100)
Item(Global): Usage Page, data= [ 0x01 ] 1
Generic Desktop Controls
Item(Local ): Usage, data= [ 0x30 ] 48
Direction-X
Item(Local ): Usage, data= [ 0x31 ] 49
Direction-Y
Item(Global): Report Count, data= [ 0x02 ] 2
Item(Global): Report Size, data= [ 0x08 ] 8
Item(Global): Logical Minimum, data= [ 0x81 ] 129
Item(Global): Logical Maximum, data= [ 0x7f ] 127
Item(Main ): Input, data= [ 0x07 ] 7
Constant Variable Relative No_Wrap Linear
Preferred_State No_Null_Position Non_Volatile
Bitfield
# sudo usbhid-dump
003:002:001:DESCRIPTOR 1357910009.758544
05 01 09 02 A1 01 05 01 09 02 A1 02 85 01 09 01
A1 00 05 09 19 01 29 05 95 05 75 01 15 00 25 01
81 02 75 03 95 01 81 01 05 01 09 30 09 31 95 02
75 08 15 81 25 7F 81 07 A1 02 85 01 09 38 35 00
45 00 15 81 25 7F 95 01 75 08 81 06 C0 A1 02 85
01 05 0C 15 81 25 7F 95 01 75 08 0A 38 02 81 06
C0 C0 C0 C0
Cc: linux-in...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Fernando Luis Vazquez Cao ferna...@oss.ntt.co.jp
---
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-core.c
linux-3.8-rc3/drivers/hid/hid-core.c
--- linux-3.8-rc3-orig/drivers/hid/hid-core.c 2013-01-13 20:54:36.846952518
+0900
+++ linux-3.8-rc3/drivers/hid/hid-core.c2013-01-13 18:21:19.901347327
+0900
@@ -1697,6 +1697,7 @@ static const struct hid_device_id hid_ha
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE)
},
+ { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP)
},
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-ids.h
linux-3.8-rc3/drivers/hid/hid-ids.h
--- linux-3.8-rc3-orig/drivers/hid/hid-ids.h2013-01-13 20:54:36.850952537
+0900
+++ linux-3.8-rc3/drivers/hid/hid-ids.h 2013-01-13 18:21:19.901347327 +0900
@@ -706,6 +706,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
#define USB_DEVICE_ID_SONY_PS3_BDREMOTE0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff -urNp linux-3.8-rc3-orig/drivers/hid/hid-sony.c
linux-3.8-rc3/drivers/hid/hid-sony.c
--- linux-3.8-rc3-orig/drivers/hid/hid-sony.c 2012-12-11 12:30:57.0
+0900
+++ linux-3.8-rc3/drivers/hid/hid-sony.c2013-01-13 18:21:19.901347327
+0900
@@ -44,19 +44,21 @@ static __u8 *sony_report_fixup(struct hi
struct sony_sc *sc = hid_get_drvdata(hdev);
if ((sc-quirks VAIO_RDESC_CONSTANT)
- *rsize = 56 rdesc[54] == 0x81 rdesc[55] == 0x07)
{
- hid_info(hdev, Fixing up Sony Vaio VGX report descriptor\n);
+ *rsize = 56 rdesc[54] == 0x81 rdesc[55] == 0x07) {
+ hid_info(hdev,
+
Re: [PATCH] usb: phy: samsung: Add support to set pmu isolation
Hi Doug,
On Tue, Jan 15, 2013 at 3:41 AM, Doug Anderson diand...@chromium.org wrote:
Vivek,
Sorry for being so absent from these reviews. I'll try to look over a
few patches today, but please don't hold up anything on account of my
reviews. I'm definitely a bit of an interested bystander in USB land.
;)
In general things look pretty good here. :) One last comment below...
On Fri, Jan 11, 2013 at 12:08 AM, Vivek Gautam
gautam.vi...@samsung.com wrote: +static int
samsung_usbphy_parse_dt(struct samsung_usbphy *sphy)
+{
+ struct device_node *usbphy_sys;
+
+ /* Getting node for system controller interface for usb-phy */
+ usbphy_sys = of_get_child_by_name(sphy-dev-of_node, usbphy-sys);
+ if (!usbphy_sys)
+ dev_warn(sphy-dev, No sys-controller interface for
usb-phy\n);
Seems like you ought to return with an error here. Calling of_iomap()
with a NULL value seems undesirable.
Yeah, true. This should have been returning error value alongwith dev_err().
+
+ sphy-pmuregs = of_iomap(usbphy_sys, 0);
+
+ of_node_put(usbphy_sys);
+
+ if (sphy-pmuregs == NULL) {
+ dev_err(sphy-dev, Can't get usb-phy pmu control
register\n);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+/*
+ * Set isolation here for phy.
+ * Here 'on = true' would mean USB PHY block is isolated, hence
+ * de-activated and vice-versa.
+ */
Thank you very much for this comment. :) This explains one of the
confusions I had earlier...
Your welcome :-)
Once you fix the one error condition above you can add my
Reviewed-by. Thanks!
Sure, thanks !!
--
Thanks Regards
Vivek
--
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
[PATCH] usbnet: dm9601: Fix incorrect command
commit 24b1042c4eb2 (usbnet: dm9601: apply introduced usb command
APIs) removes the distiction between DM_WRITE_REG and DM_WRITE_REGS
command. The distiction is reintroduced to the driver so that the
functionality of the driver remains same.
CC: Ming Lei ming@canonical.com
Signed-off-by: Tushar Behera tushar.beh...@linaro.org
---
drivers/net/usb/dm9601.c | 22 ++
1 files changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index 3f554c1..011410f 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -53,7 +53,6 @@
#define DM_RX_OVERHEAD 7 /* 3 byte header + 4 byte crc tail */
#define DM_TIMEOUT 1000
-
static int dm_read(struct usbnet *dev, u8 reg, u16 length, void *data)
{
int err;
@@ -84,32 +83,23 @@ static int dm_write(struct usbnet *dev, u8 reg, u16 length,
void *data)
static int dm_write_reg(struct usbnet *dev, u8 reg, u8 value)
{
- return usbnet_write_cmd(dev, DM_WRITE_REGS,
+ return usbnet_write_cmd(dev, DM_WRITE_REG,
USB_DIR_OUT | USB_TYPE_VENDOR |
USB_RECIP_DEVICE,
value, reg, NULL, 0);
}
-static void dm_write_async_helper(struct usbnet *dev, u8 reg, u8 value,
- u16 length, void *data)
+static void dm_write_async(struct usbnet *dev, u8 reg, u16 length, void *data)
{
usbnet_write_cmd_async(dev, DM_WRITE_REGS,
USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- value, reg, data, length);
-}
-
-static void dm_write_async(struct usbnet *dev, u8 reg, u16 length, void *data)
-{
- netdev_dbg(dev-net, dm_write_async() reg=0x%02x length=%d\n, reg,
length);
-
- dm_write_async_helper(dev, reg, 0, length, data);
+ 0, reg, data, length);
}
static void dm_write_reg_async(struct usbnet *dev, u8 reg, u8 value)
{
- netdev_dbg(dev-net, dm_write_reg_async() reg=0x%02x value=0x%02x\n,
- reg, value);
-
- dm_write_async_helper(dev, reg, value, 0, NULL);
+ usbnet_write_cmd_async(dev, DM_WRITE_REG,
+ USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ value, reg, NULL, 0);
}
static int dm_read_shared_word(struct usbnet *dev, int phy, u8 reg, __le16
*value)
--
1.7.4.1
--
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
[PATCH v7] usb: phy: samsung: Add support to set pmu isolation
Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
Reviewed-by: Doug Anderson diand...@chromium.org
---
Changes from v6:
- Returning error code in samsung_usbphy_parse_dt() when
'usbphy-sys' sub-node is not present and thereby putting up
dev_err() instead of dev_warn()
.../devicetree/bindings/usb/samsung-usbphy.txt | 36 +
drivers/usb/phy/samsung-usbphy.c | 163 +---
2 files changed, 177 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 7b26e2d..22d06cf 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,39 @@ Required properties:
- compatible : should be samsung,exynos4210-usbphy
- reg : base physical address of the phy registers and length of memory mapped
region.
+
+Optional properties:
+- #address-cells: should be '1' when usbphy node has a child node with 'reg'
+ property.
+- #size-cells: should be '1' when usbphy node has a child node with 'reg'
+ property.
+- ranges: allows valid translation between child's address space and parent's
+ address space.
+
+- The child node 'usbphy-sys' to the node 'usbphy' is for the system controller
+ interface for usb-phy. It should provide the following information required
by
+ usb-phy controller to control phy.
+ - reg : base physical address of PHY_CONTROL registers.
+ The size of this register is the total sum of size of all PHY_CONTROL
+ registers that the SoC has. For example, the size will be
+ '0x4' in case we have only one PHY_CONTROL register (e.g.
+ OTHERS register in S3C64XX or USB_PHY_CONTROL register in S5PV210)
+ and, '0x8' in case we have two PHY_CONTROL registers (e.g.
+ USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers in exynos4x).
+ and so on.
+
+Example:
+ - Exynos4210
+
+ usbphy@125B {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = samsung,exynos4210-usbphy;
+ reg = 0x125B 0x100;
+ ranges;
+
+ usbphy-sys {
+ /* USB device and host PHY_CONTROL registers */
+ reg = 0x10020704 0x8;
+ };
+ };
diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index 5c5e1bb5..30aebb5 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -24,6 +24,7 @@
#include linux/err.h
#include linux/io.h
#include linux/of.h
+#include linux/of_address.h
#include linux/usb/otg.h
#include linux/platform_data/samsung-usbphy.h
@@ -60,20 +61,46 @@
#define MHZ (1000*1000)
#endif
+#define S3C64XX_USBPHY_ENABLE (0x1 16)
+#define EXYNOS_USBPHY_ENABLE (0x1 0)
+
enum samsung_cpu_type {
TYPE_S3C64XX,
TYPE_EXYNOS4210,
};
/*
+ * struct samsung_usbphy_drvdata - driver data for various SoC variants
+ * @cpu_type: machine identifier
+ * @devphy_en_mask: device phy enable mask for PHY CONTROL register
+ * @devphy_reg_offset: offset to DEVICE PHY CONTROL register from
+ *mapped address of system controller.
+ *
+ * Here we have a separate mask for device type phy.
+ * Having different masks for host and device type phy helps
+ * in setting independent masks in case of SoCs like S5PV210,
+ * in which PHY0 and PHY1 enable bits belong to same register
+ * placed at position 0 and 1 respectively.
+ * Although for newer SoCs like exynos these bits belong to
+ * different registers altogether placed at position 0.
+ */
+struct samsung_usbphy_drvdata {
+ int cpu_type;
+ int devphy_en_mask;
+ u32 devphy_reg_offset;
+};
+
+/*
* struct samsung_usbphy - transceiver driver state
* @phy: transceiver structure
* @plat: platform data
* @dev: The parent device supplied to the probe function
* @clk: usb phy clock
- * @regs: usb phy register memory base
+ * @regs: usb phy controller registers memory base
+ * @pmuregs: USB device PHY_CONTROL register memory base
* @ref_clk_freq: reference clock frequency selection
- * @cpu_type: machine identifier
+ * @drv_data: driver data available for different SoCs
+ * @lock: lock for phy operations
*/
struct samsung_usbphy {
struct usb_phy phy;
@@ -81,12 +108,66 @@ struct samsung_usbphy {
struct device *dev;
struct clk *clk;
void __iomem*regs;
+ void __iomem*pmuregs;
int ref_clk_freq;
- int cpu_type;
+ const struct samsung_usbphy_drvdata *drv_data;
+ spinlock_t
[PATCH v2 4/4] usb: chipidea: imx: Add system suspend/resume API
During the system suspend/resume procedure, the USB also
needs to go suspend/resume procedure, this patch adds
related APIs. It is tested at i.mx6q sabrelite. Meanwhile,
it fixes the bug that the USB will out of work after
system suspend/resume.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/bits.h|1 +
drivers/usb/chipidea/ci13xxx_imx.c | 61
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index ba9c6ef..d1467bb 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -47,6 +47,7 @@
#define PORTSC_FPRBIT(6)
#define PORTSC_SUSP BIT(7)
#define PORTSC_HSPBIT(9)
+#define PORTSC_PHCD BIT(23) /* phy suspend mode */
#define PORTSC_PTC(0x0FUL 16)
/* DEVLC */
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 342eab0..dd257b1 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -25,6 +25,7 @@
#include linux/mfd/syscon.h
#include ci.h
+#include bits.h
#include ci13xxx_imx.h
#define pdev_to_phy(pdev) \
@@ -321,6 +322,63 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int ci13xxx_imx_suspend(struct device *dev)
+{
+ struct ci13xxx_imx_data *data =
+ platform_get_drvdata(to_platform_device(dev));
+ struct platform_device *plat_ci;
+ struct ci13xxx *ci;
+
+ plat_ci = data-ci_pdev;
+ ci = platform_get_drvdata(plat_ci);
+
+ hw_write(ci, OP_PORTSC, PORTSC_PHCD, 1);
+
+ if (data-phy)
+ usb_phy_set_suspend(data-phy, 1);
+
+ clk_disable_unprepare(data-clk);
+
+ return 0;
+}
+
+static int ci13xxx_imx_resume(struct device *dev)
+{
+ int ret;
+ struct ci13xxx_imx_data *data =
+ platform_get_drvdata(to_platform_device(dev));
+ struct platform_device *plat_ci;
+ struct ci13xxx *ci;
+
+ plat_ci = data-ci_pdev;
+ ci = platform_get_drvdata(plat_ci);
+
+ ret = clk_prepare_enable(data-clk);
+ if (ret) {
+ dev_err(dev,
+ Failed to prepare or enable clock, err=%d\n, ret);
+ return ret;
+ }
+
+ if (hw_read(ci, OP_PORTSC, PORTSC_PHCD)) {
+ hw_write(ci, OP_PORTSC, PORTSC_PHCD, 0);
+ /* Some clks sync between Controller and USB PHY */
+ mdelay(1);
+ }
+
+ if (data-phy)
+ usb_phy_set_suspend(data-phy, 0);
+
+ return ret;
+}
+
+static const struct dev_pm_ops ci13xxx_imx_pm_ops = {
+ .suspend= ci13xxx_imx_suspend,
+ .resume = ci13xxx_imx_resume,
+};
+#endif
+
static const struct of_device_id ci13xxx_imx_dt_ids[] = {
{ .compatible = fsl,imx27-usb, },
{ /* sentinel */ }
@@ -334,6 +392,9 @@ static struct platform_driver ci13xxx_imx_driver = {
.name = imx_usb,
.owner = THIS_MODULE,
.of_match_table = ci13xxx_imx_dt_ids,
+#ifdef CONFIG_PM
+ .pm = ci13xxx_imx_pm_ops,
+#endif
},
};
--
1.7.0.4
--
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
