Re: [PATCH 3/3] USB mxs-phy: Register phy with framework
hi,
On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
From: Sascha Hauer s.ha...@pengutronix.de
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
any chance you can move away from {write,read}[bwl]_relaxed() so we can
build this driver on other architectures ?
--
balbi
signature.asc
Description: Digital signature
[PATCH v2 corrected] usb/gadget: f_rndis: convert to new function interface with backward compatibility
Converting rndis to the new function interface requires converting
the USB rndis' function code and its users.
This patch converts the f_rndis.c to the new function interface.
The file is now compiled into a separate f_rndis_usb.ko module.
The old function interface is provided by means of a preprocessor
conditional directives. After all users are converted, the old interface
can be removed.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/Kconfig |3 +
drivers/usb/gadget/Makefile |2 +
drivers/usb/gadget/ether.c |1 +
drivers/usb/gadget/f_rndis.c | 158 ++
drivers/usb/gadget/g_ffs.c |1 +
drivers/usb/gadget/multi.c |1 +
drivers/usb/gadget/u_rndis.h | 29
7 files changed, 167 insertions(+), 28 deletions(-)
create mode 100644 drivers/usb/gadget/u_rndis.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index d02547e..4dc6d66 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -531,6 +531,9 @@ config USB_F_ECM
config USB_F_SUBSET
tristate
+config USB_F_RNDIS
+ tristate
+
choice
tristate USB Gadget Drivers
default USB_ETH
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index ddd1706..6f433fa 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -94,3 +94,5 @@ usb_f_ecm-y := f_ecm.o
obj-$(CONFIG_USB_F_ECM)+= usb_f_ecm.o
usb_f_ecm_subset-y := f_subset.o
obj-$(CONFIG_USB_F_ECM)+= usb_f_ecm_subset.o
+usb_f_rndis-y := f_rndis.o
+obj-$(CONFIG_USB_F_RNDIS) += usb_f_rndis.o
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 754b300..ebe8323 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -105,6 +105,7 @@ static inline bool has_rndis(void)
* a gcc --combine ... part1.c part2.c part3.c ... build would.
*/
#ifdef USB_ETH_RNDIS
+#define USB_FRNDIS_INCLUDED
#include f_rndis.c
#include rndis.h
#endif
diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
index 970c905..08bcbf4 100644
--- a/drivers/usb/gadget/f_rndis.c
+++ b/drivers/usb/gadget/f_rndis.c
@@ -17,15 +17,16 @@
#include linux/slab.h
#include linux/kernel.h
+#include linux/module.h
#include linux/device.h
#include linux/etherdevice.h
#include linux/atomic.h
#include u_ether.h
+#include u_rndis.h
#include rndis.h
-
/*
* This function is an RNDIS Ethernet port -- a Microsoft protocol that's
* been promoted instead of the standard CDC Ethernet. The published RNDIS
@@ -656,6 +657,13 @@ static void rndis_close(struct gether *geth)
/*-*/
+/* Some controllers can't support RNDIS ... */
+static inline bool can_support_rndis(struct usb_configuration *c)
+{
+ /* everything else is *presumably* fine */
+ return true;
+}
+
/* ethernet function driver setup/binding */
static int
@@ -666,6 +674,32 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
int status;
struct usb_ep *ep;
+#ifndef USB_FRNDIS_INCLUDED
+ struct f_rndis_opts *rndis_opts;
+
+ if (!can_support_rndis(c))
+ return -EINVAL;
+
+ rndis_opts = container_of(f-fi, struct f_rndis_opts, func_inst);
+ if (!rndis_opts-ethaddr)
+ return -EINVAL;
+#endif
+
+ if (rndis_string_defs[0].id == 0) {
+ /* ... and setup RNDIS itself */
+ status = rndis_init();
+ if (status 0)
+ return status;
+
+ status = usb_string_ids_tab(c-cdev, rndis_string_defs);
+ if (status)
+ return status;
+
+ rndis_control_intf.iInterface = rndis_string_defs[0].id;
+ rndis_data_intf.iInterface = rndis_string_defs[1].id;
+ rndis_iad_descriptor.iFunction = rndis_string_defs[2].id;
+ }
+
/* allocate instance-specific interface IDs */
status = usb_interface_id(c, f);
if (status 0)
@@ -788,8 +822,10 @@ fail:
return status;
}
+#ifdef USB_FRNDIS_INCLUDED
+
static void
-rndis_unbind(struct usb_configuration *c, struct usb_function *f)
+rndis_old_unbind(struct usb_configuration *c, struct usb_function *f)
{
struct f_rndis *rndis = func_to_rndis(f);
@@ -805,13 +841,6 @@ rndis_unbind(struct usb_configuration *c, struct
usb_function *f)
kfree(rndis);
}
-/* Some controllers can't support RNDIS ... */
-static inline bool can_support_rndis(struct usb_configuration *c)
-{
- /* everything else is *presumably* fine */
- return true;
-}
-
int
rndis_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN],
u32
[PATCH] drivers/usb/gadget: beautify code, delete unused code
originally, when deleted the relative code, left some 'another'.
need delete 'another', too.
the relative patches are:
commit 96f8db6a77e3490608e5b5b3f57e7201f8c85496
Author: Felipe Balbi ba...@ti.com
Date: Mon Oct 10 10:33:47 2011 +0300
usb: gadget: net2272: convert to new style
commit 4cf5e00b055ba5e4f3852e477a2a4346730ea283
Author: Felipe Balbi ba...@ti.com
Date: Mon Oct 10 10:37:17 2011 +0300
usb: gadget: net2280: convert to new style
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/gadget/net2272.c |4
drivers/usb/gadget/net2280.c |4
2 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index d226058..22ee57c 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -1484,10 +1484,6 @@ stop_activity(struct net2272 *dev, struct
usb_gadget_driver *driver)
{
int i;
- /* don't disconnect if it's not connected */
- if (dev-gadget.speed == USB_SPEED_UNKNOWN)
- driver = NULL;
-
/* stop hardware; prevent new request submissions;
* and kill any outstanding requests.
*/
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index a1b650e..4af3a4e 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -1935,10 +1935,6 @@ stop_activity (struct net2280 *dev, struct
usb_gadget_driver *driver)
{
int i;
- /* don't disconnect if it's not connected */
- if (dev-gadget.speed == USB_SPEED_UNKNOWN)
- driver = NULL;
-
/* stop hardware; prevent new request submissions;
* and kill any outstanding requests.
*/
--
1.7.7.6
--
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 v10 6/8] usb: chipidea: imx: add internal vbus regulator control
On Thu, Feb 28, 2013 at 09:29:46AM +0200, Felipe Balbi wrote:
On Thu, Feb 28, 2013 at 02:06:09PM +0800, Peter Chen wrote:
On Wed, Feb 27, 2013 at 09:08:11PM +0200, Alexander Shishkin wrote:
On Feb 27, 2013 8:34 AM, Peter Chen peter.c...@freescale.com wrote:
- For host, the vbus should always be on.
- For otg, the vbus is off defaultly, the vbus needs to be
turned on/off when usb role switches.
+ * @reg_vbus: used to control internal vbus regulator
*/
struct ci13xxx {
struct device *dev;
@@ -172,6 +173,7 @@ struct ci13xxx {
struct usb_otg otg;
boolid_event;
boolb_sess_valid_event;
+ struct regulator*reg_vbus;
};
I suggest that this regulator be taken care of by either the platform
code
or phy layer, it doesn't look like it really belongs in chipidea core.
The
easiest way I guess would be to do something similar to the notify
callback in msm platform code.
VBUS operation is typical operation during the USB code, eg, we need
to open vbus at Host mode, we need close/open vbus during host-gadget
switch.
I suggest vbus regulator is specified at platform code, and assign it
to ci core's pdata, like sasche's patch pdata-dr_mode and pdata-phy_mode.
you don't need platform_data for it. From ci core probe try to get the
regulator, if it doesn't exist, ignore the error and continue anyways.
The pdev of ci core is created by platform layer, at ci core probe, it
can't get platform things unless it is passed from platform layer
--
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 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
On 2013年02月28日 15:57, Li Fei wrote:
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Hi Fei:
It's not necessary. Because the did_runtime_put == true means the
port's usage count has already been decreased during
usb_port_suspend().So to keep usage count balance, we should increase
the usage count in the usb_port_resume() whatever.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..b68493b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
}
Hi Alan:
Further thinking, the device should be disconnected since the port
can't be resumed and the device will not work normally. Something like
following. Does this make sense?
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d5d3de4..cf36b11 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status 0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+ hub_port_logical_disconnect(hub, port1);
return status;
}
}
--
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 v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
Hi, On Thu, Feb 28, 2013 at 04:31:44PM +0800, Peter Chen wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. Just because dwc3/core.c doesn't know about clocks, it doesn't mean it's correct to hack it into the glue layer if that doesn't need the clock. Now that we know that's a bug, who's going to send me tested patches to teach dwc3/core.c about struct clk in a way that doesn't break PCIe, nor OMAP5 ? cheers -- balbi signature.asc Description: Digital signature
RE: [PATCH 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
-Original Message-
From: Lan, Tianyu
Sent: Thursday, February 28, 2013 4:37 PM
To: Li, Fei; st...@rowland.harvard.edu
Cc: gre...@linuxfoundation.org; sarah.a.sh...@linux.intel.com; r...@sisk.pl;
linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Liu, Chuansheng
Subject: Re: [PATCH 4/5] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On 2013年02月28日 15:57, Li Fei wrote:
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Hi Fei:
It's not necessary. Because the did_runtime_put == true means the
port's usage count has already been decreased during
usb_port_suspend().So to keep usage count balance, we should increase
the usage count in the usb_port_resume() whatever.
Thanks for your reminder.
Sorry, I forget to keep did_runtime_put as false in pm_runtime_get_sync
failed case. I'll submit patch V2 to update it.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..b68493b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3152,6 +3152,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
}
Hi Alan:
Further thinking, the device should be disconnected since the port
can't be resumed and the device will not work normally. Something like
following. Does this make sense?
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d5d3de4..cf36b11 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status 0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+ hub_port_logical_disconnect(hub, port1);
IMHO, this can't keep the usage_count balance.
Best Regards,
Li Fei
return status;
}
}
--
Best regards
Tianyu Lan
[PATCH 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
+ port_dev-did_runtime_put = false;
}
/* Skip the initial Clear-Suspend step for a remote wakeup */
--
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
Re: [PATCH 3/3] USB mxs-phy: Register phy with framework
On 02/28/2013 09:01 AM, Felipe Balbi wrote:
hi,
On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
From: Sascha Hauer s.ha...@pengutronix.de
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
any chance you can move away from {write,read}[bwl]_relaxed() so we can
build this driver on other architectures ?
The hardware is in the ARM imx2{2,8} only. Another option would be to
add an depends on ARCH_ARM.
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 2/3] USB: add devicetree helpers for determining dr_mode and phy_type
On 02/28/2013 08:57 AM, Felipe Balbi wrote: On Wed, Feb 27, 2013 at 03:16:29PM +0100, Marc Kleine-Budde wrote: From: Michael Grzeschik m.grzesc...@pengutronix.de This adds two little devicetree helper functions for determining the dr_mode (host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the devicetree. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Sascha Hauer s.ha...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de causes build breakage: include/linux/usb/of.h:21:32: error: return type is an incomplete type include/linux/usb/of.h: In function ‘of_usb_get_dr_mode’: include/linux/usb/of.h:23:9: error: ‘USB_DR_MODE_UNKNOWN’ undeclared (first use in this function) include/linux/usb/of.h:23:9: note: each undeclared identifier is reported only once for each function it appears in include/linux/usb/of.h:23:2: warning: ‘return’ with a value, in function returning void [enabled by default] make[1]: *** [drivers/usb/usb-common.o] Error 1 Doh! That occurs only for non DT kernels, but who doesn't use device tree these days? Fixed, 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 v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
Felipe Balbi ba...@ti.com writes: Hi, On Thu, Feb 28, 2013 at 04:31:44PM +0800, Peter Chen wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. I'm not too familiar with the multitudes of platforms out there, but my simple question is: why can't we have pm runtime take care of enabling/disabling the clocks so that we don't have to do it in drivers? Seems obvious that a platform/SoC/board should know about it's clock tree structure, so why doesn't the platform code then take care of all the dirty details? It seems totally unreasonable and messy to add notion of clocks to drivers just because some platforms can't get their PM right. Just because dwc3/core.c doesn't know about clocks, it doesn't mean it's correct to hack it into the glue layer if that doesn't need the clock. Now that we know that's a bug, who's going to send me tested patches to teach dwc3/core.c about struct clk in a way that doesn't break PCIe, nor OMAP5 ? So are you sure that's what you want? Regards, -- Alex -- 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 3/3] USB mxs-phy: Register phy with framework
Marc Kleine-Budde m...@pengutronix.de writes:
On 02/28/2013 09:01 AM, Felipe Balbi wrote:
hi,
On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
From: Sascha Hauer s.ha...@pengutronix.de
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
any chance you can move away from {write,read}[bwl]_relaxed() so we can
build this driver on other architectures ?
The hardware is in the ARM imx2{2,8} only. Another option would be to
add an depends on ARCH_ARM.
Doesn't mean that we shouldn't be able to compile-test the driver.
Regards,
--
Alex
--
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/3] usb: Correction to c67x00 TD data length mask
Dave == Dave Tubbs dave.tu...@portalislc.com writes:
Dave From: Dave Tubbs dave.tu...@portalislc.com
Dave TD data length is 10 bits, correct TD_PORTLENMASK_DL. Reference
Dave Cypress Semiconductor BIOS User's Manual 1.2, page 3-10
Dave Signed-off-by: Dave Tubbs dave.tu...@portalislc.com
Acked-by: Peter Korsgaard jac...@sunsite.dk
Dave ---
Dave drivers/usb/c67x00/c67x00-sched.c |2 +-
Dave 1 files changed, 1 insertions(+), 1 deletions(-)
Dave diff --git a/drivers/usb/c67x00/c67x00-sched.c
b/drivers/usb/c67x00/c67x00-sched.c
Dave index a03fbc1..aa2262f 100644
Dave --- a/drivers/usb/c67x00/c67x00-sched.c
Dave +++ b/drivers/usb/c67x00/c67x00-sched.c
Dave @@ -100,7 +100,7 @@ struct c67x00_urb_priv {
Dave #define TD_PIDEP_OFFSET 0x04
Dave #define TD_PIDEPMASK_PID0xF0
Dave #define TD_PIDEPMASK_EP 0x0F
Dave -#define TD_PORTLENMASK_DL 0x02FF
Dave +#define TD_PORTLENMASK_DL 0x03FF
Dave #define TD_PORTLENMASK_PN 0xC000
Dave #define TD_STATUS_OFFSET0x07
Dave --
Dave 1.7.1
--
Bye, Peter Korsgaard
--
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 2/3] usb: c67x00 RetryCnt value in c67x00 TD should be 3
Dave == Dave Tubbs dave.tu...@portalislc.com writes:
Dave From: Dave Tubbs dave.tu...@portalislc.com
Dave RetryCnt value in c67x00 TD should be 3 (both bits set to 1). Reference
Dave Cypress Semiconductor BIOS User's Manual 1.2, page 3-14
Acked-by: Peter Korsgaard jac...@sunsite.dk
Dave Signed-off-by: Dave Tubbs dave.tu...@portalislc.com
Dave ---
Dave drivers/usb/c67x00/c67x00-sched.c |2 +-
Dave 1 files changed, 1 insertions(+), 1 deletions(-)
Dave diff --git a/drivers/usb/c67x00/c67x00-sched.c
b/drivers/usb/c67x00/c67x00-sched.c
Dave index aa2262f..aa49162 100644
Dave --- a/drivers/usb/c67x00/c67x00-sched.c
Dave +++ b/drivers/usb/c67x00/c67x00-sched.c
Dave @@ -590,7 +590,7 @@ static int c67x00_create_td(struct c67x00_hcd
*c67x00, struct urb *urb,
Dave {
Dave struct c67x00_td *td;
Dave struct c67x00_urb_priv *urbp = urb-hcpriv;
Dave -const __u8 active_flag = 1, retry_cnt = 1;
Dave +const __u8 active_flag = 1, retry_cnt = 3;
Dave __u8 cmd = 0;
Dave int tt = 0;
Dave --
Dave 1.7.1
--
Bye, Peter Korsgaard
--
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 3/3] usb: Make sure each c67x00 TD has been executed
Dave == Dave Tubbs dave.tu...@portalislc.com writes: Dave From: Dave Tubbs dave.tu...@portalislc.com Dave Make sure each c67x00 TD has been executed or retry using the existing Dave retry mechanism. Reference Cypress Semiconductor BIOS User's Manual 1.2, Dave page 3-16 Dave Signed-off-by: Dave Tubbs dave.tu...@portalislc.com Dave --- Dave drivers/usb/c67x00/c67x00-sched.c |6 ++ Dave 1 files changed, 6 insertions(+), 0 deletions(-) Dave diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c Dave index aa49162..dd5bdb4 100644 Dave --- a/drivers/usb/c67x00/c67x00-sched.c Dave +++ b/drivers/usb/c67x00/c67x00-sched.c Dave @@ -1033,6 +1033,12 @@ static inline void c67x00_check_td_list(struct c67x00_hcd *c67x00) Dave !td_acked(td)) Dave goto cont; Dave +/* at this point, there are no errors, but it's still possible that the Dave + * td wasn't executed by the c67x00. Confirm it was executed or force a Dave + * retry */ Dave +if(((td-retry_cnt) TD_RETRYCNTMASK_ACT_FLG) (td-status == 0)) Dave + goto cont; Dave + Alignment seems off and you have trailing spaces on the last line. Dave /* Sequence ok and acked, don't need to fix toggle */ Dave ack_ok = 1; Dave -- Dave 1.7.1 -- Bye, Peter Korsgaard -- 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 8/9] usb: chipidea: tell platform layer the ci core probe's result
On Thu, Feb 28, 2013 at 11:32:09AM +0200, Alexander Shishkin wrote: Felipe Balbi ba...@ti.com writes: Hi, On Thu, Feb 28, 2013 at 04:31:44PM +0800, Peter Chen wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. I'm not too familiar with the multitudes of platforms out there, but my simple question is: why can't we have pm runtime take care of enabling/disabling the clocks so that we don't have to do it in drivers? Seems obvious that a platform/SoC/board should know about it's clock tree structure, so why doesn't the platform code then take care of all the dirty details? I agree, clock stuffs should be handled at platform layer. For this corner case (core probe fails), if all of us agree with clock needs to be closed, we may need add some special handling. For runtime pm enabled, it is easy. we can set runtime pm active at fail path, as platform is parent of core, it will call platform's runtime suspend to do low power handling. For runtime pm disabled, we may had to add some ugly things, like notify core probe fail, and platform layer needs to handle this failed notify. It seems totally unreasonable and messy to add notion of clocks to drivers just because some platforms can't get their PM right. Just because dwc3/core.c doesn't know about clocks, it doesn't mean it's correct to hack it into the glue layer if that doesn't need the clock. Now that we know that's a bug, who's going to send me tested patches to teach dwc3/core.c about struct clk in a way that doesn't break PCIe, nor OMAP5 ? So are you sure that's what you want? Regards, -- Alex -- 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] usb: dwc3: fix EP_BUSY in case of dequeue
To reproduce the issue: -- Gadget dequeues all submitted requests to the endpoint. -- Some of them was not even queued to the dwc3 core. -- Such requests will never complete and a transfer completion interrupt for them will never be received. -- In such situation, we will not be clearing DWC3_EP_BUSY flag, even when there is no request queued to dwc3 core. -- Now gadget queues a request to the one endpoint say (BULK IN) -- It is added into dep-request_list -- Host sends an IN token -- Core responds with NAK and generates xfernotready -- Since DWC3_EP_BUSY is still set, so this request will never reach to core (dep-req_queued) This patch clears DWC3_EP_BUSY during ep_dequeue if none of the request was in dep-req_queued. Reported-by: Eric Tomio eric.to...@st.com Signed-off-by: Pratyush Anand pratyush.an...@st.com --- drivers/usb/dwc3/gadget.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c8f0765..0acf1a1 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1219,6 +1219,8 @@ out1: /* giveback the request */ dwc3_gadget_giveback(dep, req, -ECONNRESET); + if (list_empty(dep-req_queued)) + dep-flags = ~DWC3_EP_BUSY; out0: spin_unlock_irqrestore(dwc-lock, flags); -- 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 0/5] musb-ux500 and ab8500-usb updates
Hello Felipe, This series is an initial effort to update current UX500 specific MUSB/OTG drivers with the various improvements and fixes developed internally by ST-Ericsson since the driver was initially merged. The driver currently deployed by STE includes many updates such as runtime PM support, new AB85xx variants, integration with modern frameworks (common clock, pin control, regulator, devm_ variants etc...), and of course some fixes as well. In this initial set I only included some essential patches so that I get an early feedback from you if I'm moving in the right direction. Thanks, Fabio Fabio Baltieri (4): usb: musb: ux500: implement musb_set_vbus usb: musb: ux500: add otg notifier support usb: otg: ab8500-usb: drop support for ab8500 pre v2.0 usb: otg: ab8500-usb: update irq handling code Virupax Sadashivpetimath (1): usb: musb: ux500_dma: add missing MEM resource check drivers/usb/musb/ux500.c | 106 drivers/usb/musb/ux500_dma.c | 12 +- drivers/usb/otg/ab8500-usb.c | 531 ++--- include/linux/usb/musb-ux500.h | 31 +++ 4 files changed, 482 insertions(+), 198 deletions(-) create mode 100644 include/linux/usb/musb-ux500.h -- 1.8.1.3 -- 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: musb: ux500: implement musb_set_vbus
Add ux500_musb_set_vbus() implementation for ux500.
This is based on the version originally developed inside ST-Ericsson.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Fabio Baltieri fabio.balti...@linaro.org
---
drivers/usb/musb/ux500.c | 64
1 file changed, 64 insertions(+)
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 13a3929..5b742ba 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -36,6 +36,68 @@ struct ux500_glue {
};
#define glue_to_musb(g)platform_get_drvdata(g-musb)
+static void ux500_musb_set_vbus(struct musb *musb, int is_on)
+{
+ u8devctl;
+ unsigned long timeout = jiffies + msecs_to_jiffies(1000);
+ /* HDRC controls CPEN, but beware current surges during device
+* connect. They can trigger transient overcurrent conditions
+* that must be ignored.
+*/
+
+ devctl = musb_readb(musb-mregs, MUSB_DEVCTL);
+
+ if (is_on) {
+ if (musb-xceiv-state == OTG_STATE_A_IDLE) {
+ /* start the session */
+ devctl |= MUSB_DEVCTL_SESSION;
+ musb_writeb(musb-mregs, MUSB_DEVCTL, devctl);
+ /*
+* Wait for the musb to set as A device to enable the
+* VBUS
+*/
+ while (musb_readb(musb-mregs, MUSB_DEVCTL) 0x80) {
+
+ if (time_after(jiffies, timeout)) {
+ dev_err(musb-controller,
+ configured as A device timeout);
+ break;
+ }
+ }
+
+ } else {
+ musb-is_active = 1;
+ musb-xceiv-otg-default_a = 1;
+ musb-xceiv-state = OTG_STATE_A_WAIT_VRISE;
+ devctl |= MUSB_DEVCTL_SESSION;
+ MUSB_HST_MODE(musb);
+ }
+ } else {
+ musb-is_active = 0;
+
+ /* NOTE: we're skipping A_WAIT_VFALL - A_IDLE and jumping
+* right to B_IDLE...
+*/
+ musb-xceiv-otg-default_a = 0;
+ devctl = ~MUSB_DEVCTL_SESSION;
+ MUSB_DEV_MODE(musb);
+ }
+ musb_writeb(musb-mregs, MUSB_DEVCTL, devctl);
+
+ /*
+* Devctl values will be updated after vbus goes below
+* session_valid. The time taken depends on the capacitance
+* on VBUS line. The max discharge time can be upto 1 sec
+* as per the spec. Typically on our platform, it is 200ms
+*/
+ if (!is_on)
+ mdelay(200);
+
+ dev_dbg(musb-controller, VBUS %s, devctl %02x\n,
+ otg_state_string(musb-xceiv-state),
+ musb_readb(musb-mregs, MUSB_DEVCTL));
+}
+
static irqreturn_t ux500_musb_interrupt(int irq, void *__hci)
{
unsigned long flags;
@@ -79,6 +141,8 @@ static int ux500_musb_exit(struct musb *musb)
static const struct musb_platform_ops ux500_ops = {
.init = ux500_musb_init,
.exit = ux500_musb_exit,
+
+ .set_vbus = ux500_musb_set_vbus,
};
static int ux500_probe(struct platform_device *pdev)
--
1.8.1.3
--
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: otg: ab8500-usb: update irq handling code
Update irq handling code to notify all possible link status changes of
AB8500 and AB8505 to the ux500-musb glue driver. The additional event
codes will be used for pm-runtime implementation, and are defined in a
separate ux500-specific header.
This also modify the irq registration code to use devm_* helpers and
drop all non necessary fail path code.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Fabio Baltieri fabio.balti...@linaro.org
---
drivers/usb/musb/ux500.c | 7 +-
drivers/usb/otg/ab8500-usb.c | 440 -
include/linux/usb/musb-ux500.h | 31 +++
3 files changed, 382 insertions(+), 96 deletions(-)
create mode 100644 include/linux/usb/musb-ux500.h
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index b20326bb..88ae475 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -26,6 +26,7 @@
#include linux/err.h
#include linux/io.h
#include linux/platform_device.h
+#include linux/usb/musb-ux500.h
#include musb_core.h
@@ -107,14 +108,14 @@ static int musb_otg_notifications(struct notifier_block
*nb,
event, otg_state_string(musb-xceiv-state));
switch (event) {
- case USB_EVENT_ID:
+ case UX500_MUSB_ID:
dev_dbg(musb-controller, ID GND\n);
ux500_musb_set_vbus(musb, 1);
break;
- case USB_EVENT_VBUS:
+ case UX500_MUSB_VBUS:
dev_dbg(musb-controller, VBUS Connect\n);
break;
- case USB_EVENT_NONE:
+ case UX500_MUSB_NONE:
dev_dbg(musb-controller, VBUS Disconnect\n);
if (is_host_active(musb))
ux500_musb_set_vbus(musb, 0);
diff --git a/drivers/usb/otg/ab8500-usb.c b/drivers/usb/otg/ab8500-usb.c
index 9f5e0e4..351b036 100644
--- a/drivers/usb/otg/ab8500-usb.c
+++ b/drivers/usb/otg/ab8500-usb.c
@@ -31,9 +31,11 @@
#include linux/delay.h
#include linux/mfd/abx500.h
#include linux/mfd/abx500/ab8500.h
+#include linux/usb/musb-ux500.h
#define AB8500_MAIN_WD_CTRL_REG 0x01
#define AB8500_USB_LINE_STAT_REG 0x80
+#define AB8505_USB_LINE_STAT_REG 0x94
#define AB8500_USB_PHY_CTRL_REG 0x8A
#define AB8500_BIT_OTG_STAT_ID (1 0)
@@ -44,36 +46,76 @@
#define AB8500_WD_KICK_DELAY_US 100 /* usec */
#define AB8500_WD_V11_DISABLE_DELAY_US 100 /* usec */
+#define AB8500_V20_31952_DISABLE_DELAY_US 100 /* usec */
/* Usb line status register */
enum ab8500_usb_link_status {
- USB_LINK_NOT_CONFIGURED = 0,
- USB_LINK_STD_HOST_NC,
- USB_LINK_STD_HOST_C_NS,
- USB_LINK_STD_HOST_C_S,
- USB_LINK_HOST_CHG_NM,
- USB_LINK_HOST_CHG_HS,
- USB_LINK_HOST_CHG_HS_CHIRP,
- USB_LINK_DEDICATED_CHG,
- USB_LINK_ACA_RID_A,
- USB_LINK_ACA_RID_B,
- USB_LINK_ACA_RID_C_NM,
- USB_LINK_ACA_RID_C_HS,
- USB_LINK_ACA_RID_C_HS_CHIRP,
- USB_LINK_HM_IDGND,
- USB_LINK_RESERVED,
- USB_LINK_NOT_VALID_LINK
+ USB_LINK_NOT_CONFIGURED_8500 = 0,
+ USB_LINK_STD_HOST_NC_8500,
+ USB_LINK_STD_HOST_C_NS_8500,
+ USB_LINK_STD_HOST_C_S_8500,
+ USB_LINK_HOST_CHG_NM_8500,
+ USB_LINK_HOST_CHG_HS_8500,
+ USB_LINK_HOST_CHG_HS_CHIRP_8500,
+ USB_LINK_DEDICATED_CHG_8500,
+ USB_LINK_ACA_RID_A_8500,
+ USB_LINK_ACA_RID_B_8500,
+ USB_LINK_ACA_RID_C_NM_8500,
+ USB_LINK_ACA_RID_C_HS_8500,
+ USB_LINK_ACA_RID_C_HS_CHIRP_8500,
+ USB_LINK_HM_IDGND_8500,
+ USB_LINK_RESERVED_8500,
+ USB_LINK_NOT_VALID_LINK_8500,
+};
+
+enum ab8505_usb_link_status {
+ USB_LINK_NOT_CONFIGURED_8505 = 0,
+ USB_LINK_STD_HOST_NC_8505,
+ USB_LINK_STD_HOST_C_NS_8505,
+ USB_LINK_STD_HOST_C_S_8505,
+ USB_LINK_CDP_8505,
+ USB_LINK_RESERVED0_8505,
+ USB_LINK_RESERVED1_8505,
+ USB_LINK_DEDICATED_CHG_8505,
+ USB_LINK_ACA_RID_A_8505,
+ USB_LINK_ACA_RID_B_8505,
+ USB_LINK_ACA_RID_C_NM_8505,
+ USB_LINK_RESERVED2_8505,
+ USB_LINK_RESERVED3_8505,
+ USB_LINK_HM_IDGND_8505,
+ USB_LINK_CHARGERPORT_NOT_OK_8505,
+ USB_LINK_CHARGER_DM_HIGH_8505,
+ USB_LINK_PHYEN_NO_VBUS_NO_IDGND_8505,
+ USB_LINK_STD_UPSTREAM_NO_IDGNG_NO_VBUS_8505,
+ USB_LINK_STD_UPSTREAM_8505,
+ USB_LINK_CHARGER_SE1_8505,
+ USB_LINK_CARKIT_CHGR_1_8505,
+ USB_LINK_CARKIT_CHGR_2_8505,
+ USB_LINK_ACA_DOCK_CHGR_8505,
+ USB_LINK_SAMSUNG_BOOT_CBL_PHY_EN_8505,
+ USB_LINK_SAMSUNG_BOOT_CBL_PHY_DISB_8505,
+ USB_LINK_SAMSUNG_UART_CBL_PHY_EN_8505,
+ USB_LINK_SAMSUNG_UART_CBL_PHY_DISB_8505,
+ USB_LINK_MOTOROLA_FACTORY_CBL_PHY_EN_8505,
+};
+
+enum ab8500_usb_mode {
+ USB_IDLE = 0,
+ USB_PERIPHERAL,
+ USB_HOST,
+ USB_DEDICATED_CHG
};
struct ab8500_usb {
struct usb_phy phy;
struct device *dev;
struct ab8500 *ab8500;
- int irq_num_link_status;
unsigned
[PATCH 1/5] usb: musb: ux500_dma: add missing MEM resource check
From: Virupax Sadashivpetimath virupax.sadashivpetim...@stericsson.com
Fix dma_controller_create() fail path in case memory resource is
missing.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Virupax Sadashivpetimath
virupax.sadashivpetim...@stericsson.com
Signed-off-by: Fabio Baltieri fabio.balti...@linaro.org
---
drivers/usb/musb/ux500_dma.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index 360c99c..6e3db71 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -372,12 +372,17 @@ struct dma_controller *dma_controller_create(struct musb
*musb,
controller = kzalloc(sizeof(*controller), GFP_KERNEL);
if (!controller)
- return NULL;
+ goto kzalloc_fail;
controller-private_data = musb;
/* Save physical address for DMA controller. */
iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!iomem) {
+ dev_err(musb-controller, no memory resource defined\n);
+ goto plat_get_fail;
+ }
+
controller-phy_base = (dma_addr_t) iomem-start;
controller-controller.start = ux500_dma_controller_start;
@@ -389,4 +394,9 @@ struct dma_controller *dma_controller_create(struct musb
*musb,
controller-controller.is_compatible = ux500_dma_is_compatible;
return controller-controller;
+
+plat_get_fail:
+ kfree(controller);
+kzalloc_fail:
+ return NULL;
}
--
1.8.1.3
--
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: musb: ux500: add otg notifier support
Add transceiver notifier event handling to the ux500 driver to set vbus
on specific transceiver events.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Fabio Baltieri fabio.balti...@linaro.org
---
drivers/usb/musb/ux500.c | 41 +
1 file changed, 41 insertions(+)
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 5b742ba..b20326bb 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -98,6 +98,36 @@ static void ux500_musb_set_vbus(struct musb *musb, int is_on)
musb_readb(musb-mregs, MUSB_DEVCTL));
}
+static int musb_otg_notifications(struct notifier_block *nb,
+ unsigned long event, void *unused)
+{
+ struct musb *musb = container_of(nb, struct musb, nb);
+
+ dev_dbg(musb-controller, musb_otg_notifications %ld %s\n,
+ event, otg_state_string(musb-xceiv-state));
+
+ switch (event) {
+ case USB_EVENT_ID:
+ dev_dbg(musb-controller, ID GND\n);
+ ux500_musb_set_vbus(musb, 1);
+ break;
+ case USB_EVENT_VBUS:
+ dev_dbg(musb-controller, VBUS Connect\n);
+ break;
+ case USB_EVENT_NONE:
+ dev_dbg(musb-controller, VBUS Disconnect\n);
+ if (is_host_active(musb))
+ ux500_musb_set_vbus(musb, 0);
+ else
+ musb-xceiv-state = OTG_STATE_B_IDLE;
+ break;
+ default:
+ dev_dbg(musb-controller, ID float\n);
+ return NOTIFY_DONE;
+ }
+ return NOTIFY_OK;
+}
+
static irqreturn_t ux500_musb_interrupt(int irq, void *__hci)
{
unsigned long flags;
@@ -120,12 +150,21 @@ static irqreturn_t ux500_musb_interrupt(int irq, void
*__hci)
static int ux500_musb_init(struct musb *musb)
{
+ int status;
+
musb-xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
if (IS_ERR_OR_NULL(musb-xceiv)) {
pr_err(HS USB OTG: no transceiver configured\n);
return -EPROBE_DEFER;
}
+ musb-nb.notifier_call = musb_otg_notifications;
+ status = usb_register_notifier(musb-xceiv, musb-nb);
+ if (status 0) {
+ dev_dbg(musb-controller, notification register failed\n);
+ return status;
+ }
+
musb-isr = ux500_musb_interrupt;
return 0;
@@ -133,6 +172,8 @@ static int ux500_musb_init(struct musb *musb)
static int ux500_musb_exit(struct musb *musb)
{
+ usb_unregister_notifier(musb-xceiv, musb-nb);
+
usb_put_phy(musb-xceiv);
return 0;
--
1.8.1.3
--
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: otg: ab8500-usb: drop support for ab8500 pre v2.0
AB8500 versions preceding 2.0 were only used internally by ST-Ericsson
and are not supported anymore. This patch drops all v1.0 and v1.1
support code.
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Fabio Baltieri fabio.balti...@linaro.org
---
drivers/usb/otg/ab8500-usb.c | 139 ---
1 file changed, 11 insertions(+), 128 deletions(-)
diff --git a/drivers/usb/otg/ab8500-usb.c b/drivers/usb/otg/ab8500-usb.c
index 2d86f26..9f5e0e4 100644
--- a/drivers/usb/otg/ab8500-usb.c
+++ b/drivers/usb/otg/ab8500-usb.c
@@ -42,10 +42,8 @@
#define AB8500_BIT_WD_CTRL_ENABLE (1 0)
#define AB8500_BIT_WD_CTRL_KICK (1 1)
-#define AB8500_V1x_LINK_STAT_WAIT (HZ/10)
#define AB8500_WD_KICK_DELAY_US 100 /* usec */
#define AB8500_WD_V11_DISABLE_DELAY_US 100 /* usec */
-#define AB8500_WD_V10_DISABLE_DELAY_MS 100 /* ms */
/* Usb line status register */
enum ab8500_usb_link_status {
@@ -70,16 +68,12 @@ enum ab8500_usb_link_status {
struct ab8500_usb {
struct usb_phy phy;
struct device *dev;
- int irq_num_id_rise;
- int irq_num_id_fall;
- int irq_num_vbus_rise;
- int irq_num_vbus_fall;
+ struct ab8500 *ab8500;
int irq_num_link_status;
unsigned vbus_draw;
struct delayed_work dwork;
struct work_struct phy_dis_work;
unsigned long link_status_wait;
- int rev;
};
static inline struct ab8500_usb *phy_to_ab(struct usb_phy *x)
@@ -102,10 +96,7 @@ static void ab8500_usb_wd_workaround(struct ab8500_usb *ab)
(AB8500_BIT_WD_CTRL_ENABLE
| AB8500_BIT_WD_CTRL_KICK));
- if (ab-rev 0x10) /* v1.1 v2.0 */
- udelay(AB8500_WD_V11_DISABLE_DELAY_US);
- else /* v1.0 */
- msleep(AB8500_WD_V10_DISABLE_DELAY_MS);
+ udelay(AB8500_WD_V11_DISABLE_DELAY_US);
abx500_set_register_interruptible(ab-dev,
AB8500_SYS_CTRL2_BLOCK,
@@ -225,29 +216,6 @@ static void ab8500_usb_delayed_work(struct work_struct
*work)
ab8500_usb_link_status_update(ab);
}
-static irqreturn_t ab8500_usb_v1x_common_irq(int irq, void *data)
-{
- struct ab8500_usb *ab = (struct ab8500_usb *) data;
-
- /* Wait for link status to become stable. */
- schedule_delayed_work(ab-dwork, ab-link_status_wait);
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t ab8500_usb_v1x_vbus_fall_irq(int irq, void *data)
-{
- struct ab8500_usb *ab = (struct ab8500_usb *) data;
-
- /* Link status will not be updated till phy is disabled. */
- ab8500_usb_peri_phy_dis(ab);
-
- /* Wait for link status to become stable. */
- schedule_delayed_work(ab-dwork, ab-link_status_wait);
-
- return IRQ_HANDLED;
-}
-
static irqreturn_t ab8500_usb_v20_irq(int irq, void *data)
{
struct ab8500_usb *ab = (struct ab8500_usb *) data;
@@ -361,86 +329,7 @@ static int ab8500_usb_set_host(struct usb_otg *otg, struct
usb_bus *host)
static void ab8500_usb_irq_free(struct ab8500_usb *ab)
{
- if (ab-rev 0x20) {
- free_irq(ab-irq_num_id_rise, ab);
- free_irq(ab-irq_num_id_fall, ab);
- free_irq(ab-irq_num_vbus_rise, ab);
- free_irq(ab-irq_num_vbus_fall, ab);
- } else {
- free_irq(ab-irq_num_link_status, ab);
- }
-}
-
-static int ab8500_usb_v1x_res_setup(struct platform_device *pdev,
- struct ab8500_usb *ab)
-{
- int err;
-
- ab-irq_num_id_rise = platform_get_irq_byname(pdev, ID_WAKEUP_R);
- if (ab-irq_num_id_rise 0) {
- dev_err(pdev-dev, ID rise irq not found\n);
- return ab-irq_num_id_rise;
- }
- err = request_threaded_irq(ab-irq_num_id_rise, NULL,
- ab8500_usb_v1x_common_irq,
- IRQF_NO_SUSPEND | IRQF_SHARED,
- usb-id-rise, ab);
- if (err 0) {
- dev_err(ab-dev, request_irq failed for ID rise irq\n);
- goto fail0;
- }
-
- ab-irq_num_id_fall = platform_get_irq_byname(pdev, ID_WAKEUP_F);
- if (ab-irq_num_id_fall 0) {
- dev_err(pdev-dev, ID fall irq not found\n);
- return ab-irq_num_id_fall;
- }
- err = request_threaded_irq(ab-irq_num_id_fall, NULL,
- ab8500_usb_v1x_common_irq,
- IRQF_NO_SUSPEND | IRQF_SHARED,
- usb-id-fall, ab);
- if (err 0) {
- dev_err(ab-dev, request_irq failed for ID fall irq\n);
- goto fail1;
- }
-
- ab-irq_num_vbus_rise = platform_get_irq_byname(pdev, VBUS_DET_R);
- if (ab-irq_num_vbus_rise 0) {
- dev_err(pdev-dev, VBUS rise irq not found\n);
- return ab-irq_num_vbus_rise;
- }
- err = request_threaded_irq(ab-irq_num_vbus_rise, NULL,
- ab8500_usb_v1x_common_irq,
- IRQF_NO_SUSPEND | IRQF_SHARED,
-
Re: [PATCH v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
Hi, On Thu, Feb 28, 2013 at 11:32:09AM +0200, Alexander Shishkin wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. I'm not too familiar with the multitudes of platforms out there, but my simple question is: why can't we have pm runtime take care of enabling/disabling the clocks so that we don't have to do it in drivers? that's what OMAP does. Seems obvious that a platform/SoC/board should know about it's clock tree structure, so why doesn't the platform code then take care of all the dirty details? it might seem that way, but it's not that obvious ;-) Some platforms have a single clock, some will split interface and functional clocks, in some cases, you have extra optional clocks which might be needed during certain use cases or to implement erratas at locations that only driver knows. It's a tradeoff, of course. It seems totally unreasonable and messy to add notion of clocks to drivers just because some platforms can't get their PM right. it's not that simple ;-) Just because dwc3/core.c doesn't know about clocks, it doesn't mean it's correct to hack it into the glue layer if that doesn't need the clock. Now that we know that's a bug, who's going to send me tested patches to teach dwc3/core.c about struct clk in a way that doesn't break PCIe, nor OMAP5 ? So are you sure that's what you want? Well, how quickly can Exynos be changed to handle clocks without driver's knowledge ? Also, I'm a lot more 'at ease' when I see the driver explicitly handling all of its resources. The whole let's hide XYZ from driver because driver authors never get things right always causes problems. Specially in the ARM land where there's no standardization at all. When you think solely about x86 platforms, where you have ACPI properly standardized and anyone wanting to build an x86 processor needs to implement ACPI, it's a lot easier :-) I'm sure the new get default pinctrl state before driver probe() will cause issues down the road (think silicon erratas, shared balls on the BGA packaging). So, if they can hide clock handling from driver easily, good, let's do just that. Otherwise, meanwhile we need to cope with the extra power consumption that error condition would cause. -- balbi signature.asc Description: Digital signature
Re: [PATCH v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
Hi, On Thu, Feb 28, 2013 at 06:06:55PM +0800, Peter Chen wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. I'm not too familiar with the multitudes of platforms out there, but my simple question is: why can't we have pm runtime take care of enabling/disabling the clocks so that we don't have to do it in drivers? Seems obvious that a platform/SoC/board should know about it's clock tree structure, so why doesn't the platform code then take care of all the dirty details? I agree, clock stuffs should be handled at platform layer. For this corner case (core probe fails), if all of us agree with clock needs to be closed, we may need add some special handling. For runtime pm enabled, it is easy. we can set runtime pm active at fail path, as platform is parent of core, it will call platform's runtime suspend to do low power handling. if core probe fails, we should still call pm_runtime_put_sync() and pm_runtime_disable() and that should be enough to trigger your -pm_domain-runtime_suspend() which can be used to turn off unnecessary clocks. For runtime pm disabled, we may had to add some ugly things, like notify core probe fail, and platform layer needs to handle this failed notify. Please stop talking about about notify core probe fail that will never happen. Not today, not ever. Forget that idea. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: fix EP_BUSY in case of dequeue
Hi, On Thu, Feb 28, 2013 at 04:05:31PM +0530, Pratyush Anand wrote: To reproduce the issue: -- Gadget dequeues all submitted requests to the endpoint. -- Some of them was not even queued to the dwc3 core. -- Such requests will never complete and a transfer completion interrupt for them will never be received. -- In such situation, we will not be clearing DWC3_EP_BUSY flag, even when there is no request queued to dwc3 core. -- Now gadget queues a request to the one endpoint say (BULK IN) -- It is added into dep-request_list -- Host sends an IN token -- Core responds with NAK and generates xfernotready -- Since DWC3_EP_BUSY is still set, so this request will never reach to core (dep-req_queued) This patch clears DWC3_EP_BUSY during ep_dequeue if none of the request was in dep-req_queued. Reported-by: Eric Tomio eric.to...@st.com Signed-off-by: Pratyush Anand pratyush.an...@st.com --- drivers/usb/dwc3/gadget.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index c8f0765..0acf1a1 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1219,6 +1219,8 @@ out1: /* giveback the request */ dwc3_gadget_giveback(dep, req, -ECONNRESET); + if (list_empty(dep-req_queued)) + dep-flags = ~DWC3_EP_BUSY; not sure this is correct. Whenever req_queue isn't empty, we call dwc3_stop_active_transfer() which will clear DWC3_EP_BUSY flag. I guess bug is elsewhere. -- balbi signature.asc Description: Digital signature
[PATCH 2/4] USB: add devicetree helpers for determining dr_mode and phy_type
From: Michael Grzeschik m.grzesc...@pengutronix.de
This adds two little devicetree helper functions for determining the dr_mode
(host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the
devicetree.
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/of.c | 47 ++
drivers/usb/usb-common.c | 37
include/linux/usb/of.h | 28 +++
include/linux/usb/otg.h |8
include/linux/usb/phy.h |9 +
6 files changed, 130 insertions(+)
create mode 100644 drivers/usb/phy/of.c
create mode 100644 include/linux/usb/of.h
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 9fa6327..e1be1e8 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
obj-$(CONFIG_USB_OTG_UTILS)+= phy.o
+obj-$(CONFIG_OF) += of.o
obj-$(CONFIG_OMAP_USB2)+= omap-usb2.o
obj-$(CONFIG_OMAP_USB3)+= omap-usb3.o
obj-$(CONFIG_OMAP_CONTROL_USB) += omap-control-usb.o
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
new file mode 100644
index 000..e6f3b74
--- /dev/null
+++ b/drivers/usb/phy/of.c
@@ -0,0 +1,47 @@
+/*
+ * USB of helper code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/usb/of.h
+#include linux/usb/otg.h
+
+static const char *usbphy_modes[] = {
+ [USBPHY_INTERFACE_MODE_UNKNOWN] = ,
+ [USBPHY_INTERFACE_MODE_UTMI]= utmi,
+ [USBPHY_INTERFACE_MODE_UTMIW] = utmi_wide,
+ [USBPHY_INTERFACE_MODE_ULPI]= ulpi,
+ [USBPHY_INTERFACE_MODE_SERIAL] = serial,
+ [USBPHY_INTERFACE_MODE_HSIC]= hsic,
+};
+
+/**
+ * of_usb_get_phy_mode - Get phy mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy_type',
+ * and returns the correspondig enum usb_phy_interface
+ */
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+ const char *phy_type;
+ int err, i;
+
+ err = of_property_read_string(np, phy_type, phy_type);
+ if (err 0)
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+
+ for (i = 0; i ARRAY_SIZE(usbphy_modes); i++)
+ if (!strcmp(phy_type, usbphy_modes[i]))
+ return i;
+
+ return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index d29503e..18b994b 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -14,6 +14,9 @@
#include linux/kernel.h
#include linux/module.h
#include linux/usb/ch9.h
+#include linux/of.h
+#include linux/usb/of.h
+#include linux/usb/otg.h
const char *usb_speed_string(enum usb_device_speed speed)
{
@@ -32,4 +35,38 @@ const char *usb_speed_string(enum usb_device_speed speed)
}
EXPORT_SYMBOL_GPL(usb_speed_string);
+#ifdef CONFIG_OF
+static const char *usb_dr_modes[] = {
+ [USB_DR_MODE_UNKNOWN] = ,
+ [USB_DR_MODE_HOST] = host,
+ [USB_DR_MODE_PERIPHERAL]= peripheral,
+ [USB_DR_MODE_OTG] = otg,
+ [USB_DR_MODE_DUAL_ROLE] = dual-role,
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+ const char *dr_mode;
+ int err, i;
+
+ err = of_property_read_string(np, dr_mode, dr_mode);
+ if (err 0)
+ return USB_DR_MODE_UNKNOWN;
+
+ for (i = 0; i ARRAY_SIZE(usb_dr_modes); i++)
+ if (!strcmp(dr_mode, usb_dr_modes[i]))
+ return i;
+
+ return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+#endif
+
MODULE_LICENSE(GPL);
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
new file mode 100644
index 000..fc98f68
--- /dev/null
+++ b/include/linux/usb/of.h
@@ -0,0 +1,28 @@
+/*
+ * OF helpers for usb devices.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_USB_OF_H
+#define __LINUX_USB_OF_H
+
+#include linux/usb/phy.h
+#include linux/usb/otg.h
+
+#ifdef CONFIG_OF
+enum usb_phy_interface
[PATCH 4/4] USB mxs-phy: Register phy with framework
From: Sascha Hauer s.ha...@pengutronix.de
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/otg/mxs-phy.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index dff10f2..9d4381e 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -127,6 +127,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
void __iomem *base;
struct clk *clk;
struct mxs_phy *mxs_phy;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -166,11 +167,19 @@ static int mxs_phy_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mxs_phy-phy);
+ ret = usb_add_phy_dev(mxs_phy-phy);
+ if (ret)
+ return ret;
+
return 0;
}
static int mxs_phy_remove(struct platform_device *pdev)
{
+ struct mxs_phy *mxs_phy = platform_get_drvdata(pdev);
+
+ usb_remove_phy(mxs_phy-phy);
+
platform_set_drvdata(pdev, NULL);
return 0;
--
1.7.10.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 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions
This patch converts the mxs-phy driver from readl_relaxed(), writel_relaxed()
to the plain readl(), writel() functions, which are available on all platforms.
This is done to enable compile time testing on non ARM platforms.
Reported-by: Alexander Shishkin alexander.shish...@linux.intel.com
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/otg/mxs-phy.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index b0d9f11..dff10f2 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -48,12 +48,12 @@ static void mxs_phy_hw_init(struct mxs_phy *mxs_phy)
stmp_reset_block(base + HW_USBPHY_CTRL);
/* Power up the PHY */
- writel_relaxed(0, base + HW_USBPHY_PWD);
+ writel(0, base + HW_USBPHY_PWD);
/* enable FS/LS device */
- writel_relaxed(BM_USBPHY_CTRL_ENUTMILEVEL2 |
- BM_USBPHY_CTRL_ENUTMILEVEL3,
- base + HW_USBPHY_CTRL_SET);
+ writel(BM_USBPHY_CTRL_ENUTMILEVEL2 |
+ BM_USBPHY_CTRL_ENUTMILEVEL3,
+ base + HW_USBPHY_CTRL_SET);
}
static int mxs_phy_init(struct usb_phy *phy)
@@ -70,8 +70,8 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
{
struct mxs_phy *mxs_phy = to_mxs_phy(phy);
- writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
- phy-io_priv + HW_USBPHY_CTRL_SET);
+ writel(BM_USBPHY_CTRL_CLKGATE,
+ phy-io_priv + HW_USBPHY_CTRL_SET);
clk_disable_unprepare(mxs_phy-clk);
}
@@ -81,15 +81,15 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
struct mxs_phy *mxs_phy = to_mxs_phy(x);
if (suspend) {
- writel_relaxed(0x, x-io_priv + HW_USBPHY_PWD);
- writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
- x-io_priv + HW_USBPHY_CTRL_SET);
+ writel(0x, x-io_priv + HW_USBPHY_PWD);
+ writel(BM_USBPHY_CTRL_CLKGATE,
+ x-io_priv + HW_USBPHY_CTRL_SET);
clk_disable_unprepare(mxs_phy-clk);
} else {
clk_prepare_enable(mxs_phy-clk);
- writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
- x-io_priv + HW_USBPHY_CTRL_CLR);
- writel_relaxed(0, x-io_priv + HW_USBPHY_PWD);
+ writel(BM_USBPHY_CTRL_CLKGATE,
+ x-io_priv + HW_USBPHY_CTRL_CLR);
+ writel(0, x-io_priv + HW_USBPHY_PWD);
}
return 0;
@@ -102,8 +102,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy,
(speed == USB_SPEED_HIGH) ? high : non-high);
if (speed == USB_SPEED_HIGH)
- writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
- phy-io_priv + HW_USBPHY_CTRL_SET);
+ writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy-io_priv + HW_USBPHY_CTRL_SET);
return 0;
}
@@ -115,8 +115,8 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
(speed == USB_SPEED_HIGH) ? high : non-high);
if (speed == USB_SPEED_HIGH)
- writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
- phy-io_priv + HW_USBPHY_CTRL_CLR);
+ writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy-io_priv + HW_USBPHY_CTRL_CLR);
return 0;
}
--
1.7.10.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 1/4] USB: move bulk of otg/otg.c to phy/phy.c
From: Sascha Hauer s.ha...@pengutronix.de
Most of otg/otg.c is not otg specific, but phy specific, so move it
to the phy directory.
Cc: Felipe Balbi ba...@ti.com
Reported-by: Kishon Vijay Abraham I kis...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/otg/otg.c| 427
drivers/usb/phy/Makefile |1 +
drivers/usb/phy/phy.c| 438 ++
3 files changed, 439 insertions(+), 427 deletions(-)
create mode 100644 drivers/usb/phy/phy.c
diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index 2bd03d2..358cfd9 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -8,436 +8,9 @@
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*/
-
-#include linux/kernel.h
#include linux/export.h
-#include linux/err.h
-#include linux/device.h
-#include linux/module.h
-#include linux/slab.h
-#include linux/of.h
-
#include linux/usb/otg.h
-static LIST_HEAD(phy_list);
-static LIST_HEAD(phy_bind_list);
-static DEFINE_SPINLOCK(phy_lock);
-
-static struct usb_phy *__usb_find_phy(struct list_head *list,
- enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
-
- list_for_each_entry(phy, list, head) {
- if (phy-type != type)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__usb_find_phy_dev(struct device *dev,
- struct list_head *list, u8 index)
-{
- struct usb_phy_bind *phy_bind = NULL;
-
- list_for_each_entry(phy_bind, list, list) {
- if (!(strcmp(phy_bind-dev_name, dev_name(dev)))
- phy_bind-index == index) {
- if (phy_bind-phy)
- return phy_bind-phy;
- else
- return ERR_PTR(-EPROBE_DEFER);
- }
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__of_usb_find_phy(struct device_node *node)
-{
- struct usb_phy *phy;
-
- list_for_each_entry(phy, phy_list, head) {
- if (node != phy-dev-of_node)
- continue;
-
- return phy;
- }
-
- return ERR_PTR(-ENODEV);
-}
-
-static void devm_usb_phy_release(struct device *dev, void *res)
-{
- struct usb_phy *phy = *(struct usb_phy **)res;
-
- usb_put_phy(phy);
-}
-
-static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
-{
- return res == match_data;
-}
-
-/**
- * devm_usb_get_phy - find the USB PHY
- * @dev - device that requests this phy
- * @type - the type of the phy the controller requires
- *
- * Gets the phy using usb_get_phy(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
-{
- struct usb_phy **ptr, *phy;
-
- ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- phy = usb_get_phy(type);
- if (!IS_ERR(phy)) {
- *ptr = phy;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy);
-
-/**
- * usb_get_phy - find the USB PHY
- * @type - the type of the phy the controller requires
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy. The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
- struct usb_phy *phy = NULL;
- unsigned long flags;
-
- spin_lock_irqsave(phy_lock, flags);
-
- phy = __usb_find_phy(phy_list, type);
- if (IS_ERR(phy) || !try_module_get(phy-dev-driver-owner)) {
- pr_err(unable to find transceiver of type %s\n,
- usb_phy_type_string(type));
- goto err0;
- }
-
- get_device(phy-dev);
-
-err0:
- spin_unlock_irqrestore(phy_lock, flags);
-
- return phy;
-}
-EXPORT_SYMBOL(usb_get_phy);
-
- /**
- * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
- * @dev - device that requests this phy
- * @phandle - name of the property holding the phy phandle value
- * @index - the index of the phy
- *
- * Returns the phy driver associated with the given phandle value,
- * after getting a refcount to it, -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. While at that, it also associates the device with
- * the phy
[PATCH 0/4] otg-for-v3.10-v2: separate phy code and add DT helper
Hello,
this series depends on the bugfix patch USB otg: use try_module_get in all
usb_get_phy functions and add missing module_put (a.k.a tags/otg-for-v3.9-v1)
posted earlier and is inteded for v3.10. It separates the phy from the otg code
and adds DT helper functions. In mxs-phy the {read,write}l_relaxed() functions
are replaced by the non _relaxed version to improve compile time coverage.
Further mxs-phy makes now use of the new usb_add_phy_dev() function to register
it's phy.
regards,
Marc
---
changes since v1:
- fix compile time breakage on non DT platforms (tnx, Alexander)
- convert mxs-phy to non _relaxed {read,write}l_relaxed() functions
(as requested by Alexander)
---
The following changes since commit 6bef020b4aebd7886281fb7fb37c788d5a365eea:
USB otg: use try_module_get in all usb_get_phy functions and add missing
module_put (2013-02-27 12:53:15 +0100)
are available in the git repository at:
git://git.pengutronix.de/git/mkl/linux.git tags/otg-for-v3.10-v2
for you to fetch changes up to f5678b135967ea98256ee5df9a360b5769861d23:
USB mxs-phy: Register phy with framework (2013-02-28 11:36:45 +0100)
USB otg, phy: separate phy and add DT helper
Move phy related code into separate file and add device tree helper functions.
Marc Kleine-Budde (1):
USB mxs-phy: use readl(), writel() instead of the _relaxed() versions
Michael Grzeschik (1):
USB: add devicetree helpers for determining dr_mode and phy_type
Sascha Hauer (2):
USB: move bulk of otg/otg.c to phy/phy.c
USB mxs-phy: Register phy with framework
drivers/usb/otg/mxs-phy.c | 41 +++--
drivers/usb/otg/otg.c | 427 ---
drivers/usb/phy/Makefile |2 +
drivers/usb/phy/of.c | 47 +
drivers/usb/phy/phy.c | 438 +
drivers/usb/usb-common.c | 37
include/linux/usb/of.h| 28 +++
include/linux/usb/otg.h |8 +
include/linux/usb/phy.h |9 +
9 files changed, 594 insertions(+), 443 deletions(-)
create mode 100644 drivers/usb/phy/of.c
create mode 100644 drivers/usb/phy/phy.c
create mode 100644 include/linux/usb/of.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 3/3] USB mxs-phy: Register phy with framework
On 02/28/2013 10:40 AM, Alexander Shishkin wrote:
Marc Kleine-Budde m...@pengutronix.de writes:
On 02/28/2013 09:01 AM, Felipe Balbi wrote:
hi,
On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
From: Sascha Hauer s.ha...@pengutronix.de
We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.
Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
any chance you can move away from {write,read}[bwl]_relaxed() so we can
build this driver on other architectures ?
The hardware is in the ARM imx2{2,8} only. Another option would be to
add an depends on ARCH_ARM.
Doesn't mean that we shouldn't be able to compile-test the driver.
Fixed in the just posted otg-for-v3.10-v2 series.
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
[PATCH 0/5] chipidea-for-v3.10-v2: USB chipidea: make use of DT helpers in chipidea driver improve driver
Hello, this series depends on the series [PATCH 0/3] otg-for-v3.10-v2: separate phy code and add DT helper (a.k.a. tags/otg-for-v3.10-v2) posted earlier and is intended for v3.10. The chipidea driver is converted to make use of the DT helper functions. No changes since v1, just rebased to otg-for-v3.10-v2, in case someone wants to pull it. regards, Marc --- The following changes since commit f5678b135967ea98256ee5df9a360b5769861d23: USB mxs-phy: Register phy with framework (2013-02-28 11:36:45 +0100) are available in the git repository at: git://git.pengutronix.de/git/mkl/linux.git tags/chipidea-for-v3.10-v2 for you to fetch changes up to 11e56207b94d65f92acdee17c795753378c581c6: USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-28 11:36:48 +0100) USB chipidea: make use of DT helpers in chipidea driver Make use of DT helper functions for handling the dr_mode and phy_type property. Michael Grzeschik (2): USB chipidea: ci13xxx-imx: create dynamic platformdata USB chipidea: add PTW and PTS handling Sascha Hauer (3): USB chipidea: introduce dual role mode pdata flags USB chipidea i.MX: introduce dr_mode property USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy .../devicetree/bindings/usb/ci13xxx-imx.txt|6 ++ drivers/usb/chipidea/bits.h| 14 +++- drivers/usb/chipidea/ci13xxx_imx.c | 67 +++- drivers/usb/chipidea/core.c| 61 -- include/linux/usb/chipidea.h |3 +- 5 files changed, 112 insertions(+), 39 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 5/5] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy
From: Sascha Hauer s.ha...@pengutronix.de
This patch replaces the hand crafted code to retrieve the phy's phandle from
the DT by the helper function devm_usb_get_phy_by_phandle() which has been
added in commit:
5d3c28b usb: otg: add device tree support to otg library
Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/chipidea/ci13xxx_imx.c | 38
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index b598bb8..6e720ce 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -30,7 +30,6 @@
((struct usb_phy *)platform_get_drvdata(pdev))
struct ci13xxx_imx_data {
- struct device_node *phy_np;
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
@@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
struct ci13xxx_platform_data *pdata;
- struct platform_device *plat_ci, *phy_pdev;
- struct device_node *phy_np;
+ struct platform_device *plat_ci;
struct resource *res;
struct regulator *reg_vbus;
struct pinctrl *pinctrl;
int ret;
+ struct usb_phy *phy;
if (of_find_property(pdev-dev.of_node, fsl,usbmisc, NULL)
!usbmisc_ops)
@@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
return ret;
}
- phy_np = of_parse_phandle(pdev-dev.of_node, fsl,usbphy, 0);
- if (phy_np) {
- data-phy_np = phy_np;
- phy_pdev = of_find_device_by_node(phy_np);
- if (phy_pdev) {
- struct usb_phy *phy;
- phy = pdev_to_phy(phy_pdev);
- if (phy
- try_module_get(phy_pdev-dev.driver-owner)) {
- usb_phy_init(phy);
- data-phy = phy;
- }
+ phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
+ if (PTR_ERR(phy) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_clk;
+ }
+
+ if (!IS_ERR(phy)) {
+ ret = usb_phy_init(phy);
+ if (ret) {
+ dev_err(pdev-dev, unable to init phy: %d\n, ret);
+ goto err_clk;
}
+
+ data-phy = phy;
}
/* we only support host now, so enable vbus here */
@@ -170,7 +170,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
dev_err(pdev-dev,
Failed to enable vbus regulator, err=%d\n,
ret);
- goto put_np;
+ goto err_clk;
}
data-reg_vbus = reg_vbus;
} else {
@@ -222,9 +222,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
err:
if (reg_vbus)
regulator_disable(reg_vbus);
-put_np:
- if (phy_np)
- of_node_put(phy_np);
+err_clk:
clk_disable_unprepare(data-clk);
return ret;
}
@@ -244,8 +242,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
module_put(data-phy-dev-driver-owner);
}
- of_node_put(data-phy_np);
-
clk_disable_unprepare(data-clk);
platform_set_drvdata(pdev, NULL);
--
1.7.10.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 4/5] USB chipidea i.MX: introduce dr_mode property
From: Sascha Hauer s.ha...@pengutronix.de
The dr_mode devicetree property allows to explicitly specify the
host/peripheral/otg mode. This is necessary for boards without proper
ID pin handling.
Reviewed-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
Documentation/devicetree/bindings/usb/ci13xxx-imx.txt |1 +
drivers/usb/chipidea/ci13xxx_imx.c|1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index dd42ccd..493a414 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -9,6 +9,7 @@ Recommended properies:
- phy_type: the type of the phy connected to the core. Should be one
of utmi, utmi_wide, ulpi, serial or hsic. Without this
property the PORTSC register won't be touched
+- dr_mode: One of host, peripheral or otg. Defaults to otg
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index ebc1148..b598bb8 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -114,6 +114,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_DISABLE_STREAMING;
pdata-phy_mode = of_usb_get_phy_mode(pdev-dev.of_node);
+ pdata-dr_mode = of_usb_get_dr_mode(pdev-dev.of_node);
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
--
1.7.10.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 1/5] USB chipidea: ci13xxx-imx: create dynamic platformdata
From: Michael Grzeschik m.grzesc...@pengutronix.de
This patch removes the limitation of having only one instance of the
ci13xxx-imx platformdata and makes different configurations possible.
Reviewed-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/chipidea/ci13xxx_imx.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..69024e0 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -85,17 +85,10 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
/* End of common functions shared by usbmisc drivers*/
-static struct ci13xxx_platform_data ci13xxx_imx_platdata = {
- .name = ci13xxx_imx,
- .flags = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
- CI13XXX_DISABLE_STREAMING,
- .capoffset = DEF_CAPOFFSET,
-};
-
static int ci13xxx_imx_probe(struct platform_device *pdev)
{
struct ci13xxx_imx_data *data;
+ struct ci13xxx_platform_data *pdata;
struct platform_device *plat_ci, *phy_pdev;
struct device_node *phy_np;
struct resource *res;
@@ -107,6 +100,18 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
!usbmisc_ops)
return -EPROBE_DEFER;
+ pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(pdev-dev, Failed to allocate CI13xxx-IMX pdata!\n);
+ return -ENOMEM;
+ }
+
+ pdata-name = ci13xxx_imx;
+ pdata-capoffset = DEF_CAPOFFSET;
+ pdata-flags = CI13XXX_REQUIRE_TRANSCEIVER |
+ CI13XXX_PULLUP_ON_VBUS |
+ CI13XXX_DISABLE_STREAMING;
+
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(pdev-dev, Failed to allocate CI13xxx-IMX data!\n);
@@ -168,7 +173,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
reg_vbus = NULL;
}
- ci13xxx_imx_platdata.phy = data-phy;
+ pdata-phy = data-phy;
if (!pdev-dev.dma_mask) {
pdev-dev.dma_mask = devm_kzalloc(pdev-dev,
@@ -193,7 +198,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
plat_ci = ci13xxx_add_device(pdev-dev,
pdev-resource, pdev-num_resources,
- ci13xxx_imx_platdata);
+ pdata);
if (IS_ERR(plat_ci)) {
ret = PTR_ERR(plat_ci);
dev_err(pdev-dev,
--
1.7.10.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 2/5] USB chipidea: add PTW and PTS handling
From: Michael Grzeschik m.grzesc...@pengutronix.de
This patch makes it possible to configure the PTW and PTS bits inside
the portsc register for host and device mode before the driver starts
and the phy can be addressed as hardware implementation is designed.
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
.../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
drivers/usb/chipidea/bits.h| 14 ++-
drivers/usb/chipidea/ci13xxx_imx.c |3 ++
drivers/usb/chipidea/core.c| 39
include/linux/usb/chipidea.h |1 +
5 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index 5778b9c..dd42ccd 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -5,6 +5,11 @@ Required properties:
- reg: Should contain registers location and length
- interrupts: Should contain controller interrupt
+Recommended properies:
+- phy_type: the type of the phy connected to the core. Should be one
+ of utmi, utmi_wide, ulpi, serial or hsic. Without this
+ property the PORTSC register won't be touched
+
Optional properties:
- fsl,usbphy: phandler of usb phy that connects to the only one port
- fsl,usbmisc: phandler of non-core register device, with one argument
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..d8ffc2f 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -48,10 +48,22 @@
#define PORTSC_SUSP BIT(7)
#define PORTSC_HSPBIT(9)
#define PORTSC_PTC(0x0FUL 16)
+/* PTS and PTW for non lpm version only */
+#define PORTSC_PTS(d) d) 0x3) 30) | (((d) 0x4) ? BIT(25) :
0))
+#define PORTSC_PTWBIT(28)
/* DEVLC */
#define DEVLC_PSPD(0x03UL 25)
-#defineDEVLC_PSPD_HS (0x02UL 25)
+#define DEVLC_PSPD_HS (0x02UL 25)
+#define DEVLC_PTW BIT(27)
+#define DEVLC_STS BIT(28)
+#define DEVLC_PTS(d) (((d) 0x7) 29)
+
+/* Encoding for DEVLC_PTS and PORTSC_PTS */
+#define PTS_UTMI 0
+#define PTS_ULPI 2
+#define PTS_SERIAL3
+#define PTS_HSIC 4
/* OTGSC */
#define OTGSC_IDPU BIT(5)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 69024e0..ebc1148 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -21,6 +21,7 @@
#include linux/clk.h
#include linux/regulator/consumer.h
#include linux/pinctrl/consumer.h
+#include linux/usb/of.h
#include ci.h
#include ci13xxx_imx.h
@@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
CI13XXX_PULLUP_ON_VBUS |
CI13XXX_DISABLE_STREAMING;
+ pdata-phy_mode = of_usb_get_phy_mode(pdev-dev.of_node);
+
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(pdev-dev, Failed to allocate CI13xxx-IMX data!\n);
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 57cae1f..04d68cb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -67,6 +67,8 @@
#include linux/usb/gadget.h
#include linux/usb/otg.h
#include linux/usb/chipidea.h
+#include linux/usb/of.h
+#include linux/phy.h
#include ci.h
#include udc.h
@@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem
*base)
return 0;
}
+static void hw_phymode_configure(struct ci13xxx *ci)
+{
+ u32 portsc, lpm;
+
+ switch (ci-platdata-phy_mode) {
+ case USBPHY_INTERFACE_MODE_UTMI:
+ portsc = PORTSC_PTS(PTS_UTMI);
+ lpm = DEVLC_PTS(PTS_UTMI);
+ break;
+ case USBPHY_INTERFACE_MODE_UTMIW:
+ portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
+ lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
+ break;
+ case USBPHY_INTERFACE_MODE_ULPI:
+ portsc = PORTSC_PTS(PTS_ULPI);
+ lpm = DEVLC_PTS(PTS_ULPI);
+ break;
+ case USBPHY_INTERFACE_MODE_SERIAL:
+ portsc = PORTSC_PTS(PTS_SERIAL);
+ lpm = DEVLC_PTS(PTS_SERIAL);
+ break;
+ case USBPHY_INTERFACE_MODE_HSIC:
+ portsc = PORTSC_PTS(PTS_HSIC);
+ lpm = DEVLC_PTS(PTS_HSIC);
+ break;
+ default:
+ return;
+ }
+
+ if (ci-hw_bank.lpm)
+ hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
+ else
+ hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
+}
+
[PATCH 3/5] USB chipidea: introduce dual role mode pdata flags
From: Sascha Hauer s.ha...@pengutronix.de
Even if a chipidea core is otg capable the board may not. This allows
to explicitly set the core to host/peripheral mode. Without these
flags the driver falls back to the old behaviour.
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
drivers/usb/chipidea/core.c | 22 --
include/linux/usb/chipidea.h |2 +-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 04d68cb..ec27060 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
struct resource *res;
void __iomem*base;
int ret;
+ enum usb_dr_mode dr_mode;
if (!dev-platform_data) {
dev_err(dev, platform data missing\n);
@@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /* For now we treat dual-role as otg */
+ dr_mode = ci-platdata-dr_mode;
+ if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == USB_DR_MODE_DUAL_ROLE)
+ dr_mode = USB_DR_MODE_OTG;
+
/* initialize role(s) before the interrupt is requested */
- ret = ci_hdrc_host_init(ci);
- if (ret)
- dev_info(dev, doesn't support host\n);
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
+ ret = ci_hdrc_host_init(ci);
+ if (ret)
+ dev_info(dev, doesn't support host\n);
+ }
- ret = ci_hdrc_gadget_init(ci);
- if (ret)
- dev_info(dev, doesn't support gadget\n);
+ if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
+ ret = ci_hdrc_gadget_init(ci);
+ if (ret)
+ dev_info(dev, doesn't support gadget\n);
+ }
if (!ci-roles[CI_ROLE_HOST] !ci-roles[CI_ROLE_GADGET]) {
dev_err(dev, no supported roles\n);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 1a2aa18..b314647 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,7 +20,7 @@ struct ci13xxx_platform_data {
#define CI13XXX_REQUIRE_TRANSCEIVERBIT(1)
#define CI13XXX_PULLUP_ON_VBUS BIT(2)
#define CI13XXX_DISABLE_STREAMING BIT(3)
-
+ enum usb_dr_modedr_mode;
#define CI13XXX_CONTROLLER_RESET_EVENT 0
#define CI13XXX_CONTROLLER_STOPPED_EVENT 1
void(*notify_event) (struct ci13xxx *ci, unsigned event);
--
1.7.10.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: About chipidea tree
Peter Chen peter.c...@freescale.com writes: On Tue, Feb 26, 2013 at 02:47:34PM +0100, Marc Kleine-Budde wrote: On 02/26/2013 02:25 PM, Alexander Shishkin wrote: Marc Kleine-Budde m...@pengutronix.de writes: On 02/26/2013 11:56 AM, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: Hi Alex, Hi, Do we have a chipidea repo which is queued for mainline? We have several patchsets for chipidea these monthes, I don't know their status. For me, I would like based on your tree if it exists. I thought about it, but then it seems like having a separate branch is bound to be confusing to most people. I'd much rather prefer everything go to usb-next, and this is my current plan. Since Greg will start applying new stuff to usb-next after -rc1 is tagged, I'll send my current stash of patches for inclusion then. If your patchset, for Can we have a look at your queued patches? Sure, http://marc.info/?l=linux-usbm=135902434508839 example, has conflicts with my stuff that's not merged, I'll try to take care of resolving the conflicts and submit everything to Greg. In other words, it should be always ok to base your chipidea patchsets on usb-next. Let me know if this sounds reasonable to you. Michael has already done that work (some S-o-b form Michael are missing) and rebased Sascha's and Peter's patches to current linus/master, see this tree : http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 Taking a quick look, quite some of those patches are not ready for inclusion yet. So if the question is, do we need a -next tree for all the chipidea patchsets floating around, it might be a good thing. But it's not what Peter was asking in the first place. I suggest that we have a branch that holds all chipidea patches that are ready for mainline. Otherwise it's really hard to code any new features I agree. Alex, you can have a repo at github or any other places which is based on usb-next, and add it to MAINTAINERS. We can develop the new feature based on your repo. Greg can pull it directly if he agrees or you can send your queued patchset before every merge windows. Ok, let's try this. I have a linux-ci repo on github, might as well do something useful with it [1], [2]. Currently, the branch called ci-for-greg is where I stack patches that I'm planning to be sending (via email) to Greg when the time is right. The policy is such that it'll be rebased on top of Greg's usb-next and probably often, so no fast forwards. Also patches may be dropped from it if necessary. Since the branch is not for pulling, the no rebase rule doesn't apply. If you have comments/suggestions/etc for a patch that is in this branch, please reply to the email with that patch on the mailing list and not via github infrastructure. Sounds reasonable? I'm now scouting my inbox for more candidates for this branch. Please don't re-send the patches that have already been sent, unless you have new versions of those, I have them all. Do send new versions, though. [1] git://github.com/virtuoso/linux-ci.git ci-for-greg [2] https://github.com/virtuoso/linux-ci/commits/ci-for-greg Regards, -- Alex -- 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 2/3] USB: add devicetree helpers for determining dr_mode and phy_type
On Thu, Feb 28, 2013 at 10:23:03AM +0100, Marc Kleine-Budde wrote: On 02/28/2013 08:57 AM, Felipe Balbi wrote: On Wed, Feb 27, 2013 at 03:16:29PM +0100, Marc Kleine-Budde wrote: From: Michael Grzeschik m.grzesc...@pengutronix.de This adds two little devicetree helper functions for determining the dr_mode (host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the devicetree. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de Signed-off-by: Sascha Hauer s.ha...@pengutronix.de Signed-off-by: Marc Kleine-Budde m...@pengutronix.de causes build breakage: include/linux/usb/of.h:21:32: error: return type is an incomplete type include/linux/usb/of.h: In function ‘of_usb_get_dr_mode’: include/linux/usb/of.h:23:9: error: ‘USB_DR_MODE_UNKNOWN’ undeclared (first use in this function) include/linux/usb/of.h:23:9: note: each undeclared identifier is reported only once for each function it appears in include/linux/usb/of.h:23:2: warning: ‘return’ with a value, in function returning void [enabled by default] make[1]: *** [drivers/usb/usb-common.o] Error 1 Doh! That occurs only for non DT kernels, but who doesn't use device tree these days? x86 and a bunch of ARM-SoC which haven't been converted yet -- balbi signature.asc Description: Digital signature
Re: [PATCH v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
Felipe Balbi ba...@ti.com writes: Hi, On Thu, Feb 28, 2013 at 11:32:09AM +0200, Alexander Shishkin wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. I'm not too familiar with the multitudes of platforms out there, but my simple question is: why can't we have pm runtime take care of enabling/disabling the clocks so that we don't have to do it in drivers? that's what OMAP does. Seems obvious that a platform/SoC/board should know about it's clock tree structure, so why doesn't the platform code then take care of all the dirty details? it might seem that way, but it's not that obvious ;-) Some platforms have a single clock, some will split interface and functional clocks, in some cases, you have extra optional clocks which might be needed during certain use cases or to implement erratas at locations that only driver knows. Then drivers could use platform fixup callbacks. Curious how many drivers out there do proper handling of interface vs functional clocks. Something tells me that the common pattern will be enable all clocks with lots of line of boilerplate copied from that other driver in probe() and disable all in remove(). It's a tradeoff, of course. It seems totally unreasonable and messy to add notion of clocks to drivers just because some platforms can't get their PM right. it's not that simple ;-) Just because dwc3/core.c doesn't know about clocks, it doesn't mean it's correct to hack it into the glue layer if that doesn't need the clock. Now that we know that's a bug, who's going to send me tested patches to teach dwc3/core.c about struct clk in a way that doesn't break PCIe, nor OMAP5 ? So are you sure that's what you want? Well, how quickly can Exynos be changed to handle clocks without driver's knowledge ? Motivation for the platforms to change should come from the general direction of linux kernel maintainers, in order for the change to happen. :) But yes, for the time being, you're right, we'll probably have to just cope with it. Also, I'm a lot more 'at ease' when I see the driver explicitly handling all of its resources. The whole let's hide XYZ from driver because driver authors never get things right always causes problems. Specially in the ARM land where there's no standardization at all. I think it's going to be like that as long as developers are working in hack it and ship it and let somebody else figure out APIs routine.
Re: About chipidea tree
On 02/28/2013 12:16 PM, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: On Tue, Feb 26, 2013 at 02:47:34PM +0100, Marc Kleine-Budde wrote: On 02/26/2013 02:25 PM, Alexander Shishkin wrote: Marc Kleine-Budde m...@pengutronix.de writes: On 02/26/2013 11:56 AM, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: Hi Alex, Hi, Do we have a chipidea repo which is queued for mainline? We have several patchsets for chipidea these monthes, I don't know their status. For me, I would like based on your tree if it exists. I thought about it, but then it seems like having a separate branch is bound to be confusing to most people. I'd much rather prefer everything go to usb-next, and this is my current plan. Since Greg will start applying new stuff to usb-next after -rc1 is tagged, I'll send my current stash of patches for inclusion then. If your patchset, for Can we have a look at your queued patches? Sure, http://marc.info/?l=linux-usbm=135902434508839 example, has conflicts with my stuff that's not merged, I'll try to take care of resolving the conflicts and submit everything to Greg. In other words, it should be always ok to base your chipidea patchsets on usb-next. Let me know if this sounds reasonable to you. Michael has already done that work (some S-o-b form Michael are missing) and rebased Sascha's and Peter's patches to current linus/master, see this tree : http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 Taking a quick look, quite some of those patches are not ready for inclusion yet. So if the question is, do we need a -next tree for all the chipidea patchsets floating around, it might be a good thing. But it's not what Peter was asking in the first place. I suggest that we have a branch that holds all chipidea patches that are ready for mainline. Otherwise it's really hard to code any new features I agree. Alex, you can have a repo at github or any other places which is based on usb-next, and add it to MAINTAINERS. We can develop the new feature based on your repo. Greg can pull it directly if he agrees or you can send your queued patchset before every merge windows. Ok, let's try this. I have a linux-ci repo on github, might as well do something useful with it [1], [2]. Currently, the branch called ci-for-greg is where I stack patches that I'm planning to be sending (via email) to Greg when the time is right. The policy is such that it'll be rebased on top of Greg's usb-next and probably often, so no fast forwards. Also patches may be dropped from it if necessary. Since the branch is not for pulling, the no rebase rule doesn't apply. If you have comments/suggestions/etc for a patch that is in this branch, please reply to the email with that patch on the mailing list and not via github infrastructure. Sounds reasonable? go ahead. 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 3/3] usb: Make sure each c67x00 TD has been executed
Hello. On 28-02-2013 13:59, Peter Korsgaard wrote: Dave From: Dave Tubbs dave.tu...@portalislc.com Dave Make sure each c67x00 TD has been executed or retry using the existing Dave retry mechanism. Reference Cypress Semiconductor BIOS User's Manual 1.2, Dave page 3-16 Dave Signed-off-by: Dave Tubbs dave.tu...@portalislc.com Dave --- Dave drivers/usb/c67x00/c67x00-sched.c |6 ++ Dave 1 files changed, 6 insertions(+), 0 deletions(-) Dave diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c Dave index aa49162..dd5bdb4 100644 Dave --- a/drivers/usb/c67x00/c67x00-sched.c Dave +++ b/drivers/usb/c67x00/c67x00-sched.c Dave @@ -1033,6 +1033,12 @@ static inline void c67x00_check_td_list(struct c67x00_hcd *c67x00) Dave !td_acked(td)) Dave goto cont; Dave +/* at this point, there are no errors, but it's still possible that the Dave + * td wasn't executed by the c67x00. Confirm it was executed or force a Dave + * retry */ Dave +if(((td-retry_cnt) TD_RETRYCNTMASK_ACT_FLG) (td-status == 0)) Dave + goto cont; Dave + Alignment seems off and you have trailing spaces on the last line. Also, indented code with spaces instead of tabs. 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 RFC] usb: dwc3: Get PHY from platform specific dwc3 dt node.
Hi, On Thu, Jan 31, 2013 at 9:08 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Jan 31, 2013 at 09:00:37PM +0530, Vivek Gautam wrote: Hi Felipe, On Thu, Jan 31, 2013 at 8:55 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, Jan 31, 2013 at 08:53:27PM +0530, Vivek Gautam wrote: Moreover, SoCs having multiple dwc3 controllers will have multiple PHYs, which eventually be added using usb_add_phy_dev(), and not using usb_add_phy(). So each dwc3 controller won't be able to get PHYs by simply calling devm_usb_get_phy() also. No. We have added usb_get_phy_dev() for that purpose in the case of non-dt. I think, instead you can have a patch to use devm_usb_get_phy_dev() here and in exynos platform specific code use usb_bind_phy() to bind the phy and controller till you change it to dt. We have dt support for dwc3-exynos, in such case we should go ahead with of_platform_populate(), right ? But if when i use of_platform_populate() i will not be able to set dma_mask to dwc3-dev. :-( do you have a special need for dma_mask because OF already sets it. If i am not wrong of_platform_device_create_pdata() will set dev-dev.coherent_dma_mask = DMA_BIT_MASK(32) and not dma_mask. I fact we had some discussion sometime back when we needed the same for dwc3-exynos in the thread: [PATCH v2 1/2] USB: dwc3-exynos: Add support for device tree But couldn't get final node on it. So suggestions here please. :-) hmm.. you're right there. Grant, Rob ? Any hints ? Any suggestions on this ? -- balbi -- 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] usb: host: tegra: Reset Tegra USB controller before init
On Thu, 28 Feb 2013, Venu Byravarasu wrote: To clear any configurations made by U-Boot on Tegra USB controller, reset it before init in probe. Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com --- When U-Boot configures a Tegra USB controller in device mode and if the EHCI driver of kernel tries to set it to HOST mode, message irq 52: nobody cared appears and IRQ gets disabled. This issue was initially reported with: http://marc.info/?l=linux-tegram=136110175423601w=2 To avoid such issues, due to configurations made by U-Boot driver, reset the Tegra USB controller, before configuring it by kernel. Does the Tegra platform use shared interrupts? If it does, what happens if the IRQ is enabled and in use by another device before ehci-tegra resets the USB controller? Does the unwanted interrupt occur only when the controller is switched to host mode? If not, it seems to me this reset belongs in the platform code, not in the glue layer. If yes, the reset belongs somewher before the controller is switched -- where does that occur? 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 4/5] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
On Thu, 28 Feb 2013, Lan Tianyu wrote:
Hi Alan:
Further thinking, the device should be disconnected since the port
can't be resumed and the device will not work normally. Something like
following. Does this make sense?
---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d5d3de4..cf36b11 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3170,6 +3170,7 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (status 0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+ hub_port_logical_disconnect(hub, port1);
return status;
}
}
I don't know. If you do this, will the port ever get powered on again?
This sort of thing is hard to test.
As far as the device is concerned, it won't make much difference.
Either way, the device won't work.
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 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
On Thu, 28 Feb 2013, Li Fei wrote:
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
+ port_dev-did_runtime_put = false;
}
I don't see much point in this. After a failed resume, the port's
runtime PM status is undefined. Whether or not you do a
pm_runtime_put_sync won't make any difference.
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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
Hi Sarah, Tony, On 2/27/2013 6:20 PM, Sarah Sharp wrote: On Wed, Feb 27, 2013 at 04:41:45PM -0500, Alan Stern wrote: Well, at this point, I have to say that Alex knows more about the quirk than I do. However, my gut feeling is that bus_suspend/resume is not the right place to stop the compliance timer. What I understood was that the Synopsis electrical part could cause USB 3.0 ports to go into Inactive mode with no port status change event generated, but only if all ports hadn't been into the U0 status once since boot/system resume. You are right, a port is subject to suffer the Compliance mode condition only if it hasn't previously entered U0. That is way the timer stops only if all ports have seen U0, otherwise it has to remain running. I don't know if all ports means all USB 3.0 ports or all ports on the host. I don't know if the ports can go into the Inactive mode when the port is suspended, like when all USB 3.0 ports are suspended in xhci_bus_suspend. This issue only hits to USB 3.0 ports since that part only touches USB3 lines. I would like be conservative here, and keep as much of the current code we have (with the compliance timer being manipulated in xhci_suspend and xhci_resume). That was the code that was submitted by TI, and we should stick as close to it as possible, without further information from Alex. Basically, I'd like Tony to make his first patch work, rather than pursuing moving the timer manipulation to xhci_bus_suspend/resume. 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] pci: do not try to assign irq 255
On 02/27/2013 10:13 PM, Bjorn Helgaas wrote:
[+cc Andy]
On Wed, Feb 20, 2013 at 11:53 PM, Hannes Reinecke h...@suse.de wrote:
On 02/20/2013 05:57 PM, Yinghai Lu wrote:
On Tue, Feb 19, 2013 at 11:58 PM, Hannes Reinecke h...@suse.de wrote:
Apparently this device is meant to use MSI _only_ so the BIOS developer
didn't feel the need to assign an INTx here.
According to PCI-3.0, section 6.8 (Message Signalled Interrupts):
It is recommended that devices implement interrupt pins to
provide compatibility in systems that do not support MSI
(devices default to interrupt pins). However, it is expected
that the need for interrupt pins will diminish over time.
Devices that do not support interrupt pins due to pin
constraints (rely on polling for device service) may implement
messages to increase performance without adding additional pins.
Therefore, system configuration software must not assume that a
message capable device has an interrupt pin.
Which sounds to me as if the implementation is valid...
it seems you mess pin with interrupt line.
current code:
unsigned char irq;
pci_read_config_byte(dev, PCI_INTERRUPT_PIN, irq);
dev-pin = irq;
if (irq)
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, irq);
dev-irq = irq;
so if the device does not have interrupt pin implemented, pin should be
zero.
and pin and irq in dev should
be all 0.
But the device _has_ an interrupt pin implemented.
The whole point here is that the interrupt line is _NOT_ zero.
00:14.0 USB controller [0c03]: Intel Corporation 7 Series/C210 Series
Chipset Family USB xHCI Host Controller [8086:1e31] (rev 04) (prog-if 30
[XHCI])
Subsystem: Hewlett-Packard Company Device [103c:179b]
Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort-
TAbort- MAbort- SERR- PERR- INTx-
Interrupt: pin A routed to IRQ 255
Region 0: Memory at d472 (64-bit, non-prefetchable) [size=64K]
Capabilities: [70] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA
PME(D0-,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [80] MSI: Enable- Count=1/8 Maskable- 64bit+
Address: Data:
So at one point we have to decide that -irq is not valid, despite it being
not set to zero.
An alternative fix would be this:
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 68a921d..4a480cb 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -469,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
} else {
dev_warn(dev-dev, PCI INT %c: no GSI\n,
pin_name(pin));
+ dev-irq = 0;
}
return 0;
}
Which probably is a better solution, as here -irq is _definitely_
not valid, so we should reset it to '0' to avoid confusion on upper
layers.
I didn't like the pci_read_irq() change because the PCI spec doesn't
say anything about any PCI_INTERRUPT_LINE values being invalid.
I like this solution better, but I still don't quite understand it.
We have the following code in acpi_pci_irq_enable(). We have
previously tried to look up gsi, but the _PRT doesn't mention this
device, so we have gsi == -1 at this point:
/*
* No IRQ known to the ACPI subsystem - maybe the BIOS /
* driver reported one, then use it. Exit in any case.
*/
if (gsi 0) {
u32 dev_gsi;
/* Interrupt Line values above 0xF are forbidden */
if (dev-irq 0 (dev-irq = 0xF)
(acpi_isa_irq_to_gsi(dev-irq, dev_gsi) == 0)) {
dev_warn(dev-dev, PCI INT %c: no GSI -
using ISA IRQ %d\n,
pin_name(pin), dev-irq);
acpi_register_gsi(dev-dev, dev_gsi,
ACPI_LEVEL_SENSITIVE,
ACPI_ACTIVE_LOW);
} else {
dev_warn(dev-dev, PCI INT %c: no GSI\n,
pin_name(pin));
}
return 0;
}
1) I don't know where the restriction of 0x1-0xF came from.
Presumably this value of dev-irq came from PCI_INTERRUPT_LINE, and I
don't know what forbids values 0xF. The test was added by Andy
Grover in the attached commit. This is ancient history; probably Andy
doesn't remember either :)
2) The proposed change (setting dev-irq = 0 when we didn't find
anything in the _PRT and dev-irq 0xF) throws away some information,
and I fear it could break systems. For example, what would happen if
a system put an IOAPIC pin number
Re: [PATCH v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/27/2013 07:20 PM, Sarah Sharp wrote:
Basically, I'd like Tony to make his first patch work, rather than
pursuing moving the timer manipulation to xhci_bus_suspend/resume.
Not to add confusion, but here is a less intrusive patch that simply
checks to see if the Compliance Mode Recovery Timer already exists
before attempting to initialize it.
---
drivers/usb/host/xhci.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f1f01a8..e51db78 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -445,6 +445,11 @@ static void compliance_mode_recovery(unsigned long arg)
*/
static void compliance_mode_recovery_timer_init(struct xhci_hcd *xhci)
{
+ if (timer_pending(xhci-comp_mode_recovery_timer)) {
+ xhci_dbg(xhci, Compliance Mode Recovery Timer already active.\n);
+ return;
+ }
+
xhci-port_status_u0 = 0;
init_timer(xhci-comp_mode_recovery_timer);
--
1.7.1
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
On 02/28/2013 08:09 AM, Alan Stern wrote: On Thu, 28 Feb 2013, Venu Byravarasu wrote: To clear any configurations made by U-Boot on Tegra USB controller, reset it before init in probe. Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com --- When U-Boot configures a Tegra USB controller in device mode and if the EHCI driver of kernel tries to set it to HOST mode, message irq 52: nobody cared appears and IRQ gets disabled. This issue was initially reported with: http://marc.info/?l=linux-tegram=136110175423601w=2 To avoid such issues, due to configurations made by U-Boot driver, reset the Tegra USB controller, before configuring it by kernel. Does the Tegra platform use shared interrupts? If it does, what happens if the IRQ is enabled and in use by another device before ehci-tegra resets the USB controller? I believe there's a dedicated interrupt just for each individual controller. -- 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: host: tegra: Reset Tegra USB controller before init
On 02/27/2013 11:36 PM, Venu Byravarasu wrote: To clear any configurations made by U-Boot on Tegra USB controller, reset it before init in probe. diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c @@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct platform_device *pdev) if (err) goto fail_clk; + tegra_periph_reset_assert(tegra-clk); + udelay(1); + tegra_periph_reset_deassert(tegra-clk); I think this patch might cause unintended consequences. When the Tegra PHY code is converted to a driver (i.e. has its own probe), the initial order of execution of the PHY and EHCI driver probes will not be guaranteed. In particular, since the EHCI probe will attempt to find the PHY device, and defer the EHCI probe until it can do so, this guarantees that the PHY's probe() will have completed before EHCI's probe() completes (although EHCI's probe may start running first some number of times, and be retried with -EPROBE_DEFERRED for a variety of reasons). Now, if the PHY driver's probe() actually touches HW and sets up some registers, isn't this reset call going to trash any of that register setup? Or, will PHY probe() not touch registers, but only do so during the standard PHY open/init op/API calls? I think the way to solve this is to put the reset call into the PHY driver. I assume it has access to the appropriate clock object. This may also address Alan's query re: when the unexpected interrupt occurs; it's triggered by (or correlated with at least) the PHY (or USB port in general) being in device mode due to the boot ROM setting it up this way, then switching to host mode via the Linux driver. I /think/ that device/host mode switching is more related to the PHY than EHCI driver, although I could well be wrong here. -- 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/3] usb: Make sure each c67x00 TD has been executed
From: Dave Tubbs dave.tu...@portalislc.com Make sure each c67x00 TD has been executed or retry using the existing retry mechanism. Reference Cypress Semiconductor BIOS User's Manual 1.2, page 3-16 Signed-off-by: Dave Tubbs dave.tu...@portalislc.com --- drivers/usb/c67x00/c67x00-sched.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c index aa49162..dd5bdb4 100644 --- a/drivers/usb/c67x00/c67x00-sched.c +++ b/drivers/usb/c67x00/c67x00-sched.c @@ -1033,6 +1033,12 @@ static inline void c67x00_check_td_list(struct c67x00_hcd *c67x00) !td_acked(td)) goto cont; + /* at this point, there are no errors, but it's still possible that the + * td wasn't executed by the c67x00. Confirm it was executed or force a + * retry */ + if(((td-retry_cnt) TD_RETRYCNTMASK_ACT_FLG) (td-status == 0)) + goto cont; + /* Sequence ok and acked, don't need to fix toggle */ ack_ok = 1; -- 1.7.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/3] usb: Make sure each c67x00 TD has been executed
From: Dave Tubbs dave.tu...@portalislc.com Make sure each c67x00 TD has been executed or retry using the existing retry mechanism. Reference Cypress Semiconductor BIOS User's Manual 1.2, page 3-16 Signed-off-by: Dave Tubbs dave.tu...@portalislc.com --- drivers/usb/c67x00/c67x00-sched.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c index aa49162..dd5bdb4 100644 --- a/drivers/usb/c67x00/c67x00-sched.c +++ b/drivers/usb/c67x00/c67x00-sched.c @@ -1033,6 +1033,12 @@ static inline void c67x00_check_td_list(struct c67x00_hcd *c67x00) !td_acked(td)) goto cont; + /* at this point, there are no errors, but it's still possible that the + * td wasn't executed by the c67x00. Confirm it was executed or force a + * retry */ + if(((td-retry_cnt) TD_RETRYCNTMASK_ACT_FLG) (td-status == 0)) + goto cont; + /* Sequence ok and acked, don't need to fix toggle */ ack_ok = 1; -- 1.7.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: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
On Thu, 28 Feb 2013, Stephen Warren wrote: I think this patch might cause unintended consequences. When the Tegra PHY code is converted to a driver (i.e. has its own probe), the initial order of execution of the PHY and EHCI driver probes will not be guaranteed. In particular, since the EHCI probe will attempt to find the PHY device, and defer the EHCI probe until it can do so, this guarantees that the PHY's probe() will have completed before EHCI's probe() completes (although EHCI's probe may start running first some number of times, and be retried with -EPROBE_DEFERRED for a variety of reasons). Now, if the PHY driver's probe() actually touches HW and sets up some registers, isn't this reset call going to trash any of that register setup? Or, will PHY probe() not touch registers, but only do so during the standard PHY open/init op/API calls? I think the way to solve this is to put the reset call into the PHY driver. I assume it has access to the appropriate clock object. This may also address Alan's query re: when the unexpected interrupt occurs; it's triggered by (or correlated with at least) the PHY (or USB port in general) being in device mode due to the boot ROM setting it up this way, then switching to host mode via the Linux driver. I /think/ that device/host mode switching is more related to the PHY than EHCI driver, although I could well be wrong here. With the PCI platform driver, the handoff from the firmware (we can categorize U-Boot as firmware for this discussion) is handled as soon as the controller is discovered by the platform-specific code. There's a special pci-quirks.c file to take care of it. It is not handled by the driver or the glue layer. In general I think that's what needs to be done. Errant interrupt sources should be disabled as quickly as possible. In this case I don't know exactly when the earliest opportunity is. I assume that the EHCI driver and/or the PHY driver gets probed because some platform-layer code has registered the device. If this is so then that platform-layer code is the right place to do the reset. 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: host: tegra: Reset Tegra USB controller before init
On 02/28/2013 11:52 AM, Alan Stern wrote: On Thu, 28 Feb 2013, Stephen Warren wrote: I think this patch might cause unintended consequences. When the Tegra PHY code is converted to a driver (i.e. has its own probe), the initial order of execution of the PHY and EHCI driver probes will not be guaranteed. In particular, since the EHCI probe will attempt to find the PHY device, and defer the EHCI probe until it can do so, this guarantees that the PHY's probe() will have completed before EHCI's probe() completes (although EHCI's probe may start running first some number of times, and be retried with -EPROBE_DEFERRED for a variety of reasons). Now, if the PHY driver's probe() actually touches HW and sets up some registers, isn't this reset call going to trash any of that register setup? Or, will PHY probe() not touch registers, but only do so during the standard PHY open/init op/API calls? I think the way to solve this is to put the reset call into the PHY driver. I assume it has access to the appropriate clock object. This may also address Alan's query re: when the unexpected interrupt occurs; it's triggered by (or correlated with at least) the PHY (or USB port in general) being in device mode due to the boot ROM setting it up this way, then switching to host mode via the Linux driver. I /think/ that device/host mode switching is more related to the PHY than EHCI driver, although I could well be wrong here. With the PCI platform driver, the handoff from the firmware (we can categorize U-Boot as firmware for this discussion) is handled as soon as the controller is discovered by the platform-specific code. There's a special pci-quirks.c file to take care of it. It is not handled by the driver or the glue layer. In general I think that's what needs to be done. Errant interrupt sources should be disabled as quickly as possible. In this case I don't know exactly when the earliest opportunity is. I assume that the EHCI driver and/or the PHY driver gets probed because some platform-layer code has registered the device. If this is so then that platform-layer code is the right place to do the reset. The first platform-specific code that is executed in this case is the (struct platform_driver) probe() functions for the PHY and/or EHCI device. On Tegra, all devices are listed in device tree, and core kernel code instantiates platform devices based on the information in device tree. The first non-core code that runs on a platform device is the driver's probe() method. That probe() method is for a very specific piece of HW, and hence seems to be the best place to put any quirks for it. Putting the quirks outside the driver would mean some piece of the core DT code would need to have a quirk list, and be able to map the registers for the device, acquire clocks, etc. etc., implement the quirk, and tear it all down again. All of which would just be duplicated with the driver's own probe() function. -- 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 3/3] usb: Make sure each c67x00 TD has been executed
Hello. On 02/28/2013 09:39 PM, Dave Tubbs wrote: From: Dave Tubbsdave.tu...@portalislc.com Make sure each c67x00 TD has been executed or retry using the existing retry mechanism. Reference Cypress Semiconductor BIOS User's Manual 1.2, page 3-16 Signed-off-by: Dave Tubbsdave.tu...@portalislc.com --- drivers/usb/c67x00/c67x00-sched.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c index aa49162..dd5bdb4 100644 --- a/drivers/usb/c67x00/c67x00-sched.c +++ b/drivers/usb/c67x00/c67x00-sched.c @@ -1033,6 +1033,12 @@ static inline void c67x00_check_td_list(struct c67x00_hcd *c67x00) !td_acked(td)) goto cont; + /* at this point, there are no errors, but it's still possible that the + * td wasn't executed by the c67x00. Confirm it was executed or force a + * retry */ Read Documentation/CodingStyle chapter 8 on the preferred multi-line comment style. + if(((td-retry_cnt) TD_RETRYCNTMASK_ACT_FLG) (td-status == 0)) Space must follow *if* (always run your patches thru scripts/checkpatch.pl). And you don't need () around td-retry_cnt; neither around the last ==. 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: Problem with linux-3.7.7 (Stern - d01875f11f05f76fc471cec94d701201c1b34389)
On Thu, 28 Feb 2013, Adrian Bassett wrote: First, things will be a lot easier if you do your testing with a 3.8 kernel rather than 3.7.7. Ok. using 3.8 checked out from the linux-stable git tree from kernel.org. I built three test kernels - 1/ vanilla, 2/ 640c3339 reverted, 3/ no revert but incorporating your patch. And presumably the first failed, the second worked, and the third failed. Second, just to be certain we are on the same page, is it correct that you find suspend works okay when you revert commit 6e0c3339a6f19d7 but it fails if the commit is in place? That's correct. Excepting I think that with 3.8 (possibly with 3.7.7 also but I haven't re-run the tests) it's with resume rather than suspend where the problem lies. What I've found is that a number of suspend/resume cycles (in close succession) will work OK but then resume will delay for several minutes before returning the system. Network is usually a problem if resume attempts follow suspend too soon. External USB mass storage has not always been re-available, either. Obviously this is because your USB mass storage and wireless devices are attached to the EHCI controller. Third: Does applying the patch below help at all? It didn't appear to, in my tests. Fourth: If not, please build a kernel with CONFIG_USB_DEBUG enabled and post the output from dmesg following a failed suspend attempt. Following output is from /var/log/messages, filtered on kernel: messages and with cfg80211 llines removed for brevity (well, it's not brief but perhaps it will be useful). This is all rather odd. Evidently the EHCI controller stopped working for a while and then started working again. Also, I don't know what this and the related warnings are all about: Feb 28 14:56:14 mypc kernel: [ cut here ] Feb 28 14:56:14 mypc kernel: WARNING: at drivers/net/wireless/ath/ath9k/hw.c:2223 ath9k_hw_setpower+0x355/0x590 [ath9k_hw]() Feb 28 14:56:14 mypc kernel: Hardware name: G41MT-S2P Feb 28 14:56:14 mypc kernel: Modules linked in: it87 hwmon_vid tcp_westwood i915 ath9k_htc ath9k_common ath9k_hw pwc drm_kms_helper videobuf2_vmalloc videobuf2_memops drm videobuf2_core cpufreq_ondemand rtl8180 ath eeprom_93cx6 coretemp uhci_hcd ehci_pci ehci_hcd Feb 28 14:56:14 mypc kernel: Pid: 14318, comm: s2ram Not tainted 3.8.0-patched-2-g10460f0-dirty #1 Feb 28 14:56:14 mypc kernel: Call Trace: Feb 28 14:56:14 mypc kernel: [810439da] warn_slowpath_common+0x7a/0xb0 Feb 28 14:56:14 mypc kernel: [81043a25] warn_slowpath_null+0x15/0x20 Feb 28 14:56:14 mypc kernel: [a00cd465] ath9k_hw_setpower+0x355/0x590 [ath9k_hw] Feb 28 14:56:14 mypc kernel: [a0144106] ath9k_htc_setpower+0x36/0x60 [ath9k_htc] Feb 28 14:56:14 mypc kernel: [a0144d76] ath9k_htc_start+0x56/0x260 [ath9k_htc] Feb 28 14:56:14 mypc kernel: [8182c7a1] ieee80211_reconfig+0x1b1/0x2360 Feb 28 14:56:14 mypc kernel: [81876ef1] ? _raw_spin_lock_bh+0x11/0x40 Feb 28 14:56:14 mypc kernel: [817b9639] ? wiphy_resume+0x59/0x1c0 Feb 28 14:56:14 mypc kernel: [8181d3c8] ieee80211_resume+0x28/0x70 Feb 28 14:56:14 mypc kernel: [817b969a] wiphy_resume+0xba/0x1c0 Feb 28 14:56:14 mypc kernel: [817b95e0] ? index_show+0x30/0x30 Feb 28 14:56:14 mypc kernel: [8142c106] dpm_run_callback.isra.5+0x36/0x70 Feb 28 14:56:14 mypc kernel: [8142c41f] device_resume+0x9f/0x140 Feb 28 14:56:14 mypc kernel: [8142d34c] dpm_resume+0xfc/0x230 Feb 28 14:56:14 mypc kernel: [8142d650] dpm_resume_end+0x10/0x20 Feb 28 14:56:14 mypc kernel: [813bc8d1] ? acpi_suspend_begin_old+0x28/0x28 Feb 28 14:56:14 mypc kernel: [8108c12d] suspend_devices_and_enter+0x13d/0x370 Feb 28 14:56:14 mypc kernel: [8108c4eb] pm_suspend+0x18b/0x1f0 Feb 28 14:56:14 mypc kernel: [8108b4e9] state_store+0x89/0xf0 Feb 28 14:56:14 mypc kernel: [8136898f] kobj_attr_store+0xf/0x30 Feb 28 14:56:14 mypc kernel: [811b74ed] sysfs_write_file+0xdd/0x160 Feb 28 14:56:14 mypc kernel: [8114b3fa] vfs_write+0xaa/0x180 Feb 28 14:56:14 mypc kernel: [8114b72d] sys_write+0x4d/0x90 Feb 28 14:56:14 mypc kernel: [8187e152] system_call_fastpath+0x16/0x1b Feb 28 14:56:14 mypc kernel: ---[ end trace fd27824e37f48d17 ]--- Feb 28 14:56:14 mypc kernel: ath: phy2: timeout (10 us) on reg 0x7000: 0xfffb 0x0003 != 0x Feb 28 14:56:14 mypc kernel: ath: phy2: Chip reset failed Feb 28 14:56:14 mypc kernel: ath: phy2: Unable to reset hardware; reset status -22 (freq 2462 MHz) They look like a bug in the ath9k driver. Anyway, the problem appears to be that the EHCI controller isn't issuing IRQs as it should. Does the patch below make any difference? It turns on the I/O watchdog timer for async transfers on Intel hardware. Finally, I don't see how this could possibly
[PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
After PCI and USB have stopped using the .find_bridge() callback in
struct acpi_bus_type, the only remaining user of it is SATA, but SATA
only pretends to be a user, because it points that callback to a stub
always returning -ENODEV.
For this reason, drop the SATA's dummy .find_bridge() callback and
remove .find_bridge(), which is not used any more, from struct
acpi_bus_type entirely.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/glue.c | 26 +-
drivers/ata/libata-acpi.c |6 --
include/acpi/acpi_bus.h |3 ---
3 files changed, 1 insertion(+), 34 deletions(-)
Index: linux-pm/drivers/ata/libata-acpi.c
===
--- linux-pm.orig/drivers/ata/libata-acpi.c
+++ linux-pm/drivers/ata/libata-acpi.c
@@ -1144,14 +1144,8 @@ static int ata_acpi_find_device(struct d
return -ENODEV;
}
-static int ata_acpi_find_dummy(struct device *dev, acpi_handle *handle)
-{
- return -ENODEV;
-}
-
static struct acpi_bus_type ata_acpi_bus = {
.name = ATA,
- .find_bridge = ata_acpi_find_dummy,
.find_device = ata_acpi_find_device,
};
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -439,10 +439,7 @@ struct acpi_bus_type {
struct list_head list;
const char *name;
bool (*match)(struct device *dev);
- /* For general devices under the bus */
int (*find_device) (struct device *, acpi_handle *);
- /* For bridges, such as PCI root bridge, IDE controller */
- int (*find_bridge) (struct device *, acpi_handle *);
void (*setup)(struct device *);
void (*cleanup)(struct device *);
};
Index: linux-pm/drivers/acpi/glue.c
===
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -78,22 +78,6 @@ static struct acpi_bus_type *acpi_get_bu
return ret;
}
-static int acpi_find_bridge_device(struct device *dev, acpi_handle * handle)
-{
- struct acpi_bus_type *tmp;
- int ret = -ENODEV;
-
- down_read(bus_type_sem);
- list_for_each_entry(tmp, bus_type_list, list) {
- if (tmp-find_bridge !tmp-find_bridge(dev, handle)) {
- ret = 0;
- break;
- }
- }
- up_read(bus_type_sem);
- return ret;
-}
-
static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
void *addr_p, void **ret_p)
{
@@ -262,15 +246,7 @@ static int acpi_platform_notify(struct d
int ret;
ret = acpi_bind_one(dev, NULL);
- if (ret) {
- if (!type) {
- ret = acpi_find_bridge_device(dev, handle);
- if (!ret)
- ret = acpi_bind_one(dev, handle);
-
- goto out;
- }
-
+ if (ret type) {
ret = type-find_device(dev, handle);
if (ret) {
DBG(Unable to get handle for %s\n, dev_name(dev));
--
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/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
USB uses the .find_bridge() callback from struct acpi_bus_type
incorrectly, because as a result of the way it is used by USB every
device in the system that doesn't have a bus type or parent is
passed to usb_acpi_find_device() for inspection.
What USB actually needs, though, is to call usb_acpi_find_device()
for USB ports that don't have a bus type defined, but have
usb_port_device_type as their device type, as well as for USB
devices.
To fix that replace the struct bus_type pointer in struct
acpi_bus_type used for matching devices to specific subsystems
with a .match() callback to be used for this purpose and update
the users of struct acpi_bus_type, including USB, accordingly.
Define the .match() callback routine for USB, usb_acpi_bus_match(),
in such a way that it will cover both USB devices and USB ports
and remove the now redundant .find_bridge() callback pointer from
usb_acpi_bus.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/glue.c | 39 +--
drivers/ata/libata-acpi.c |1 +
drivers/pci/pci-acpi.c |8 +++-
drivers/pnp/pnpacpi/core.c |8 +++-
drivers/scsi/scsi_lib.c |7 ++-
drivers/usb/core/usb-acpi.c |9 +++--
include/acpi/acpi_bus.h |3 ++-
7 files changed, 43 insertions(+), 32 deletions(-)
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -437,7 +437,8 @@ void acpi_remove_dir(struct acpi_device
*/
struct acpi_bus_type {
struct list_head list;
- struct bus_type *bus;
+ const char *name;
+ bool (*match)(struct device *dev);
/* For general devices under the bus */
int (*find_device) (struct device *, acpi_handle *);
/* For bridges, such as PCI root bridge, IDE controller */
Index: linux-pm/drivers/acpi/glue.c
===
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -36,12 +36,11 @@ int register_acpi_bus_type(struct acpi_b
{
if (acpi_disabled)
return -ENODEV;
- if (type type-bus type-find_device) {
+ if (type type-match type-find_device) {
down_write(bus_type_sem);
list_add_tail(type-list, bus_type_list);
up_write(bus_type_sem);
- printk(KERN_INFO PREFIX bus type %s registered\n,
- type-bus-name);
+ printk(KERN_INFO PREFIX bus type %s registered\n, type-name);
return 0;
}
return -ENODEV;
@@ -56,24 +55,21 @@ int unregister_acpi_bus_type(struct acpi
down_write(bus_type_sem);
list_del_init(type-list);
up_write(bus_type_sem);
- printk(KERN_INFO PREFIX ACPI bus type %s unregistered\n,
- type-bus-name);
+ printk(KERN_INFO PREFIX bus type %s unregistered\n,
+ type-name);
return 0;
}
return -ENODEV;
}
EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);
-static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type)
+static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
{
struct acpi_bus_type *tmp, *ret = NULL;
- if (!type)
- return NULL;
-
down_read(bus_type_sem);
list_for_each_entry(tmp, bus_type_list, list) {
- if (tmp-bus == type) {
+ if (tmp-match(dev)) {
ret = tmp;
break;
}
@@ -261,26 +257,17 @@ err:
static int acpi_platform_notify(struct device *dev)
{
- struct acpi_bus_type *type;
+ struct acpi_bus_type *type = acpi_get_bus_type(dev);
acpi_handle handle;
int ret;
ret = acpi_bind_one(dev, NULL);
- if (ret (!dev-bus || !dev-parent)) {
- /* bridge devices genernally haven't bus or parent */
- ret = acpi_find_bridge_device(dev, handle);
- if (!ret) {
- ret = acpi_bind_one(dev, handle);
- if (ret)
- goto out;
- }
- }
-
- type = acpi_get_bus_type(dev-bus);
if (ret) {
- if (!type || !type-find_device) {
- DBG(No ACPI bus support for %s\n, dev_name(dev));
- ret = -EINVAL;
+ if (!type) {
+ ret = acpi_find_bridge_device(dev, handle);
+ if (!ret)
+ ret = acpi_bind_one(dev, handle);
+
goto out;
}
@@ -316,7 +303,7 @@ static int acpi_platform_notify_remove(s
{
struct acpi_bus_type *type;
- type
Re: [Resend][PATCH] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
On Wednesday, February 27, 2013 06:33:13 PM Greg Kroah-Hartman wrote: On Thu, Feb 28, 2013 at 02:11:58AM +0100, Rafael J. Wysocki wrote: On Wednesday, February 27, 2013 02:20:32 PM Greg Kroah-Hartman wrote: On Wed, Feb 27, 2013 at 11:06:52PM +0100, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com After PCI has stopped using the .find_bridge() callback in struct acpi_bus_type, the only remaining users of it are SATA and USB. However, SATA only pretends to be a user, because it points that callback to a stub always returning -ENODEV, and USB uses it incorrectly, because as a result of the way it is used by USB every device in the system that doesn't have a bus type or parent is passed to usb_acpi_find_device() for inspection. What USB actually needs, though, is to call usb_acpi_find_device() for USB ports that don't have a bus type defined, but have usb_port_device_type as their device type. Ick, that's not good. Can you have the original creator of that code (someone else from Intel, I can't remember at the moment), fix that up properly and send me patches? That won't be necessary afer this patch. Or do you want to fix up USB separately first? No, sorry, my misunderstanding, I was assuming we still needed to do other USB work after this. If not, that's an even better reason to accept this patch :) Heh, thanks! Still, it seems that we can do a bit better that this. The two patches that will follow should be almost functionally equivalent to the $subject one, but the resulting code looks somewhat cleaner to me. [1/2] Add .macth() callback to struct acpi_bus_type (instead of the bus pointer). [2/2] Drop .find_bridge() from struct acpi_bus_type Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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 2/2] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
On 02/28/2013 04:53 PM, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com After PCI and USB have stopped using the .find_bridge() callback in struct acpi_bus_type, the only remaining user of it is SATA, but SATA only pretends to be a user, because it points that callback to a stub always returning -ENODEV. For this reason, drop the SATA's dummy .find_bridge() callback and remove .find_bridge(), which is not used any more, from struct acpi_bus_type entirely. Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com --- drivers/acpi/glue.c | 26 +- drivers/ata/libata-acpi.c |6 -- include/acpi/acpi_bus.h |3 --- 3 files changed, 1 insertion(+), 34 deletions(-) patches 1-2 Acked-by: Jeff Garzik jgar...@pobox.com -- 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] ACPI / glue: Add .match() callback to struct acpi_bus_type
On Thu, Feb 28, 2013 at 1:53 PM, Rafael J. Wysocki r...@sisk.pl wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
USB uses the .find_bridge() callback from struct acpi_bus_type
incorrectly, because as a result of the way it is used by USB every
device in the system that doesn't have a bus type or parent is
passed to usb_acpi_find_device() for inspection.
What USB actually needs, though, is to call usb_acpi_find_device()
for USB ports that don't have a bus type defined, but have
usb_port_device_type as their device type, as well as for USB
devices.
To fix that replace the struct bus_type pointer in struct
acpi_bus_type used for matching devices to specific subsystems
with a .match() callback to be used for this purpose and update
the users of struct acpi_bus_type, including USB, accordingly.
Define the .match() callback routine for USB, usb_acpi_bus_match(),
in such a way that it will cover both USB devices and USB ports
and remove the now redundant .find_bridge() callback pointer from
usb_acpi_bus.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/glue.c | 39 +--
drivers/ata/libata-acpi.c |1 +
drivers/pci/pci-acpi.c |8 +++-
drivers/pnp/pnpacpi/core.c |8 +++-
drivers/scsi/scsi_lib.c |7 ++-
drivers/usb/core/usb-acpi.c |9 +++--
include/acpi/acpi_bus.h |3 ++-
7 files changed, 43 insertions(+), 32 deletions(-)
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -437,7 +437,8 @@ void acpi_remove_dir(struct acpi_device
*/
struct acpi_bus_type {
struct list_head list;
- struct bus_type *bus;
+ const char *name;
+ bool (*match)(struct device *dev);
/* For general devices under the bus */
int (*find_device) (struct device *, acpi_handle *);
/* For bridges, such as PCI root bridge, IDE controller */
Index: linux-pm/drivers/acpi/glue.c
===
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -36,12 +36,11 @@ int register_acpi_bus_type(struct acpi_b
{
if (acpi_disabled)
return -ENODEV;
- if (type type-bus type-find_device) {
+ if (type type-match type-find_device) {
down_write(bus_type_sem);
list_add_tail(type-list, bus_type_list);
up_write(bus_type_sem);
- printk(KERN_INFO PREFIX bus type %s registered\n,
- type-bus-name);
+ printk(KERN_INFO PREFIX bus type %s registered\n,
type-name);
return 0;
}
return -ENODEV;
@@ -56,24 +55,21 @@ int unregister_acpi_bus_type(struct acpi
down_write(bus_type_sem);
list_del_init(type-list);
up_write(bus_type_sem);
- printk(KERN_INFO PREFIX ACPI bus type %s unregistered\n,
- type-bus-name);
+ printk(KERN_INFO PREFIX bus type %s unregistered\n,
+ type-name);
return 0;
}
return -ENODEV;
}
EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);
-static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type)
+static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
{
struct acpi_bus_type *tmp, *ret = NULL;
- if (!type)
- return NULL;
-
down_read(bus_type_sem);
list_for_each_entry(tmp, bus_type_list, list) {
- if (tmp-bus == type) {
+ if (tmp-match(dev)) {
ret = tmp;
break;
}
@@ -261,26 +257,17 @@ err:
static int acpi_platform_notify(struct device *dev)
{
- struct acpi_bus_type *type;
+ struct acpi_bus_type *type = acpi_get_bus_type(dev);
acpi_handle handle;
int ret;
ret = acpi_bind_one(dev, NULL);
- if (ret (!dev-bus || !dev-parent)) {
- /* bridge devices genernally haven't bus or parent */
- ret = acpi_find_bridge_device(dev, handle);
- if (!ret) {
- ret = acpi_bind_one(dev, handle);
- if (ret)
- goto out;
- }
- }
-
- type = acpi_get_bus_type(dev-bus);
if (ret) {
- if (!type || !type-find_device) {
- DBG(No ACPI bus support for %s\n, dev_name(dev));
- ret = -EINVAL;
+ if (!type) {
+ ret = acpi_find_bridge_device(dev, handle);
+ if (!ret)
+ ret =
Re: [PATCH 1/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
On Thursday, February 28, 2013 02:29:56 PM Yinghai Lu wrote:
On Thu, Feb 28, 2013 at 1:53 PM, Rafael J. Wysocki r...@sisk.pl wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
USB uses the .find_bridge() callback from struct acpi_bus_type
incorrectly, because as a result of the way it is used by USB every
device in the system that doesn't have a bus type or parent is
passed to usb_acpi_find_device() for inspection.
What USB actually needs, though, is to call usb_acpi_find_device()
for USB ports that don't have a bus type defined, but have
usb_port_device_type as their device type, as well as for USB
devices.
To fix that replace the struct bus_type pointer in struct
acpi_bus_type used for matching devices to specific subsystems
with a .match() callback to be used for this purpose and update
the users of struct acpi_bus_type, including USB, accordingly.
Define the .match() callback routine for USB, usb_acpi_bus_match(),
in such a way that it will cover both USB devices and USB ports
and remove the now redundant .find_bridge() callback pointer from
usb_acpi_bus.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/glue.c | 39 +--
drivers/ata/libata-acpi.c |1 +
drivers/pci/pci-acpi.c |8 +++-
drivers/pnp/pnpacpi/core.c |8 +++-
drivers/scsi/scsi_lib.c |7 ++-
drivers/usb/core/usb-acpi.c |9 +++--
include/acpi/acpi_bus.h |3 ++-
7 files changed, 43 insertions(+), 32 deletions(-)
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -437,7 +437,8 @@ void acpi_remove_dir(struct acpi_device
*/
struct acpi_bus_type {
struct list_head list;
- struct bus_type *bus;
+ const char *name;
+ bool (*match)(struct device *dev);
/* For general devices under the bus */
int (*find_device) (struct device *, acpi_handle *);
/* For bridges, such as PCI root bridge, IDE controller */
Index: linux-pm/drivers/acpi/glue.c
===
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -36,12 +36,11 @@ int register_acpi_bus_type(struct acpi_b
{
if (acpi_disabled)
return -ENODEV;
- if (type type-bus type-find_device) {
+ if (type type-match type-find_device) {
down_write(bus_type_sem);
list_add_tail(type-list, bus_type_list);
up_write(bus_type_sem);
- printk(KERN_INFO PREFIX bus type %s registered\n,
- type-bus-name);
+ printk(KERN_INFO PREFIX bus type %s registered\n,
type-name);
return 0;
}
return -ENODEV;
@@ -56,24 +55,21 @@ int unregister_acpi_bus_type(struct acpi
down_write(bus_type_sem);
list_del_init(type-list);
up_write(bus_type_sem);
- printk(KERN_INFO PREFIX ACPI bus type %s unregistered\n,
- type-bus-name);
+ printk(KERN_INFO PREFIX bus type %s unregistered\n,
+ type-name);
return 0;
}
return -ENODEV;
}
EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);
-static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type)
+static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
{
struct acpi_bus_type *tmp, *ret = NULL;
- if (!type)
- return NULL;
-
down_read(bus_type_sem);
list_for_each_entry(tmp, bus_type_list, list) {
- if (tmp-bus == type) {
+ if (tmp-match(dev)) {
ret = tmp;
break;
}
@@ -261,26 +257,17 @@ err:
static int acpi_platform_notify(struct device *dev)
{
- struct acpi_bus_type *type;
+ struct acpi_bus_type *type = acpi_get_bus_type(dev);
acpi_handle handle;
int ret;
ret = acpi_bind_one(dev, NULL);
- if (ret (!dev-bus || !dev-parent)) {
- /* bridge devices genernally haven't bus or parent */
- ret = acpi_find_bridge_device(dev, handle);
- if (!ret) {
- ret = acpi_bind_one(dev, handle);
- if (ret)
- goto out;
- }
- }
-
- type = acpi_get_bus_type(dev-bus);
if (ret) {
- if (!type || !type-find_device) {
- DBG(No ACPI bus support for %s\n, dev_name(dev));
- ret = -EINVAL;
+
Re: [PATCH 2/2] ACPI / glue: Drop .find_bridge() callback from struct acpi_bus_type
On Thursday, February 28, 2013 05:13:27 PM Jeff Garzik wrote: On 02/28/2013 04:53 PM, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com After PCI and USB have stopped using the .find_bridge() callback in struct acpi_bus_type, the only remaining user of it is SATA, but SATA only pretends to be a user, because it points that callback to a stub always returning -ENODEV. For this reason, drop the SATA's dummy .find_bridge() callback and remove .find_bridge(), which is not used any more, from struct acpi_bus_type entirely. Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com --- drivers/acpi/glue.c | 26 +- drivers/ata/libata-acpi.c |6 -- include/acpi/acpi_bus.h |3 --- 3 files changed, 1 insertion(+), 34 deletions(-) patches 1-2 Acked-by: Jeff Garzik jgar...@pobox.com Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
On Friday, March 01, 2013 12:38:07 AM Liu, Chuansheng wrote:
-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu]
Sent: Thursday, February 28, 2013 11:17 PM
To: Li, Fei
Cc: gre...@linuxfoundation.org; Lan, Tianyu; sarah.a.sh...@linux.intel.com;
r...@sisk.pl; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Liu,
Chuansheng
Subject: Re: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On Thu, 28 Feb 2013, Li Fei wrote:
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
+ port_dev-did_runtime_put = false;
}
I don't see much point in this. After a failed resume, the port's
runtime PM status is undefined. Whether or not you do a
pm_runtime_put_sync won't make any difference.
In case of failed resume, calling pm_runtime_put_sync() is just for decrease
the dev-power.usage_count,
because pm_runtime_get_sync() always increase the dev-power.usage_count even
failed.
If not pairing runtime_get/put, after that case, the device can not enter
runtime suspend any more due to dev-power.usage_count 0 always.
Is it making sense?
Well, not really.
Before returning an error code, rpm_callback() assigns that code to
dev-power.runtime_error and that will effectively disable runtime PM for dev
going forward anyway.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
-Original Message-
From: Rafael J. Wysocki [mailto:r...@sisk.pl]
Sent: Friday, March 01, 2013 8:51 AM
To: Liu, Chuansheng
Cc: Alan Stern; Li, Fei; gre...@linuxfoundation.org; Lan, Tianyu;
sarah.a.sh...@linux.intel.com; linux-usb@vger.kernel.org;
linux-ker...@vger.kernel.org
Subject: Re: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On Friday, March 01, 2013 12:38:07 AM Liu, Chuansheng wrote:
-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu]
Sent: Thursday, February 28, 2013 11:17 PM
To: Li, Fei
Cc: gre...@linuxfoundation.org; Lan, Tianyu;
sarah.a.sh...@linux.intel.com;
r...@sisk.pl; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
Liu,
Chuansheng
Subject: Re: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On Thu, 28 Feb 2013, Li Fei wrote:
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device
*udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
+ port_dev-did_runtime_put = false;
}
I don't see much point in this. After a failed resume, the port's
runtime PM status is undefined. Whether or not you do a
pm_runtime_put_sync won't make any difference.
In case of failed resume, calling pm_runtime_put_sync() is just for decrease
the dev-power.usage_count,
because pm_runtime_get_sync() always increase the
dev-power.usage_count even failed.
If not pairing runtime_get/put, after that case, the device can not enter
runtime suspend any more due to dev-power.usage_count 0 always.
Is it making sense?
Well, not really.
Before returning an error code, rpm_callback() assigns that code to
dev-power.runtime_error and that will effectively disable runtime PM for dev
going forward anyway.
Thanks your pointing out.
dev-power.runtime_error!=0 will really block the runtime PM resume/suspend to
continue.
But in case of rpm_resume return error when dev-power.disable_depth 0, the
dev-power.runtime_error
is not set yet. Is it the case? Thanks your comments again.
And another case is when user called pm_runtime_set_status to clear the
runtime_error after dev-power.runtime_error
is set during pm_runtime_get_sync(), the runtime_resume/suspend() can be tried
again? But the dev-power.usage_count is still wrong?
Thanks your correction again:)
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
Re: About chipidea tree
On Thu, Feb 28, 2013 at 01:16:04PM +0200, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: On Tue, Feb 26, 2013 at 02:47:34PM +0100, Marc Kleine-Budde wrote: On 02/26/2013 02:25 PM, Alexander Shishkin wrote: Marc Kleine-Budde m...@pengutronix.de writes: On 02/26/2013 11:56 AM, Alexander Shishkin wrote: Peter Chen peter.c...@freescale.com writes: Hi Alex, Hi, Do we have a chipidea repo which is queued for mainline? We have several patchsets for chipidea these monthes, I don't know their status. For me, I would like based on your tree if it exists. I thought about it, but then it seems like having a separate branch is bound to be confusing to most people. I'd much rather prefer everything go to usb-next, and this is my current plan. Since Greg will start applying new stuff to usb-next after -rc1 is tagged, I'll send my current stash of patches for inclusion then. If your patchset, for Can we have a look at your queued patches? Sure, http://marc.info/?l=linux-usbm=135902434508839 example, has conflicts with my stuff that's not merged, I'll try to take care of resolving the conflicts and submit everything to Greg. In other words, it should be always ok to base your chipidea patchsets on usb-next. Let me know if this sounds reasonable to you. Michael has already done that work (some S-o-b form Michael are missing) and rebased Sascha's and Peter's patches to current linus/master, see this tree : http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10 Taking a quick look, quite some of those patches are not ready for inclusion yet. So if the question is, do we need a -next tree for all the chipidea patchsets floating around, it might be a good thing. But it's not what Peter was asking in the first place. I suggest that we have a branch that holds all chipidea patches that are ready for mainline. Otherwise it's really hard to code any new features I agree. Alex, you can have a repo at github or any other places which is based on usb-next, and add it to MAINTAINERS. We can develop the new feature based on your repo. Greg can pull it directly if he agrees or you can send your queued patchset before every merge windows. Ok, let's try this. I have a linux-ci repo on github, might as well do something useful with it [1], [2]. Currently, the branch called ci-for-greg is where I stack patches that I'm planning to be sending (via email) to Greg when the time is right. The policy is such that it'll be rebased on top of Greg's usb-next and probably often, so no fast forwards. Also patches may be dropped from it if necessary. Since the branch is not for pulling, the no rebase rule doesn't apply. If you have comments/suggestions/etc for a patch that is in this branch, please reply to the email with that patch on the mailing list and not via github infrastructure. Sounds reasonable? Agree I'm now scouting my inbox for more candidates for this branch. Please don't re-send the patches that have already been sent, unless you have new versions of those, I have them all. Do send new versions, though. [1] git://github.com/virtuoso/linux-ci.git ci-for-greg [2] https://github.com/virtuoso/linux-ci/commits/ci-for-greg Regards, -- Alex -- 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 v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
On Thu, Feb 28, 2013 at 12:45:59PM +0200, Felipe Balbi wrote: Hi, On Thu, Feb 28, 2013 at 06:06:55PM +0800, Peter Chen wrote: For runtime pm disabled, we may had to add some ugly things, like notify core probe fail, and platform layer needs to handle this failed notify. Please stop talking about about notify core probe fail that will never happen. Not today, not ever. Forget that idea. Of course, it is ugly way, I just thought if it will affect power, eg dvfs, bus freq if the usb clock is on. Maybe the user will find USB core init fails case causes runtime power rise, and fix the core init fail bug. -- 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 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
On Friday, March 01, 2013 12:59:23 AM Liu, Chuansheng wrote:
-Original Message-
From: Rafael J. Wysocki [mailto:r...@sisk.pl]
Sent: Friday, March 01, 2013 8:51 AM
To: Liu, Chuansheng
Cc: Alan Stern; Li, Fei; gre...@linuxfoundation.org; Lan, Tianyu;
sarah.a.sh...@linux.intel.com; linux-usb@vger.kernel.org;
linux-ker...@vger.kernel.org
Subject: Re: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On Friday, March 01, 2013 12:38:07 AM Liu, Chuansheng wrote:
-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu]
Sent: Thursday, February 28, 2013 11:17 PM
To: Li, Fei
Cc: gre...@linuxfoundation.org; Lan, Tianyu;
sarah.a.sh...@linux.intel.com;
r...@sisk.pl; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
Liu,
Chuansheng
Subject: Re: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On Thu, 28 Feb 2013, Li Fei wrote:
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device
*udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
return status;
}
+ port_dev-did_runtime_put = false;
}
I don't see much point in this. After a failed resume, the port's
runtime PM status is undefined. Whether or not you do a
pm_runtime_put_sync won't make any difference.
In case of failed resume, calling pm_runtime_put_sync() is just for
decrease
the dev-power.usage_count,
because pm_runtime_get_sync() always increase the
dev-power.usage_count even failed.
If not pairing runtime_get/put, after that case, the device can not enter
runtime suspend any more due to dev-power.usage_count 0 always.
Is it making sense?
Well, not really.
Before returning an error code, rpm_callback() assigns that code to
dev-power.runtime_error and that will effectively disable runtime PM for
dev
going forward anyway.
Thanks your pointing out.
dev-power.runtime_error!=0 will really block the runtime PM resume/suspend
to continue.
But in case of rpm_resume return error when dev-power.disable_depth 0, the
dev-power.runtime_error
is not set yet. Is it the case?
Yes, it is.
And another case is when user called pm_runtime_set_status to clear the
runtime_error after dev-power.runtime_error
is set during pm_runtime_get_sync(), the runtime_resume/suspend() can be
tried again? But the dev-power.usage_count is still wrong?
If you clear runtime_error using pm_runtime_set_status(), you can correct the
reference counter as well.
But I agree that with runtime PM disabled it is actually useful to keep the
reference counter balanced appropriately so that you don't need to special
case that. All depends on how runtime PM is used in the given piece of code.
That's why I didn't comment your other patches. That said, I didn't look at
them in detail either.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
On Friday, March 01, 2013 02:07:54 AM Liu, Chuansheng wrote:
-Original Message-
From: Li, Fei
Sent: Thursday, February 28, 2013 5:06 PM
To: gre...@linuxfoundation.org; Lan, Tianyu; st...@rowland.harvard.edu;
sarah.a.sh...@linux.intel.com
Cc: r...@sisk.pl; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
Liu,
Chuansheng; Li, Fei
Subject: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
Rechecked the usb similar codes, in usb_autoresume_device() and
usb_autopm_get_interface(),
when pm_runtime_get_sync() failed, the paired pm_runtime_put_sync() will be
called.
Alan and Rafael, is it reasonable to consider this cleanup patch also? Thanks.
You can very well use pm_runtime_put_noidle() here too. Then, it will
be kind of clear what it's for.
return status;
}
+ port_dev-did_runtime_put = false;
}
/* Skip the initial Clear-Suspend step for a remote wakeup */
--
1.7.4.1
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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 4/5 V2] usb: call pm_runtime_put_sync in pm_runtime_get_sync failed case
-Original Message-
From: Rafael J. Wysocki [mailto:r...@sisk.pl]
Sent: Friday, March 01, 2013 10:22 AM
To: Liu, Chuansheng
Cc: Li, Fei; gre...@linuxfoundation.org; Lan, Tianyu;
st...@rowland.harvard.edu; sarah.a.sh...@linux.intel.com;
linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
On Friday, March 01, 2013 02:07:54 AM Liu, Chuansheng wrote:
-Original Message-
From: Li, Fei
Sent: Thursday, February 28, 2013 5:06 PM
To: gre...@linuxfoundation.org; Lan, Tianyu; st...@rowland.harvard.edu;
sarah.a.sh...@linux.intel.com
Cc: r...@sisk.pl; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org;
Liu,
Chuansheng; Li, Fei
Subject: [PATCH 4/5 V2] usb: call pm_runtime_put_sync in
pm_runtime_get_sync failed case
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..f72dede 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device
*udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port,
status %d\n,
status);
+ pm_runtime_put_sync(port_dev-dev);
Rechecked the usb similar codes, in usb_autoresume_device() and
usb_autopm_get_interface(),
when pm_runtime_get_sync() failed, the paired pm_runtime_put_sync() will
be called.
Alan and Rafael, is it reasonable to consider this cleanup patch also?
Thanks.
You can very well use pm_runtime_put_noidle() here too. Then, it will
be kind of clear what it's for.
Thanks. Your advice really express we want to do. Will update the patch soon.
return status;
}
+ port_dev-did_runtime_put = false;
}
/* Skip the initial Clear-Suspend step for a remote wakeup */
--
1.7.4.1
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
[PATCH 4/5 V3] usb: call pm_runtime_put_noidle in pm_runtime_get_sync failed case
Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value and runtime power management to behave correctly, call
pm_runtime_put(_sync/noidle) in such case.
Signed-off-by Liu Chuansheng chuansheng@intel.com
Signed-off-by: Li Fei fei...@intel.com
---
drivers/usb/core/hub.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5480352..4a6c055 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3148,12 +3148,13 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
- port_dev-did_runtime_put = false;
if (status 0) {
dev_dbg(udev-dev, can't resume usb port, status
%d\n,
status);
+ pm_runtime_put_noidle(port_dev-dev);
return status;
}
+ port_dev-did_runtime_put = false;
}
/* Skip the initial Clear-Suspend step for a remote wakeup */
--
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
Re: [PATCH 1/2] ACPI / glue: Add .match() callback to struct acpi_bus_type
On Thu, Feb 28, 2013 at 10:53:21PM +0100, Rafael J. Wysocki wrote: From: Rafael J. Wysocki rafael.j.wyso...@intel.com USB uses the .find_bridge() callback from struct acpi_bus_type incorrectly, because as a result of the way it is used by USB every device in the system that doesn't have a bus type or parent is passed to usb_acpi_find_device() for inspection. What USB actually needs, though, is to call usb_acpi_find_device() for USB ports that don't have a bus type defined, but have usb_port_device_type as their device type, as well as for USB devices. To fix that replace the struct bus_type pointer in struct acpi_bus_type used for matching devices to specific subsystems with a .match() callback to be used for this purpose and update the users of struct acpi_bus_type, including USB, accordingly. Define the .match() callback routine for USB, usb_acpi_bus_match(), in such a way that it will cover both USB devices and USB ports and remove the now redundant .find_bridge() callback pointer from usb_acpi_bus. Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org -- 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: storage: onetouch: tighten a range check
Smatch complains because we only allocate ONETOUCH_PKT_LEN (2) bytes but later when we call usb_fill_int_urb() we assume maxp can be up to 8 bytes. I talked to the maintainer and maxp should be capped at ONETOUCH_PKT_LEN. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/usb/storage/onetouch.c b/drivers/usb/storage/onetouch.c index cb79de6..2696489 100644 --- a/drivers/usb/storage/onetouch.c +++ b/drivers/usb/storage/onetouch.c @@ -195,6 +195,7 @@ static int onetouch_connect_input(struct us_data *ss) pipe = usb_rcvintpipe(udev, endpoint-bEndpointAddress); maxp = usb_maxpacket(udev, pipe, usb_pipeout(pipe)); + maxp = min(maxp, ONETOUCH_PKT_LEN); onetouch = kzalloc(sizeof(struct usb_onetouch), GFP_KERNEL); input_dev = input_allocate_device(); @@ -245,8 +246,7 @@ static int onetouch_connect_input(struct us_data *ss) input_dev-open = usb_onetouch_open; input_dev-close = usb_onetouch_close; - usb_fill_int_urb(onetouch-irq, udev, pipe, onetouch-data, -(maxp 8 ? 8 : maxp), + usb_fill_int_urb(onetouch-irq, udev, pipe, onetouch-data, maxp, usb_onetouch_irq, onetouch, endpoint-bInterval); onetouch-irq-transfer_dma = onetouch-data_dma; onetouch-irq-transfer_flags |= URB_NO_TRANSFER_DMA_MAP; -- 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 8/9] usb: chipidea: tell platform layer the ci core probe's result
On Fri, Mar 01, 2013 at 09:45:51AM +0800, Peter Chen wrote: On Thu, Feb 28, 2013 at 12:45:59PM +0200, Felipe Balbi wrote: Hi, On Thu, Feb 28, 2013 at 06:06:55PM +0800, Peter Chen wrote: For runtime pm disabled, we may had to add some ugly things, like notify core probe fail, and platform layer needs to handle this failed notify. Please stop talking about about notify core probe fail that will never happen. Not today, not ever. Forget that idea. Of course, it is ugly way, I just thought if it will affect power, eg dvfs, bus freq if the usb clock is on. Maybe the user will find USB core init fails case causes runtime power rise, and fix the core init fail bug. right, we need to fix the bug, but hiding it under some obscure way of passing return values back to glue layer isn't correct. We should fix the real issue, always, instead of hacking around buggy code. -- balbi signature.asc Description: Digital signature
Re: [PATCH v9 8/9] usb: chipidea: tell platform layer the ci core probe's result
Hi, On Thu, Feb 28, 2013 at 01:55:30PM +0200, Alexander Shishkin wrote: If the probe fails, the ci13xxx_add_device will not return error, (bus_probe_device doesn't has return value) therefore, the platform layer can't know whether core's probe is successful or not, if platform layer goes on using core's struct which is initialized at core's probe, the error will occur. This error is showed when I only compile gadget, the host-only controller reports no supported roles, and fails probe, but imx platform code doesn't know it, and goes on using core's private data. Signed-off-by: Peter Chen peter.c...@freescale.com this just tells you that platform code shouldn't be using the driver directly. passing probe_retval via platform_data is an abomination, fix the real problem instead, whatever it is. So you suggest the platform glue layer should not use core driver's data directly, eg, for your dwc3, the platform glue layer should not use struct dwc3 *dwc directly? yes, and it doesn't. Ever. If the dwc3 core fails to probe, but controller core clk is still on, is it a valid case? of course not, but then again, core clk shouldn't be handled by glue layer. You need to figure out who owns the clock, if it feeds DWC3 why would you clk_get() and clk_prepare_enable() from glue ? Makes no sense. Sorry? I can't find clk_prepare_enable at dwc3/core.c, but at dwc3 core, it try to access register at probe, unless platform layer open the clock, how can the core visit the core register. Is it really this difficult to figure out ? Fair enough, below are all the details: To understand the reason why dwc3/core.c doesn't know about struct clk, you need to consider where the driver was originally written; it was written on an OMAP platform (actually first on a virtual model OMAP - somewhat like QEMU -, than on a PCIe FPGA prototype of the IP, then ARM-based FPGA prototype, then OMAP5, none of which needed explicit clock control, see below). OMAP's PM is written in such a way that a pm_runtime_get() will enable the device the all clocks necessary to be usable. Since OMAP would never need to use clocks directly and I would never be able to test that code, I decided not to add it. Now, if dwc3-exynos needs it, the sane thing to do would be add struct clk knowledge to dwc3/core.c but make it optional. If there are no clocks available, don't bail out. I'm not too familiar with the multitudes of platforms out there, but my simple question is: why can't we have pm runtime take care of enabling/disabling the clocks so that we don't have to do it in drivers? that's what OMAP does. Seems obvious that a platform/SoC/board should know about it's clock tree structure, so why doesn't the platform code then take care of all the dirty details? it might seem that way, but it's not that obvious ;-) Some platforms have a single clock, some will split interface and functional clocks, in some cases, you have extra optional clocks which might be needed during certain use cases or to implement erratas at locations that only driver knows. Then drivers could use platform fixup callbacks. Curious how many ugh, I just puked in my mouth a little bit... drivers out there do proper handling of interface vs functional clocks. Something tells me that the common pattern will be enable all that, in itself, is no argument against explicitly handling clocks. Bugs are bugs and will always exist, instead of hiding the problem, we should fix them ;-) clocks with lots of line of boilerplate copied from that other driver in probe() and disable all in remove(). for a first iteration, you're likely right. When we get to a point where driver is really stable we can start trying to optimize runtime power consumption by fiddling with clocks. If we're not going to access a device's register, why would you keep interface clock running ? That's where e.g. runtime_pm fails to give us a good solution. There's no (easy) way to tell pm_domain code that it can gate interface clock but not functional clock. Well, how quickly can Exynos be changed to handle clocks without driver's knowledge ? Motivation for the platforms to change should come from the general direction of linux kernel maintainers, in order for the change to happen. :) But yes, for the time being, you're right, we'll probably have to just cope with it. exactly. Just because XYZ wants clocks to be handled all via pm_domain, it doesn't mean we can switch to that overnight. On top of that there are situations such as the one describe above. Also, I'm a lot more
Re: [PATCH] usb: dwc3: fix EP_BUSY in case of dequeue
Hi,
On Thu, Feb 28, 2013 at 04:51:29PM +0530, Pratyush Anand wrote:
On 2/28/2013 4:18 PM, Felipe Balbi wrote:
+ if (list_empty(dep-req_queued))
+ dep-flags = ~DWC3_EP_BUSY;
not sure this is correct. Whenever req_queue isn't empty, we call
dwc3_stop_active_transfer() which will clear DWC3_EP_BUSY flag.
Yes, if we clear DWC3_EP_BUSY in dwc3_stop_active_transfer then its
fine. But we do not do that. Probably , error was introduced when
End Transfer completion interrupt handling was removed.
static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
struct usb_request *request)
{
[...]
if (r != req) {
list_for_each_entry(r, dep-req_queued, list) {
if (r == req)
break;
}
if (r == req) {
/* wait until it is processed */
dwc3_stop_active_transfer(dwc, dep-number);
^
look here
goto out1;
}
dev_err(dwc-dev, request %p was not queued to %s\n,
request, ep-name);
ret = -EINVAL;
goto out0;
}
out1:
/* giveback the request */
dwc3_gadget_giveback(dep, req, -ECONNRESET);
out0:
spin_unlock_irqrestore(dwc-lock, flags);
return ret;
}
static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)
{
[...]
cmd = DWC3_DEPCMD_ENDTRANSFER;
cmd |= DWC3_DEPCMD_HIPRI_FORCERM | DWC3_DEPCMD_CMDIOC;
cmd |= DWC3_DEPCMD_PARAM(dep-resource_index);
memset(params, 0, sizeof(params));
ret = dwc3_send_gadget_ep_cmd(dwc, dep-number, cmd, params);
WARN_ON_ONCE(ret);
dep-resource_index = 0;
dep-flags = ~DWC3_EP_BUSY;
^
and here
udelay(100);
}
??
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH RFC] usb: dwc3: Get PHY from platform specific dwc3 dt node.
Hi, On Thu, Feb 28, 2013 at 08:09:33PM +0530, Vivek Gautam wrote: On Thu, Jan 31, 2013 at 9:08 PM, Felipe Balbi ba...@ti.com wrote: On Thu, Jan 31, 2013 at 09:00:37PM +0530, Vivek Gautam wrote: Hi Felipe, On Thu, Jan 31, 2013 at 8:55 PM, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, Jan 31, 2013 at 08:53:27PM +0530, Vivek Gautam wrote: Moreover, SoCs having multiple dwc3 controllers will have multiple PHYs, which eventually be added using usb_add_phy_dev(), and not using usb_add_phy(). So each dwc3 controller won't be able to get PHYs by simply calling devm_usb_get_phy() also. No. We have added usb_get_phy_dev() for that purpose in the case of non-dt. I think, instead you can have a patch to use devm_usb_get_phy_dev() here and in exynos platform specific code use usb_bind_phy() to bind the phy and controller till you change it to dt. We have dt support for dwc3-exynos, in such case we should go ahead with of_platform_populate(), right ? But if when i use of_platform_populate() i will not be able to set dma_mask to dwc3-dev. :-( do you have a special need for dma_mask because OF already sets it. If i am not wrong of_platform_device_create_pdata() will set dev-dev.coherent_dma_mask = DMA_BIT_MASK(32) and not dma_mask. I fact we had some discussion sometime back when we needed the same for dwc3-exynos in the thread: [PATCH v2 1/2] USB: dwc3-exynos: Add support for device tree But couldn't get final node on it. So suggestions here please. :-) hmm.. you're right there. Grant, Rob ? Any hints ? Any suggestions on this ? anyone ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] pci: do not try to assign irq 255
On 02/27/2013 10:13 PM, Bjorn Helgaas wrote:
[+cc Andy]
On Wed, Feb 20, 2013 at 11:53 PM, Hannes Reinecke h...@suse.de wrote:
On 02/20/2013 05:57 PM, Yinghai Lu wrote:
On Tue, Feb 19, 2013 at 11:58 PM, Hannes Reinecke h...@suse.de wrote:
Apparently this device is meant to use MSI _only_ so the BIOS developer
didn't feel the need to assign an INTx here.
According to PCI-3.0, section 6.8 (Message Signalled Interrupts):
It is recommended that devices implement interrupt pins to
provide compatibility in systems that do not support MSI
(devices default to interrupt pins). However, it is expected
that the need for interrupt pins will diminish over time.
Devices that do not support interrupt pins due to pin
constraints (rely on polling for device service) may implement
messages to increase performance without adding additional pins.
Therefore, system configuration software must not assume that a
message capable device has an interrupt pin.
Which sounds to me as if the implementation is valid...
it seems you mess pin with interrupt line.
current code:
unsigned char irq;
pci_read_config_byte(dev, PCI_INTERRUPT_PIN, irq);
dev-pin = irq;
if (irq)
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, irq);
dev-irq = irq;
so if the device does not have interrupt pin implemented, pin should be
zero.
and pin and irq in dev should
be all 0.
But the device _has_ an interrupt pin implemented.
The whole point here is that the interrupt line is _NOT_ zero.
00:14.0 USB controller [0c03]: Intel Corporation 7 Series/C210 Series
Chipset Family USB xHCI Host Controller [8086:1e31] (rev 04) (prog-if 30
[XHCI])
Subsystem: Hewlett-Packard Company Device [103c:179b]
Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort-
TAbort- MAbort- SERR- PERR- INTx-
Interrupt: pin A routed to IRQ 255
Region 0: Memory at d472 (64-bit, non-prefetchable) [size=64K]
Capabilities: [70] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA
PME(D0-,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [80] MSI: Enable- Count=1/8 Maskable- 64bit+
Address: Data:
So at one point we have to decide that -irq is not valid, despite it being
not set to zero.
An alternative fix would be this:
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 68a921d..4a480cb 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -469,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
} else {
dev_warn(dev-dev, PCI INT %c: no GSI\n,
pin_name(pin));
+ dev-irq = 0;
}
return 0;
}
Which probably is a better solution, as here -irq is _definitely_
not valid, so we should reset it to '0' to avoid confusion on upper
layers.
I didn't like the pci_read_irq() change because the PCI spec doesn't
say anything about any PCI_INTERRUPT_LINE values being invalid.
I like this solution better, but I still don't quite understand it.
We have the following code in acpi_pci_irq_enable(). We have
previously tried to look up gsi, but the _PRT doesn't mention this
device, so we have gsi == -1 at this point:
/*
* No IRQ known to the ACPI subsystem - maybe the BIOS /
* driver reported one, then use it. Exit in any case.
*/
if (gsi 0) {
u32 dev_gsi;
/* Interrupt Line values above 0xF are forbidden */
if (dev-irq 0 (dev-irq = 0xF)
(acpi_isa_irq_to_gsi(dev-irq, dev_gsi) == 0)) {
dev_warn(dev-dev, PCI INT %c: no GSI -
using ISA IRQ %d\n,
pin_name(pin), dev-irq);
acpi_register_gsi(dev-dev, dev_gsi,
ACPI_LEVEL_SENSITIVE,
ACPI_ACTIVE_LOW);
} else {
dev_warn(dev-dev, PCI INT %c: no GSI\n,
pin_name(pin));
}
return 0;
}
1) I don't know where the restriction of 0x1-0xF came from.
Presumably this value of dev-irq came from PCI_INTERRUPT_LINE, and I
don't know what forbids values 0xF. The test was added by Andy
Grover in the attached commit. This is ancient history; probably Andy
doesn't remember either :)
This is most likely due to ISA compability. Cf ACPI 4.0,
section 5.2.12.4 Platforms with APIC and Dual 8259 Support:
Systems that support both APIC and dual 8259 interrupt models
must map global system interrupts 0-15 to the 8259
