[PATCH v3] usb: phy: samsung: Add support to set pmu isolation
Changes from v2: - Removed the phandle type of device node properties, instead using sub-nodes now. - Removed the property 'samsung,enable-mask' since it is SoC dependent (SoCs like S5PV210 and S3C64XX have different bits to enable/disable phy controller in comparison to exysno4210 onwards). - Maintaining the phy enable mask (which is SoC dependent) for device type phy and host type phy in the driver data. - Re-structuring to get device properties using sub-nodes for 'usbphy-pmu' Changes from v1: - Changed the name of property for phy handler from'samsung,usb-phyctrl' to 'samsung,usb-phyhandle' to make it look more generic. - Similarly 'samsung,phyctrl-reg' is changed to 'samsung,phyhandle-reg' - Added a check for 'samsung,usb-phyhandle' before getting node from phandle. - Putting the node using 'of_node_put()' which had been missed. - Adding necessary check for the pointer in 'samsung_usbphy_set_isolation()' to avoid any NULL pointer dereferencing. - Unmapping the register ioremapped in 'samsung_usbphy_parse_dt_param()'. Based on usb-next branch with Praveen Paneri's patches on top of it. - http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/134476.html - http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/131763.html Vivek Gautam (1): usb: phy: samsung: Add support to set pmu isolation .../devicetree/bindings/usb/samsung-usbphy.txt | 28 drivers/usb/phy/samsung-usbphy.c | 145 +--- 2 files changed, 152 insertions(+), 21 deletions(-) -- 1.7.6.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usb: phy: samsung: Add support to set pmu isolation
Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
.../devicetree/bindings/usb/samsung-usbphy.txt | 28
drivers/usb/phy/samsung-usbphy.c | 145 +---
2 files changed, 152 insertions(+), 21 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 7b26e2d..09f06f8 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,31 @@ Required properties:
- compatible : should be samsung,exynos4210-usbphy
- reg : base physical address of the phy registers and length of memory mapped
region.
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Optional properties:
+- The samsung usbphy nodes should provide the following information required
+ by USB-phy controller to enable/disable the phy controller.
+ - reg : base physical address of PHY control register in PMU which
+ enables/disables the phy controller.
+ The size of this register is the total sum of size of all
phy-control
+ registers that the SoC has. For example, the size will be
+ '0x4' in case we have only one phy-control register (like in
S3C64XX) or
+ '0x8' in case we have two phy-control registers (like in Exynos4210)
+ and so on.
+
+Example:
+ - Exysno4210
+
+ usbphy@125B {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = samsung,exynos4210-usbphy;
+ reg = 0x125B 0x100;
+
+ usbphy-pmu {
+ /* USB device and host PHY_CONTROL registers */
+ reg = 0x10020704 0x8;
+ };
+ };
diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index 5c5e1bb5..2260029 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -60,20 +60,43 @@
#define MHZ (1000*1000)
#endif
+#define S3C64XX_USBPHY_ENABLE (0x1 16)
+#define EXYNOS_USBPHY_ENABLE (0x1 0)
+
enum samsung_cpu_type {
TYPE_S3C64XX,
TYPE_EXYNOS4210,
};
/*
+ * struct samsung_usbphy_drvdata - driver data for various SoC variants
+ * @cpu_type: machine identifier
+ * @devphy_en_mask: device phy enable mask for PHY CONTROL register
+ * @hostphy_en_mask: host phy enable mask for PHY CONTROL register
+ *
+ * having different mask for host and device type phy
+ * helps in setting independent masks in case of SoCs like
+ * S5PV210 in which PHY0 and PHY1 enable bits belong to same
+ * register placed at [0] and [1] respectively.
+ * Although for newer SoCs like exynos these bits belong to
+ * different registers altogether placed at [0].
+ */
+struct samsung_usbphy_drvdata {
+ int cpu_type;
+ int devphy_en_mask;
+ int hostphy_en_mask;
+};
+
+/*
* struct samsung_usbphy - transceiver driver state
* @phy: transceiver structure
* @plat: platform data
* @dev: The parent device supplied to the probe function
* @clk: usb phy clock
* @regs: usb phy register memory base
+ * @phyctrl_pmureg: usb device phy-control pmu register memory base
* @ref_clk_freq: reference clock frequency selection
- * @cpu_type: machine identifier
+ * @drv_data: driver data available for different cpu types
*/
struct samsung_usbphy {
struct usb_phy phy;
@@ -81,12 +104,66 @@ struct samsung_usbphy {
struct device *dev;
struct clk *clk;
void __iomem*regs;
+ void __iomem*phyctrl_pmureg;
int ref_clk_freq;
- int cpu_type;
+ struct samsung_usbphy_drvdata *drv_data;
};
#define phy_to_sphy(x) container_of((x), struct samsung_usbphy, phy)
+static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy)
+{
+ struct device_node *usbphy_pmu;
+ u32 reg[2];
+ int ret;
+
+ if (!sphy-dev-of_node) {
+ dev_err(sphy-dev, Can't get usb-phy node\n);
+ return -ENODEV;
+ }
+
+ usbphy_pmu = of_get_child_by_name(sphy-dev-of_node, usbphy-pmu);
+ if (!usbphy_pmu)
+ dev_warn(sphy-dev, Can't get usb-phy pmu control node\n);
+
+ ret = of_property_read_u32_array(usbphy_pmu, reg, reg, 2);
+ if (!ret)
+ sphy-phyctrl_pmureg = ioremap(reg[0], reg[1]);
+
+ of_node_put(usbphy_pmu);
+
+ if (IS_ERR_OR_NULL(sphy-phyctrl_pmureg)) {
+ dev_err(sphy-dev, Can't get usb-phy pmu control register\n);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+/*
+ * Set isolation here for phy.
+ * SOCs control this by controlling corresponding PMU registers
+ */
+static void
Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device
Hi all, On Wed, Dec 19, 2012 at 7:16 PM, Vivek Gautam gautamvivek1...@gmail.com wrote: CC: Doug Anderson On Sat, Dec 15, 2012 at 12:50 PM, Grant Likely grant.lik...@secretlab.ca wrote: On Thu, 13 Dec 2012 20:22:26 +0530, Vivek Gautam gautam.vi...@samsung.com wrote: Using chip specific compatible string as it should be. So fixing this for ehci-s5p, ohci-exynos and dwc3-exynos which till now used a generic 'exynos' in their compatible strings. This goes as per the discussion happened in the thread for [PATCH v2] ARM: Exynos5250: Enabling dwc3-exynos driver available at: http://www.spinics.net/lists/linux-usb/msg74145.html Vivek Gautam (2): usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device usb: dwc3-exynos: Fix compatible strings for the device for both patches: Acked-by: Grant Likely grant.lik...@secretlab.ca Any more thought about this patch-set? Or does this change seems fine? drivers/usb/dwc3/dwc3-exynos.c |2 +- drivers/usb/host/ehci-s5p.c|2 +- drivers/usb/host/ohci-exynos.c |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) -- 1.7.6.5 -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- 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 -- Thanks Regards Vivek -- 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 v3 1/2] ARM: Exynos5250: Enabling ehci-s5p driver
Hi all,
On Wed, Dec 19, 2012 at 7:20 PM, Vivek Gautam gautamvivek1...@gmail.com wrote:
CC: Doug Anderson
On Sat, Dec 15, 2012 at 12:53 PM, Grant Likely
grant.lik...@secretlab.ca wrote:
On Thu, 13 Dec 2012 22:06:01 +0530, Vivek Gautam gautam.vi...@samsung.com
wrote:
Adding EHCI device tree node for Exynos5250 along with
the device base adress and gpio line for vbus.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
Acked-by: Jingoo Han jg1@samsung.com
---
.../devicetree/bindings/usb/exynos-usb.txt | 25
arch/arm/boot/dts/exynos5250-smdk5250.dts |4 +++
arch/arm/boot/dts/exynos5250.dtsi |6
arch/arm/mach-exynos/include/mach/map.h|1 +
arch/arm/mach-exynos/mach-exynos5-dt.c |2 +
5 files changed, 38 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/exynos-usb.txt
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
new file mode 100644
index 000..e8bbb47
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -0,0 +1,25 @@
+Samsung Exynos SoC USB controller
+
+The USB devices interface with USB controllers on Exynos SOCs.
+The device node has following properties.
+
+EHCI
+Required properties:
+ - compatible: should be samsung,exynos4210-ehci for USB 2.0
+ EHCI controller in host mode.
+ - reg: physical base address of the controller and length of memory mapped
+ region.
+ - interrupts: interrupt number to the cpu.
+
+Optional properties:
+ - samsung,vbus-gpio: if present, specifies the GPIO that
+ needs to be pulled up for the bus to be powered.
+
+Example:
+
+ usb@1211 {
+ compatible = samsung,exynos4210-ehci;
+ reg = 0x1211 0x100;
+ interrupts = 0 71 0;
+ samsung,vbus-gpio = gpx2 6 1 3 3;
+ };
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 711b55f..f990086 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -218,4 +218,8 @@
i2s_2: i2s@12D7 {
status = disabled;
};
+
+ usb@1211 {
+ samsung,vbus-gpio = gpx2 6 1 3 3;
+ };
};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi
b/arch/arm/boot/dts/exynos5250.dtsi
index 581e57a..584bb9a 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -299,6 +299,12 @@
rx-dma-channel = pdma0 11; /* preliminary */
};
+ usb@1211 {
+ compatible = samsung,exynos4210-ehci;
+ reg = 0x1211 0x100;
+ interrupts = 0 71 0;
+ };
+
amba {
#address-cells = 1;
#size-cells = 1;
diff --git a/arch/arm/mach-exynos/include/mach/map.h
b/arch/arm/mach-exynos/include/mach/map.h
index cbb2852..b2c662f 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -201,6 +201,7 @@
#define EXYNOS4_PA_EHCI 0x1258
#define EXYNOS4_PA_OHCI 0x1259
#define EXYNOS4_PA_HSPHY 0x125B
+#define EXYNOS5_PA_EHCI 0x1211
#define EXYNOS4_PA_MFC 0x1340
#define EXYNOS4_PA_UART 0x1380
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c
b/arch/arm/mach-exynos/mach-exynos5-dt.c
index 462e5ac..b3b9af1 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -110,6 +110,8 @@ static const struct of_dev_auxdata
exynos5250_auxdata_lookup[] __initconst = {
samsung-i2s.1, NULL),
OF_DEV_AUXDATA(samsung,samsung-i2s, 0x12D7,
samsung-i2s.2, NULL),
+ OF_DEV_AUXDATA(samsung,exynos4210-ehci, EXYNOS5_PA_EHCI,
+ s5p-ehci, NULL),
I'm assuming the above change is temporary. What is left to be done to
drop the auxdata in theses two patches?
Otherwise the patch looks fine.
Acked-by: Grant Likely grant.lik...@secretlab.ca
Any more thought about this patch?
Or does this change seems fine?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Thanks Regards
Vivek
--
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] arm/davinci/musb: fix mispint introduced by commit 032ec49f5351e9cb242b1a1c367d14415043ab95
On Fri, Dec 21, 2012 at 01:59:06AM +0400, Mikhail Kshevetskiy wrote:
please respin this patch with a real commit log.
Signed-off-by: Mikhail Kshevetskiy mikhail.kshevets...@gmail.com
---
drivers/usb/musb/da8xx.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 8bc44b7..0ed24a6 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -327,7 +327,7 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void
*hci)
u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
int err;
- err = musb-int_usb USB_INTR_VBUSERROR;
+ err = musb-int_usb MUSB_INTR_VBUSERROR;
if (err) {
/*
* The Mentor core doesn't debounce VBUS as needed
--
1.7.10.4
--
balbi
signature.asc
Description: Digital signature
Re: 3.7 kernel hangs when doing scp
Hi Peter,
On Fri, Dec 21, 2012 at 12:22 AM, Peter Chen peter.c...@freescale.com wrote:
Current chipidea driver only considers disable stream mode at device
mode, in fact, it may be related to below chipidea bug, and needs
to consider all usb modes.
STAR 9000378958
Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host to Hang
on OUT Retry
www.synopsys.com/dw/star.php?c=dwc_usb2_hs_otg_controllerfixedIn=2.20a
To fix this, we need to add CI13XXX_DISABLE_STREAMING after role-start/init.
Yes, setting CI13XXX_DISABLE_STREAMING inside ci_role_start does work.
If you think the patch below is fine I can properly submit it.
---
drivers/usb/chipidea/ci.h | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index e25d126..7fe652a 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -18,6 +18,8 @@
#include linux/usb.h
#include linux/usb/gadget.h
+#define USBMODE_CI_SDIS BIT(4)
+
/**
* DEFINE
*/
@@ -173,22 +175,6 @@ static inline struct ci_role_driver
*ci_role(struct ci13xxx *ci)
return ci-roles[ci-role];
}
-static inline int ci_role_start(struct ci13xxx *ci, enum ci_role role)
-{
- int ret;
-
- if (role = CI_ROLE_END)
- return -EINVAL;
-
- if (!ci-roles[role])
- return -ENXIO;
-
- ret = ci-roles[role]-start(ci);
- if (!ret)
- ci-role = role;
- return ret;
-}
-
static inline void ci_role_stop(struct ci13xxx *ci)
{
enum ci_role role = ci-role;
@@ -307,6 +293,27 @@ static inline u32 hw_test_and_write(struct
ci13xxx *ci, enum ci13xxx_regs reg,
return (val mask) ffs_nr(mask);
}
+
+static inline int ci_role_start(struct ci13xxx *ci, enum ci_role role)
+{
+ int ret;
+
+ if (role = CI_ROLE_END)
+ return -EINVAL;
+
+ if (!ci-roles[role])
+ return -ENXIO;
+
+ ret = ci-roles[role]-start(ci);
+ if (!ret)
+ ci-role = role;
+
+ if (ci-platdata-flags CI13XXX_DISABLE_STREAMING)
+ hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
+
+ return ret;
+}
+
int hw_device_reset(struct ci13xxx *ci, u32 mode);
int hw_port_test_set(struct ci13xxx *ci, u8 mode);
--
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250
Hi Felipe,
On Wed, Dec 19, 2012 at 1:25 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Dec 19, 2012 at 11:52:01AM +0530, Vivek Gautam wrote:
@@ -736,7 +1035,41 @@ static int __devinit samsung_usbphy_probe(struct
platform_device *pdev)
sphy-clk = clk;
- return usb_add_phy(sphy-phy, USB_PHY_TYPE_USB2);
+ sphy-has_usb3 = (sphy-cpu_type == TYPE_EXYNOS5250);
+
+ if (sphy-has_usb3) {
+ struct resource *usb3phy_mem;
+ void __iomem*usb3phy_base;
+
+ usb3phy_mem = platform_get_resource(pdev, IORESOURCE_MEM,
1);
+ if (!usb3phy_mem) {
+ dev_err(dev, %s: missing mem resource\n,
__func__);
+ return -ENODEV;
+ }
+
+ usb3phy_base = devm_request_and_ioremap(dev, usb3phy_mem);
+ if (!usb3phy_base) {
+ dev_err(dev, %s: register mapping failed\n,
__func__);
+ return -ENXIO;
+ }
+
+ sphy-usb3phy.regs_phy = usb3phy_base;
+ sphy-usb3phy.phy.dev = sphy-dev;
+ sphy-usb3phy.phy.label = samsung-usb3phy;
+ sphy-usb3phy.phy.init = samsung_usb3phy_init;
+ sphy-usb3phy.phy.shutdown = samsung_usb3phy_shutdown;
+ }
+
+ ret = usb_add_phy(sphy-phy, USB_PHY_TYPE_USB2);
+ if (ret)
+ return ret;
is this realy how your HW behaves ? USB2 and USB3 phys are a single HW
entity ? I kinda doubt that :-s
They are separate HW in fact.
So, do you expect to see a separate driver interface for USB 3.0 type phy ?
yes. Just as we did on OMAP. One driver for the USB2 part and one driver
for USB3 part (which are actually two, but you can only talk to them as
one) :-)
Sure as you suggest, we will pull out the USB 3.0 code from here and put
a new driver in place for the same.
That will be quite similar architecturally to current samsung-usbphy
driver for USB 2.0 type phy,
and may require some code duplication too.
If it duplicates code, then perhaps it's best to keep it as is but I'm
actually surprised you guys have similar programming model on both
parts. I mean, the differences at HW behavior are huge: on one side you
use ULPI/UTMI+ on the other PIPE3, on one side you have 480Mbps
half-duplex signalling, on the other you have 5Gbps dual simplex
signalling, the differences go on and on.
The programming model for USB phy is just about setting up the phy registers
for example to provide proper clocks to PHY controller, setting up setting up
the UTMI block etc, under a sequence.
Also, what you say about duplicating, it seems to me that it will
duplicate only the boilerplate part (allocating memory, having a
platform_driver, and so on), because you _do_ have completely separate
functions to handle usb3 part.
Yes, the duplication was in fact just the boilerplate part ;-)
apart from having separate functions for USB 3.0 type phy.
One more comment below:
+static u32 exynos5_usb3phy_set_clock(struct samsung_usbphy *sphy)
+{
+ u32 reg;
+ u32 refclk;
+
+ refclk = sphy-ref_clk_freq;
+
+ reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
+ PHYCLKRST_FSEL(refclk);
+
+ switch (refclk) {
+ case HOST_CTRL0_FSEL_CLKSEL_50M:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x00));
+ break;
+ case HOST_CTRL0_FSEL_CLKSEL_20M:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x00));
+ break;
+ case HOST_CTRL0_FSEL_CLKSEL_19200K:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x88));
+ break;
+ case HOST_CTRL0_FSEL_CLKSEL_24M:
+ default:
+ reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
+ PHYCLKRST_SSC_REFCLKSEL(0x88));
+ break;
+ }
+
+ return reg;
+}
looks like this should be done by commong clock framework by clock
reparenting perhaps ?
Need to check on this one.
--
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: 3.7 kernel hangs when doing scp
On Fri, Dec 21, 2012 at 07:24:14AM -0200, Fabio Estevam wrote: Hi Peter, On Fri, Dec 21, 2012 at 12:22 AM, Peter Chen peter.c...@freescale.com wrote: Current chipidea driver only considers disable stream mode at device mode, in fact, it may be related to below chipidea bug, and needs to consider all usb modes. STAR 9000378958 Title: Non-Double Word Aligned Buffer Address Sometimes Causes Host to Hang on OUT Retry www.synopsys.com/dw/star.php?c=dwc_usb2_hs_otg_controllerfixedIn=2.20a To fix this, we need to add CI13XXX_DISABLE_STREAMING after role-start/init. Yes, setting CI13XXX_DISABLE_STREAMING inside ci_role_start does work. If you think the patch below is fine I can properly submit it. Why did you need to move this function? -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: 3.7 kernel hangs when doing scp
On Fri, Dec 21, 2012 at 9:49 AM, Wolfram Sang w.s...@pengutronix.de wrote: Yes, setting CI13XXX_DISABLE_STREAMING inside ci_role_start does work. If you think the patch below is fine I can properly submit it. Why did you need to move this function? Only to make the compiler happy. hw_write() is now used by ci_role_start(), so I re-ordered so that hw_write comes first in the code and compiler does not complain. Regards, Fabio Estevam -- 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: 3.7 kernel hangs when doing scp
Only to make the compiler happy. hw_write() is now used by ci_role_start(), so I re-ordered so that hw_write comes first in the code and compiler does not complain. Yup, spotted this on second glance. If you submit the patch, might be helpful to mention this. -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: Remove CONFIG_USB_SUSPEND?
On Fri, 21 Dec 2012, Greg KH wrote: On Fri, Dec 21, 2012 at 01:21:18PM +0800, Peter Chen wrote: On Thu, Dec 20, 2012 at 08:55:33PM -0800, Greg KH wrote: On Fri, Dec 21, 2012 at 12:28:38PM +0800, Peter Chen wrote: On Tue, Dec 18, 2012 at 11:11:15AM -0500, Alan Stern wrote: I suggest that we remove the CONFIG_USB_SUSPEND option, starting in 3.9. Practically everyone enables it, and the amount of code it protects is fairly small (just portions of usbcore, nothing in the drivers). Basically, if people don't want their kernels to save power then they should turn off CONFIG_PM. Objections, anyone? Sorry, I may not agree with this due to below reasons: - Some customers wants system PM, but do not want usb runtime power management, such as Car Navigation System. They want system responds fast after user press power on key, but they don't care usb power during runtime. What is fast? And you can always explicitly keep the device away by writing to the proper sysfs file, right? I mean system suspend/resume. That's totally different and is not relevant here. Actually it is slightly relevant. When CONFIG_USB_SUSPEND is enabled, during system suspend we go through the whole USB device tree and explicitly suspend each device (and we do the opposite during system resume). Without CONFIG_USB_SUSPEND, only the root hubs are suspended explicitly -- this is what the USB spec calls a global suspend. It doesn't work for USB-3 buses, but I assume Peter is referring only to USB-2. Even though the suspend and resume activities are carried out in multiple threads in parallel, it seems reasonable that a global approach would take less time. In theory we should be able to use global approach for system sleeps even with CONFIG_USB_SUSPEND enabled. I trust this would answer Peter's objection. So you need to get this working properly for your devices, what's wrong with that? :) Yes, we can debug that but it needs some efforts. Maybe for alpha release we only add basic USB function, for beta release we will add usb suspend/resume function. Then, at next release, we will add USB runtime PM. I just want to say keep CONFIG_USB_SUSPEND can give users more choices no matter debug, development or final product procedure. This ought to be irrelevant, because you can effectively turn off runtime PM in userspace whenever you need to. Or you can turn it off by default for all USB devices by making a one-line change to the kernel source (initialize usb_autosuspend_delay to -1 in drivers/usb/core/usb.c). That really is only your issue, and one that might force your company to actually work better with the Linux kernel community in order to get your hardware working properly upstream (hint, Freescale has a horrible reputation here, I personally always advise people to use other hardware because of it.) We don't have alpha and the like type of releases. If this option goes away, it still doesn't affect your ability to actually support USB suspend in a better way, but it becomes more obvious that you don't, which I think is a good thing. Without commenting on Freescale's degree of community support, it still seems clear that there is essentially no real advantage to disabling CONFIG_USB_SUSPEND -- or at least, there won't be after we add support for global USB suspend and resume during system sleep. 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: USB root hub suspend the device during negotiate, Atmel AT91SAM9G45-EKES, kernel 2.6.30 Re: Can you take a look the echi log?
On Fri, 21 Dec 2012, Kuo Shou-Chien wrote: Hi Alan, Thank you for your replay and sorry for the lengthy log from booting. Kernel version is 2.6.30. Yes, the board has OHCI controller. By the way, the dmesg output is the output from the dmesg command. It is not the contents of a system log file. Jan �1 00:01:18 at91sam user.debug kernel: usb usb1: usb resume Jan �1 00:01:18 at91sam user.debug kernel: atmel-ehci atmel-ehci: resume root hub Jan �1 00:01:18 at91sam user.debug kernel: hub 1-0:1.0: hub_resume Jan �1 00:01:18 at91sam user.debug kernel: atmel-ehci atmel-ehci: GetStatus port 2 status 001803 POWER sig=j CSC CONNECT Jain ehci_irq �in ohci_irq Did you add these messages yourself? n �1 00:01:18 at91sam user.debug kernel: hub 1-0:1.0: port 2: status 0501 change 0001 Jan �1 00:01:19 at91sam user.debug kernel: hub 1-0:1.0: state 7 ports 2 chg 0004 evt Jan �1 00:01:19 at91sam user.debug kernel: hub 1-0:1.0: port 2, status 0501, change , 480 Mb/s Jan �1 00:01:19 at91sam user.debug kernel: hub 1-0:1.0: skuo : retry 0 status = 0 Jan �1 00:01:19 at91sam user.debug kernel: atmel-ehci atmel-ehci: port 2 full speed -- companion Jan �1 00:01:19 at91sam user.debug kernel: atmel-ehci atmel-ehci: GetStatus port 2 status 003801 POWER OWNER sig=j CONNECT Jan �1 00:01:19 at91sam user.info kernel: in ehci_irq Jan �1 00:01:19 at91sam user.info kernel: �in ohci_irq The problem is here. The OHCI hardware or the ohci-hcd driver isn't working right. A usb usb2: usb resume message should appear here. Jan �1 00:01:19 at91sam user.debug kernel: hub 1-0:1.0: port 2 not reset yet, waiting 50ms Jan �1 00:01:19 at91sam user.debug kernel: atmel-ehci atmel-ehci: GetStatus port 2 status 003002 POWER OWNER sig=se0 CSC Jan �1 00:01:19 at91sam user.debug kernel: hub 1-0:1.0: skuo : retry status = -107 Jan �1 00:01:19 at91sam user.debug kernel: hub 1-0:1.0: state 7 ports 2 chg evt 0004 Jan �1 00:01:21 at91sam user.debug kernel: hub 1-0:1.0: hub_suspend Jan �1 00:01:21 at91sam user.debug kernel: usb usb1: bus auto-suspend Jan �1 00:01:21 at91sam user.debug kernel: atmel-ehci atmel-ehci: suspend root hub 2.6.30 is a very old kernel. You should try using a newer kernel. 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
How to add USB device ID
I use an USB device that looks like a serial port to the kernel. However, since the Device ID is unknown, I have to either modprobe usbserial with the vendor/id codes as parameters, or modify generic.c so it knows to handle the device. I have been going with the code modification since there are 2 versions of this device with different numbers, and you can only do 1 on the command line. What is the right way to see if it can be pushed into mainline support? It seems overkill to write another driver that essentially wraps around generic.c. Thanks, Kevin -- 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 22/25] usb/at91_udc: don't use [delayed_]work_pending()
There's no need to test whether a (delayed) work item in pending before queueing, flushing or cancelling it. Most uses are unnecessary and quite a few of them are buggy. Remove unnecessary pending tests from at91_udc. Only compile tested. Signed-off-by: Tejun Heo t...@kernel.org Cc: Andrew Victor li...@maxim.org.za Cc: Nicolas Ferre nicolas.fe...@atmel.com Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com Cc: Felipe Balbi ba...@ti.com Cc: linux-usb@vger.kernel.org --- Please let me know how this patch should be routed. I can take it through the workqueue tree if necessary. Thanks. drivers/usb/gadget/at91_udc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index f4a21f6..e81d8a2 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -1621,8 +1621,7 @@ static void at91_vbus_timer(unsigned long data) * bus such as i2c or spi which may sleep, so schedule some work * to read the vbus gpio */ - if (!work_pending(udc-vbus_timer_work)) - schedule_work(udc-vbus_timer_work); + schedule_work(udc-vbus_timer_work); } static int at91_start(struct usb_gadget *gadget, -- 1.8.0.2 -- 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: chipidea: Allow disabling streaming not only in udc mode
On Fri, Dec 21, 2012 at 10:27:30AM -0200, Fabio Estevam wrote:
From: Fabio Estevam fabio.este...@freescale.com
When running a scp transfer using a USB/Ethernet adapter the following crash
happens:
$ scp test.tar.gz fabio@192.168.1.100:/home/fabio
fabio@192.168.1.100's password:
test.tar.gz 0%0 0.0KB/s --:--
ETA
[ cut here ]
WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0x2cc/0x2f0()
NETDEV WATCHDOG: eth0 (asix): transmit queue 0 timed out
Modules linked in:
Backtrace:
[80011c94] (dump_backtrace+0x0/0x10c) from [804d3a5c]
(dump_stack+0x18/0x1c)
r6:00ff r5:80412388 r4:80685dc0 r3:80696cc0
[804d3a44] (dump_stack+0x0/0x1c) from [80021868]
(warn_slowpath_common+0x54/0x6c)
[80021814] (warn_slowpath_common+0x0/0x6c) from [80021924]
(warn_slowpath_fmt+0x38/0x40)
...
Setting SDIS (Stream Disable Mode- bit 4 of USBMODE register) fixes the
problem.
However, in current code CI13XXX_DISABLE_STREAMING flag is only set in udc
mode,
so allow disabling streaming also in host mode.
Also, in order to make the compiler happy, ci_role_start() was moved after
hw_write().
Tested on a mx6qsabrelite board.
Suggested-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
drivers/usb/chipidea/ci.h | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index e25d126..7fe652a 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -18,6 +18,8 @@
#include linux/usb.h
#include linux/usb/gadget.h
+#define USBMODE_CI_SDIS BIT(4)
+
/**
* DEFINE
*/
@@ -173,22 +175,6 @@ static inline struct ci_role_driver *ci_role(struct
ci13xxx *ci)
return ci-roles[ci-role];
}
-static inline int ci_role_start(struct ci13xxx *ci, enum ci_role role)
-{
- int ret;
-
- if (role = CI_ROLE_END)
- return -EINVAL;
-
- if (!ci-roles[role])
- return -ENXIO;
-
- ret = ci-roles[role]-start(ci);
- if (!ret)
- ci-role = role;
- return ret;
-}
-
static inline void ci_role_stop(struct ci13xxx *ci)
{
enum ci_role role = ci-role;
@@ -307,6 +293,27 @@ static inline u32 hw_test_and_write(struct ci13xxx *ci,
enum ci13xxx_regs reg,
return (val mask) ffs_nr(mask);
}
+
+static inline int ci_role_start(struct ci13xxx *ci, enum ci_role role)
+{
+ int ret;
+
+ if (role = CI_ROLE_END)
+ return -EINVAL;
+
+ if (!ci-roles[role])
+ return -ENXIO;
+
+ ret = ci-roles[role]-start(ci);
+ if (!ret)
+ ci-role = role;
+
+ if (ci-platdata-flags CI13XXX_DISABLE_STREAMING)
+ hw_write(ci, OP_USBMODE, USBMODE_CI_SDIS, USBMODE_CI_SDIS);
+
+ return ret;
+}
+
int hw_device_reset(struct ci13xxx *ci, u32 mode);
How about only adding it at end of host_start of drivers/usb/chipidea/host.c
int hw_port_test_set(struct ci13xxx *ci, u8 mode);
--
1.7.9.5
--
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: Remove CONFIG_USB_SUSPEND?
On Fri, Dec 21, 2012 at 07:00:58AM -0800, Greg KH wrote: On Fri, Dec 21, 2012 at 01:21:18PM +0800, Peter Chen wrote: On Thu, Dec 20, 2012 at 08:55:33PM -0800, Greg KH wrote: On Fri, Dec 21, 2012 at 12:28:38PM +0800, Peter Chen wrote: On Tue, Dec 18, 2012 at 11:11:15AM -0500, Alan Stern wrote: I suggest that we remove the CONFIG_USB_SUSPEND option, starting in 3.9. Practically everyone enables it, and the amount of code it protects is fairly small (just portions of usbcore, nothing in the drivers). Basically, if people don't want their kernels to save power then they should turn off CONFIG_PM. Objections, anyone? Sorry, I may not agree with this due to below reasons: - Some customers wants system PM, but do not want usb runtime power management, such as Car Navigation System. They want system responds fast after user press power on key, but they don't care usb power during runtime. What is fast? And you can always explicitly keep the device away by writing to the proper sysfs file, right? I mean system suspend/resume. That's totally different and is not relevant here. And this isn't always true from all automotive systems, some that I talk to in that industry do need this, as they are not allowed to drain a lot of power all the time (think about when running on a car battery), and their power budget is quite low. Correct, CONFIG_USB_SUSPEND can let the user choose if they want USB runtime PM or not, you know different users may have different requirements. I don't know anyone that doesn't want USB runtime suspend, that's the issue here. - For embedded system, we have may SoCs, and USB low power mode is the most challenge job when we bring up new USB module, usually, our develop sequence like: basic USB functions -- USB function after System suspend/resume -- USB Runtime PM. We usually disable USB runtime PM at first. So you need to get this working properly for your devices, what's wrong with that? :) Yes, we can debug that but it needs some efforts. Maybe for alpha release we only add basic USB function, for beta release we will add usb suspend/resume function. Then, at next release, we will add USB runtime PM. I just want to say keep CONFIG_USB_SUSPEND can give users more choices no matter debug, development or final product procedure. That really is only your issue, and one that might force your company to actually work better with the Linux kernel community in order to get your hardware working properly upstream (hint, Freescale has a horrible reputation here, I personally always advise people to use other hardware because of it.) Thanks, greg. I will share it to other Freescale guys, hope we can work better with community in near future. We don't have alpha and the like type of releases. If this option goes away, it still doesn't affect your ability to actually support USB suspend in a better way, but it becomes more obvious that you don't, which I think is a good thing. Good luck, greg k-h -- 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: Remove CONFIG_USB_SUSPEND?
On Fri, Dec 21, 2012 at 11:53:40AM -0500, Alan Stern wrote: On Fri, 21 Dec 2012, Greg KH wrote: On Fri, Dec 21, 2012 at 01:21:18PM +0800, Peter Chen wrote: On Thu, Dec 20, 2012 at 08:55:33PM -0800, Greg KH wrote: On Fri, Dec 21, 2012 at 12:28:38PM +0800, Peter Chen wrote: On Tue, Dec 18, 2012 at 11:11:15AM -0500, Alan Stern wrote: I suggest that we remove the CONFIG_USB_SUSPEND option, starting in 3.9. Practically everyone enables it, and the amount of code it protects is fairly small (just portions of usbcore, nothing in the drivers). Basically, if people don't want their kernels to save power then they should turn off CONFIG_PM. Objections, anyone? Sorry, I may not agree with this due to below reasons: - Some customers wants system PM, but do not want usb runtime power management, such as Car Navigation System. They want system responds fast after user press power on key, but they don't care usb power during runtime. What is fast? And you can always explicitly keep the device away by writing to the proper sysfs file, right? I mean system suspend/resume. That's totally different and is not relevant here. Actually it is slightly relevant. When CONFIG_USB_SUSPEND is enabled, during system suspend we go through the whole USB device tree and explicitly suspend each device (and we do the opposite during system resume). Without CONFIG_USB_SUSPEND, only the root hubs are suspended explicitly -- this is what the USB spec calls a global suspend. It doesn't work for USB-3 buses, but I assume Peter is referring only to USB-2. Even though the suspend and resume activities are carried out in multiple threads in parallel, it seems reasonable that a global approach would take less time. In theory we should be able to use global approach for system sleeps even with CONFIG_USB_SUSPEND enabled. I trust this would answer Peter's objection. Thanks, Alan. Clear now. So you need to get this working properly for your devices, what's wrong with that? :) Yes, we can debug that but it needs some efforts. Maybe for alpha release we only add basic USB function, for beta release we will add usb suspend/resume function. Then, at next release, we will add USB runtime PM. I just want to say keep CONFIG_USB_SUSPEND can give users more choices no matter debug, development or final product procedure. This ought to be irrelevant, because you can effectively turn off runtime PM in userspace whenever you need to. Or you can turn it off by default for all USB devices by making a one-line change to the kernel source (initialize usb_autosuspend_delay to -1 in drivers/usb/core/usb.c). This was one of my concerns that in case the user wants to stop USB runtime PM at all (Even don't want the first one), the solution is we can change the value of usb_autosuspend_delay. Thanks. -- 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: How to add USB device ID
On Fri, Dec 21, 2012 at 05:54:33PM -0600, Kevin K wrote: I use an USB device that looks like a serial port to the kernel. However, since the Device ID is unknown, I have to either modprobe usbserial with the vendor/id codes as parameters, or modify generic.c so it knows to handle the device. I have been going with the code modification since there are 2 versions of this device with different numbers, and you can only do 1 on the command line. What is the right way to see if it can be pushed into mainline support? It seems overkill to write another driver that essentially wraps around generic.c. No it isn't overkill, we have a few drivers like this. What type of USB serial chip is this device? Perhaps we should just add the device ids to an existing driver? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
