[PATCH V2] usb: {ohci,ehci}-platform: Use new OF big-endian helper function
This handles the existing big-endian case, and in addition, it also does the right thing when native-endian is specified. Signed-off-by: Kevin Cernekee cerne...@gmail.com Acked-by: Alan Stern st...@rowland.harvard.edu --- Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 ++ Documentation/devicetree/bindings/usb/usb-ohci.txt | 2 ++ drivers/usb/host/ehci-platform.c | 2 +- drivers/usb/host/ohci-platform.c | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) V1-V2: Tweak documentation per feedback on the list This depends on the new of_device_is_big_endian() function, which is being handled through Grant's tree. diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index 43c1a4e06767..927757b13ff7 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -12,6 +12,8 @@ Optional properties: - big-endian-regs : boolean, set this for hcds with big-endian registers - big-endian-desc : boolean, set this for hcds with big-endian descriptors - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc + - native-endian : boolean, enables big-endian-regs + big-endian-desc + if the CPU is running in big-endian mode - clocks : a list of phandle + clock specifier pairs - phys : phandle + phy specifier pair - phy-names : usb diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt index 19233b7365e1..42f63b865b97 100644 --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt @@ -9,6 +9,8 @@ Optional properties: - big-endian-regs : boolean, set this for hcds with big-endian registers - big-endian-desc : boolean, set this for hcds with big-endian descriptors - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc +- native-endian : boolean, enables big-endian-regs + big-endian-desc + if the CPU is running in big-endian mode - no-big-frame-no : boolean, set if frame_no lives in bits [15:0] of HCCA - num-ports : u32, to override the detected port count - clocks : a list of phandle + clock specifier pairs diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index 2f5b9ce3e042..0da9d7061108 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -187,7 +187,7 @@ static int ehci_platform_probe(struct platform_device *dev) if (of_property_read_bool(dev-dev.of_node, big-endian-desc)) ehci-big_endian_desc = 1; - if (of_property_read_bool(dev-dev.of_node, big-endian)) + if (of_device_is_big_endian(dev-dev.of_node)) ehci-big_endian_mmio = ehci-big_endian_desc = 1; priv-phy = devm_phy_get(dev-dev, usb); diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index 7793c3cfcf1f..029a606e4d4a 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -157,7 +157,7 @@ static int ohci_platform_probe(struct platform_device *dev) if (of_property_read_bool(dev-dev.of_node, big-endian-desc)) ohci-flags |= OHCI_QUIRK_BE_DESC; - if (of_property_read_bool(dev-dev.of_node, big-endian)) + if (of_device_is_big_endian(dev-dev.of_node)) ohci-flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC; if (of_property_read_bool(dev-dev.of_node, no-big-frame-no)) -- 2.1.0 -- 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 9/9] usb: {ohci,ehci}-platform: Use new OF big-endian helper function
On Wed, Nov 26, 2014 at 7:14 AM, Alan Stern st...@rowland.harvard.edu wrote: diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index 43c1a4e..9505c31 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -12,6 +12,8 @@ Optional properties: - big-endian-regs : boolean, set this for hcds with big-endian registers - big-endian-desc : boolean, set this for hcds with big-endian descriptors - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc + - native-endian : boolean, enables big-endian-regs + big-endian-desc + iff the kernel was compiled for big endian Is this really a property of the hardware? It appears to depend on the kernel configuration. As such, is it appropriate for DT? Yes, the peripheral registers automatically adjust their endianness to match the CPU. So if the CPU is running an LE kernel, the peripheral needs to be accessed in LE mode; if the CPU is running a BE kernel, the peripheral needs to be accessed in BE 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
[PATCH 4/9] bus: brcmstb_gisb: Make the driver buildable on MIPS
BCM7xxx ARM and MIPS platforms share a similar hardware block for
reporting GISB errors, so they both benefit from the use of this driver.
Conditionally compile the ARM-specific bus error handler so that the
GISB error IRQ handler works on other architectures.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/bus/Kconfig| 2 +-
drivers/bus/brcmstb_gisb.c | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 603eb1b..b99729e 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -6,7 +6,7 @@ menu Bus devices
config BRCMSTB_GISB_ARB
bool Broadcom STB GISB bus arbiter
- depends on ARM
+ depends on ARM || MIPS
help
Driver for the Broadcom Set Top Box System-on-a-chip internal bus
arbiter. This driver provides timeout and target abort error handling
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index f2cd6a2d..5da935a 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -24,8 +24,10 @@
#include linux/of.h
#include linux/bitops.h
+#ifdef CONFIG_ARM
#include asm/bug.h
#include asm/signal.h
+#endif
#define ARB_TIMER 0x008
#define ARB_ERR_CAP_CLR0x7e4
@@ -141,6 +143,7 @@ static int brcmstb_gisb_arb_decode_addr(struct
brcmstb_gisb_arb_device *gdev,
return 0;
}
+#ifdef CONFIG_ARM
static int brcmstb_bus_error_handler(unsigned long addr, unsigned int fsr,
struct pt_regs *regs)
{
@@ -165,6 +168,7 @@ void __init brcmstb_hook_fault_code(void)
hook_fault_code(22, brcmstb_bus_error_handler, SIGBUS, 0,
imprecise external abort);
}
+#endif
static irqreturn_t brcmstb_gisb_timeout_handler(int irq, void *dev_id)
{
--
2.1.0
--
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/9] power/reset: brcmstb: Use the DT compatible string to indicate bit positions
Some of the older chips used different bits to arm and trigger the reset.
Add the infrastructure needed to specify this through the compatible
string.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/power/reset/brcmstb-reboot.c | 35 ---
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/power/reset/brcmstb-reboot.c
b/drivers/power/reset/brcmstb-reboot.c
index 3306241..4e61c3f 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -11,6 +11,7 @@
* GNU General Public License for more details.
*/
+#include linux/bitops.h
#include linux/device.h
#include linux/errno.h
#include linux/init.h
@@ -34,13 +35,20 @@ static struct regmap *regmap;
static u32 rst_src_en;
static u32 sw_mstr_rst;
+struct reset_reg_mask {
+ u32 rst_src_en_mask;
+ u32 sw_mstr_rst_mask;
+};
+
+static const struct reset_reg_mask *reset_masks;
+
static int brcmstb_restart_handler(struct notifier_block *this,
unsigned long mode, void *cmd)
{
int rc;
u32 tmp;
- rc = regmap_write(regmap, rst_src_en, 1);
+ rc = regmap_write(regmap, rst_src_en, reset_masks-rst_src_en_mask);
if (rc) {
pr_err(failed to write rst_src_en (%d)\n, rc);
return NOTIFY_DONE;
@@ -52,7 +60,7 @@ static int brcmstb_restart_handler(struct notifier_block
*this,
return NOTIFY_DONE;
}
- rc = regmap_write(regmap, sw_mstr_rst, 1);
+ rc = regmap_write(regmap, sw_mstr_rst, reset_masks-sw_mstr_rst_mask);
if (rc) {
pr_err(failed to write sw_mstr_rst (%d)\n, rc);
return NOTIFY_DONE;
@@ -75,10 +83,28 @@ static struct notifier_block brcmstb_restart_nb = {
.priority = 128,
};
+static const struct reset_reg_mask reset_bits_40nm = {
+ .rst_src_en_mask = BIT(0),
+ .sw_mstr_rst_mask = BIT(0),
+};
+
+static const struct of_device_id of_match[] = {
+ { .compatible = brcm,brcmstb-reboot, .data = reset_bits_40nm },
+ {},
+};
+
static int brcmstb_reboot_probe(struct platform_device *pdev)
{
int rc;
struct device_node *np = pdev-dev.of_node;
+ const struct of_device_id *of_id;
+
+ of_id = of_match_node(of_match, np);
+ if (!of_id) {
+ pr_err(failed to look up compatible string\n);
+ return -EINVAL;
+ }
+ reset_masks = of_id-data;
regmap = syscon_regmap_lookup_by_phandle(np, syscon);
if (IS_ERR(regmap)) {
@@ -108,11 +134,6 @@ static int brcmstb_reboot_probe(struct platform_device
*pdev)
return 0;
}
-static const struct of_device_id of_match[] = {
- { .compatible = brcm,brcmstb-reboot, },
- {},
-};
-
static struct platform_driver brcmstb_reboot_driver = {
.probe = brcmstb_reboot_probe,
.driver = {
--
2.1.0
--
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/9] Extend various drivers to run on bi-endian BMIPS hosts
This patch series incorporates the following changes:
- Extend brcmstb reset driver to work on MIPS (currently ARM-only).
- Extend brcmstb GISB bus driver to work on MIPS (currently ARM-only).
- Extend brcmstb GISB bus driver to work on BE systems (currently LE-only).
- Extend both drivers to support the older register layouts used on many
of the BMIPS platforms.
- Extend {ohci,ehci}-platform drivers to accept the new native-endian
DT property, to accommodate BCM7xxx platforms that can be switched
between LE/BE with a board jumper.
Dependencies:
power/reset: brcmstb: Register with kernel restart handler (Guenter Roeck)
of: Add helper function to check MMIO register endianness (Kevin Cernekee)
These are both tentatively accepted, but might not be present in the same
tree yet. As such, we might want to review now, merge later.
Kevin Cernekee (9):
power/reset: brcmstb: Make the driver buildable on MIPS
power/reset: brcmstb: Use the DT compatible string to indicate bit
positions
power/reset: brcmstb: Add support for old 65nm chips
bus: brcmstb_gisb: Make the driver buildable on MIPS
bus: brcmstb_gisb: Introduce wrapper functions for MMIO accesses
bus: brcmstb_gisb: Look up register offsets in a table
bus: brcmstb_gisb: Add register offset tables for older chips
bus: brcmstb_gisb: Honor the big-endian and native-endian DT
properties
usb: {ohci,ehci}-platform: Use new OF big-endian helper function
.../devicetree/bindings/arm/brcm-brcmstb.txt | 4 +-
.../devicetree/bindings/bus/brcm,gisb-arb.txt | 6 +-
Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 +
Documentation/devicetree/bindings/usb/usb-ohci.txt | 2 +
drivers/bus/Kconfig| 2 +-
drivers/bus/brcmstb_gisb.c | 127 ++---
drivers/power/reset/Kconfig| 9 +-
drivers/power/reset/brcmstb-reboot.c | 41 +--
drivers/usb/host/ehci-platform.c | 2 +-
drivers/usb/host/ohci-platform.c | 2 +-
10 files changed, 161 insertions(+), 36 deletions(-)
--
2.1.0
--
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 9/9] usb: {ohci,ehci}-platform: Use new OF big-endian helper function
This handles the existing big-endian case, and in addition, it also does the right thing when native-endian is specified. Signed-off-by: Kevin Cernekee cerne...@gmail.com --- Documentation/devicetree/bindings/usb/usb-ehci.txt | 2 ++ Documentation/devicetree/bindings/usb/usb-ohci.txt | 2 ++ drivers/usb/host/ehci-platform.c | 2 +- drivers/usb/host/ohci-platform.c | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt index 43c1a4e..9505c31 100644 --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt @@ -12,6 +12,8 @@ Optional properties: - big-endian-regs : boolean, set this for hcds with big-endian registers - big-endian-desc : boolean, set this for hcds with big-endian descriptors - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc + - native-endian : boolean, enables big-endian-regs + big-endian-desc + iff the kernel was compiled for big endian - clocks : a list of phandle + clock specifier pairs - phys : phandle + phy specifier pair - phy-names : usb diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt index 19233b7..3bb9673 100644 --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt @@ -9,6 +9,8 @@ Optional properties: - big-endian-regs : boolean, set this for hcds with big-endian registers - big-endian-desc : boolean, set this for hcds with big-endian descriptors - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc +- native-endian : boolean, enables big-endian-regs + big-endian-desc + iff the kernel was compiled for big endian - no-big-frame-no : boolean, set if frame_no lives in bits [15:0] of HCCA - num-ports : u32, to override the detected port count - clocks : a list of phandle + clock specifier pairs diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c index 2f5b9ce..0da9d70 100644 --- a/drivers/usb/host/ehci-platform.c +++ b/drivers/usb/host/ehci-platform.c @@ -187,7 +187,7 @@ static int ehci_platform_probe(struct platform_device *dev) if (of_property_read_bool(dev-dev.of_node, big-endian-desc)) ehci-big_endian_desc = 1; - if (of_property_read_bool(dev-dev.of_node, big-endian)) + if (of_device_is_big_endian(dev-dev.of_node)) ehci-big_endian_mmio = ehci-big_endian_desc = 1; priv-phy = devm_phy_get(dev-dev, usb); diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index 7793c3c..029a606 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -157,7 +157,7 @@ static int ohci_platform_probe(struct platform_device *dev) if (of_property_read_bool(dev-dev.of_node, big-endian-desc)) ohci-flags |= OHCI_QUIRK_BE_DESC; - if (of_property_read_bool(dev-dev.of_node, big-endian)) + if (of_device_is_big_endian(dev-dev.of_node)) ohci-flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC; if (of_property_read_bool(dev-dev.of_node, no-big-frame-no)) -- 2.1.0 -- 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/9] power/reset: brcmstb: Make the driver buildable on MIPS
Now that the driver doesn't use any ARM-specific headers, it is safe to build on MIPS or with COMPILE_TEST. Signed-off-by: Kevin Cernekee cerne...@gmail.com --- drivers/power/reset/Kconfig | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig index f65ff49..0379846 100644 --- a/drivers/power/reset/Kconfig +++ b/drivers/power/reset/Kconfig @@ -39,14 +39,13 @@ config POWER_RESET_AXXIA Say Y if you have an Axxia family SoC. config POWER_RESET_BRCMSTB - bool Broadcom STB reset driver if COMPILE_TEST - depends on ARM + bool Broadcom STB reset driver + depends on ARM || MIPS || COMPILE_TEST default ARCH_BRCMSTB help - This driver provides restart support for ARM-based Broadcom STB - boards. + This driver provides restart support for Broadcom STB boards. - Say Y here if you have an ARM-based Broadcom STB board and you wish + Say Y here if you have a Broadcom STB board and you wish to have restart support. config POWER_RESET_GPIO -- 2.1.0 -- 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 6/9] bus: brcmstb_gisb: Look up register offsets in a table
There are at least 4 incompatible variations of this hardware block,
so let's use the ARB_* constants as a table index instead of hardcoding
specific register offsets. Also, allow for the possibility of adding
old devices that are missing some of the registers.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/bus/brcmstb_gisb.c | 42 ++
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 8ff403d..ef1e423 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -29,23 +29,37 @@
#include asm/signal.h
#endif
-#define ARB_TIMER 0x008
-#define ARB_ERR_CAP_CLR0x7e4
#define ARB_ERR_CAP_CLEAR (1 0)
-#define ARB_ERR_CAP_HI_ADDR0x7e8
-#define ARB_ERR_CAP_ADDR 0x7ec
-#define ARB_ERR_CAP_DATA 0x7f0
-#define ARB_ERR_CAP_STATUS 0x7f4
#define ARB_ERR_CAP_STATUS_TIMEOUT(1 12)
#define ARB_ERR_CAP_STATUS_TEA(1 11)
#define ARB_ERR_CAP_STATUS_BS_SHIFT (1 2)
#define ARB_ERR_CAP_STATUS_BS_MASK0x3c
#define ARB_ERR_CAP_STATUS_WRITE (1 1)
#define ARB_ERR_CAP_STATUS_VALID (1 0)
-#define ARB_ERR_CAP_MASTER 0x7f8
+
+enum {
+ ARB_TIMER,
+ ARB_ERR_CAP_CLR,
+ ARB_ERR_CAP_HI_ADDR,
+ ARB_ERR_CAP_ADDR,
+ ARB_ERR_CAP_DATA,
+ ARB_ERR_CAP_STATUS,
+ ARB_ERR_CAP_MASTER,
+};
+
+static const int gisb_offsets_bcm7445[] = {
+ [ARB_TIMER] = 0x008,
+ [ARB_ERR_CAP_CLR] = 0x7e4,
+ [ARB_ERR_CAP_HI_ADDR] = 0x7e8,
+ [ARB_ERR_CAP_ADDR] = 0x7ec,
+ [ARB_ERR_CAP_DATA] = 0x7f0,
+ [ARB_ERR_CAP_STATUS]= 0x7f4,
+ [ARB_ERR_CAP_MASTER]= 0x7f8,
+};
struct brcmstb_gisb_arb_device {
void __iomem*base;
+ const int *gisb_offsets;
struct mutexlock;
struct list_head next;
u32 valid_mask;
@@ -56,11 +70,21 @@ static LIST_HEAD(brcmstb_gisb_arb_device_list);
static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg)
{
- return ioread32(gdev-base + reg);
+ int offset = gdev-gisb_offsets[reg];
+
+ /* return 1 if the hardware doesn't have ARB_ERR_CAP_MASTER */
+ if (offset == -1)
+ return 1;
+
+ return ioread32(gdev-base + offset);
}
static void gisb_write(struct brcmstb_gisb_arb_device *gdev, u32 val, int reg)
{
+ int offset = gdev-gisb_offsets[reg];
+
+ if (offset == -1)
+ return;
iowrite32(val, gdev-base + reg);
}
@@ -230,6 +254,8 @@ static int brcmstb_gisb_arb_probe(struct platform_device
*pdev)
if (IS_ERR(gdev-base))
return PTR_ERR(gdev-base);
+ gdev-gisb_offsets = gisb_offsets_bcm7445;
+
err = devm_request_irq(pdev-dev, timeout_irq,
brcmstb_gisb_timeout_handler, 0, pdev-name,
gdev);
--
2.1.0
--
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 7/9] bus: brcmstb_gisb: Add register offset tables for older chips
This will select the appropriate register layout based on the DT
compatible string.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
.../devicetree/bindings/bus/brcm,gisb-arb.txt | 6 ++-
drivers/bus/brcmstb_gisb.c | 52 +++---
2 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
b/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
index e2d501d..1eceefb 100644
--- a/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
+++ b/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
@@ -2,7 +2,11 @@ Broadcom GISB bus Arbiter controller
Required properties:
-- compatible: should be brcm,gisb-arb
+- compatible:
+brcm,gisb-arb or brcm,bcm7445-gisb-arb for 28nm chips
+brcm,bcm7435-gisb-arb for newer 40nm chips
+brcm,bcm7400-gisb-arb for older 40nm chips and all 65nm chips
+brcm,bcm7038-gisb-arb for 130nm chips
- reg: specifies the base physical address and size of the registers
- interrupt-parent: specifies the phandle to the parent interrupt controller
this arbiter gets interrupt line from
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index ef1e423..172908d 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -47,6 +47,36 @@ enum {
ARB_ERR_CAP_MASTER,
};
+static const int gisb_offsets_bcm7038[] = {
+ [ARB_TIMER] = 0x00c,
+ [ARB_ERR_CAP_CLR] = 0x0c4,
+ [ARB_ERR_CAP_HI_ADDR] = -1,
+ [ARB_ERR_CAP_ADDR] = 0x0c8,
+ [ARB_ERR_CAP_DATA] = 0x0cc,
+ [ARB_ERR_CAP_STATUS]= 0x0d0,
+ [ARB_ERR_CAP_MASTER]= -1,
+};
+
+static const int gisb_offsets_bcm7400[] = {
+ [ARB_TIMER] = 0x00c,
+ [ARB_ERR_CAP_CLR] = 0x0c8,
+ [ARB_ERR_CAP_HI_ADDR] = -1,
+ [ARB_ERR_CAP_ADDR] = 0x0cc,
+ [ARB_ERR_CAP_DATA] = 0x0d0,
+ [ARB_ERR_CAP_STATUS]= 0x0d4,
+ [ARB_ERR_CAP_MASTER]= 0x0d8,
+};
+
+static const int gisb_offsets_bcm7435[] = {
+ [ARB_TIMER] = 0x00c,
+ [ARB_ERR_CAP_CLR] = 0x168,
+ [ARB_ERR_CAP_HI_ADDR] = -1,
+ [ARB_ERR_CAP_ADDR] = 0x16c,
+ [ARB_ERR_CAP_DATA] = 0x170,
+ [ARB_ERR_CAP_STATUS]= 0x174,
+ [ARB_ERR_CAP_MASTER]= 0x178,
+};
+
static const int gisb_offsets_bcm7445[] = {
[ARB_TIMER] = 0x008,
[ARB_ERR_CAP_CLR] = 0x7e4,
@@ -230,10 +260,20 @@ static struct attribute_group gisb_arb_sysfs_attr_group =
{
.attrs = gisb_arb_sysfs_attrs,
};
+static const struct of_device_id brcmstb_gisb_arb_of_match[] = {
+ { .compatible = brcm,gisb-arb, .data = gisb_offsets_bcm7445 },
+ { .compatible = brcm,bcm7445-gisb-arb, .data = gisb_offsets_bcm7445 },
+ { .compatible = brcm,bcm7435-gisb-arb, .data = gisb_offsets_bcm7435 },
+ { .compatible = brcm,bcm7400-gisb-arb, .data = gisb_offsets_bcm7400 },
+ { .compatible = brcm,bcm7038-gisb-arb, .data = gisb_offsets_bcm7038 },
+ { },
+};
+
static int brcmstb_gisb_arb_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev-dev.of_node;
struct brcmstb_gisb_arb_device *gdev;
+ const struct of_device_id *of_id;
struct resource *r;
int err, timeout_irq, tea_irq;
unsigned int num_masters, j = 0;
@@ -254,7 +294,12 @@ static int brcmstb_gisb_arb_probe(struct platform_device
*pdev)
if (IS_ERR(gdev-base))
return PTR_ERR(gdev-base);
- gdev-gisb_offsets = gisb_offsets_bcm7445;
+ of_id = of_match_node(brcmstb_gisb_arb_of_match, dn);
+ if (!of_id) {
+ pr_err(failed to look up compatible string\n);
+ return -EINVAL;
+ }
+ gdev-gisb_offsets = of_id-data;
err = devm_request_irq(pdev-dev, timeout_irq,
brcmstb_gisb_timeout_handler, 0, pdev-name,
@@ -307,11 +352,6 @@ static int brcmstb_gisb_arb_probe(struct platform_device
*pdev)
return 0;
}
-static const struct of_device_id brcmstb_gisb_arb_of_match[] = {
- { .compatible = brcm,gisb-arb },
- { },
-};
-
static struct platform_driver brcmstb_gisb_arb_driver = {
.probe = brcmstb_gisb_arb_probe,
.driver = {
--
2.1.0
--
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/9] power/reset: brcmstb: Add support for old 65nm chips
The register bit fields are a little different, so add an entry and a
compatible string to accommodate them.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
Documentation/devicetree/bindings/arm/brcm-brcmstb.txt | 4 +++-
drivers/power/reset/brcmstb-reboot.c | 6 ++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
index 3c436cc..430608e 100644
--- a/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
+++ b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
@@ -79,7 +79,9 @@ reboot
Required properties
- compatible
-The string property brcm,brcmstb-reboot.
+The string property brcm,brcmstb-reboot for 40nm/28nm chips with
+the new SYS_CTRL interface, or brcm,bcm7038-reboot for 65nm
+chips with the old SUN_TOP_CTRL interface.
- syscon
A phandle / integer array that points to the syscon node which
describes
diff --git a/drivers/power/reset/brcmstb-reboot.c
b/drivers/power/reset/brcmstb-reboot.c
index 4e61c3f..33af4f3 100644
--- a/drivers/power/reset/brcmstb-reboot.c
+++ b/drivers/power/reset/brcmstb-reboot.c
@@ -88,8 +88,14 @@ static const struct reset_reg_mask reset_bits_40nm = {
.sw_mstr_rst_mask = BIT(0),
};
+static const struct reset_reg_mask reset_bits_65nm = {
+ .rst_src_en_mask = BIT(3),
+ .sw_mstr_rst_mask = BIT(31),
+};
+
static const struct of_device_id of_match[] = {
{ .compatible = brcm,brcmstb-reboot, .data = reset_bits_40nm },
+ { .compatible = brcm,bcm7038-reboot, .data = reset_bits_65nm },
{},
};
--
2.1.0
--
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/9] bus: brcmstb_gisb: Introduce wrapper functions for MMIO accesses
These will be used to abstract out chip-to-chip differences.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/bus/brcmstb_gisb.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 5da935a..8ff403d 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -54,6 +54,16 @@ struct brcmstb_gisb_arb_device {
static LIST_HEAD(brcmstb_gisb_arb_device_list);
+static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg)
+{
+ return ioread32(gdev-base + reg);
+}
+
+static void gisb_write(struct brcmstb_gisb_arb_device *gdev, u32 val, int reg)
+{
+ iowrite32(val, gdev-base + reg);
+}
+
static ssize_t gisb_arb_get_timeout(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -63,7 +73,7 @@ static ssize_t gisb_arb_get_timeout(struct device *dev,
u32 timeout;
mutex_lock(gdev-lock);
- timeout = ioread32(gdev-base + ARB_TIMER);
+ timeout = gisb_read(gdev, ARB_TIMER);
mutex_unlock(gdev-lock);
return sprintf(buf, %d, timeout);
@@ -85,7 +95,7 @@ static ssize_t gisb_arb_set_timeout(struct device *dev,
return -EINVAL;
mutex_lock(gdev-lock);
- iowrite32(val, gdev-base + ARB_TIMER);
+ gisb_write(gdev, val, ARB_TIMER);
mutex_unlock(gdev-lock);
return count;
@@ -112,18 +122,18 @@ static int brcmstb_gisb_arb_decode_addr(struct
brcmstb_gisb_arb_device *gdev,
const char *m_name;
char m_fmt[11];
- cap_status = ioread32(gdev-base + ARB_ERR_CAP_STATUS);
+ cap_status = gisb_read(gdev, ARB_ERR_CAP_STATUS);
/* Invalid captured address, bail out */
if (!(cap_status ARB_ERR_CAP_STATUS_VALID))
return 1;
/* Read the address and master */
- arb_addr = ioread32(gdev-base + ARB_ERR_CAP_ADDR) 0x;
+ arb_addr = gisb_read(gdev, ARB_ERR_CAP_ADDR) 0x;
#if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
- arb_addr |= (u64)ioread32(gdev-base + ARB_ERR_CAP_HI_ADDR) 32;
+ arb_addr |= (u64)gisb_read(gdev, ARB_ERR_CAP_HI_ADDR) 32;
#endif
- master = ioread32(gdev-base + ARB_ERR_CAP_MASTER);
+ master = gisb_read(gdev, ARB_ERR_CAP_MASTER);
m_name = brcmstb_gisb_master_to_str(gdev, master);
if (!m_name) {
@@ -138,7 +148,7 @@ static int brcmstb_gisb_arb_decode_addr(struct
brcmstb_gisb_arb_device *gdev,
m_name);
/* clear the GISB error */
- iowrite32(ARB_ERR_CAP_CLEAR, gdev-base + ARB_ERR_CAP_CLR);
+ gisb_write(gdev, ARB_ERR_CAP_CLEAR, ARB_ERR_CAP_CLR);
return 0;
}
--
2.1.0
--
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 8/9] bus: brcmstb_gisb: Honor the big-endian and native-endian DT properties
On chips strapped for BE, we'll need to use ioread32be/iowrite32be instead of
ioread32/iowrite32.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/bus/brcmstb_gisb.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 172908d..969b992 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -90,6 +90,7 @@ static const int gisb_offsets_bcm7445[] = {
struct brcmstb_gisb_arb_device {
void __iomem*base;
const int *gisb_offsets;
+ boolbig_endian;
struct mutexlock;
struct list_head next;
u32 valid_mask;
@@ -106,7 +107,10 @@ static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev,
int reg)
if (offset == -1)
return 1;
- return ioread32(gdev-base + offset);
+ if (gdev-big_endian)
+ return ioread32be(gdev-base + offset);
+ else
+ return ioread32(gdev-base + offset);
}
static void gisb_write(struct brcmstb_gisb_arb_device *gdev, u32 val, int reg)
@@ -115,7 +119,11 @@ static void gisb_write(struct brcmstb_gisb_arb_device
*gdev, u32 val, int reg)
if (offset == -1)
return;
- iowrite32(val, gdev-base + reg);
+
+ if (gdev-big_endian)
+ iowrite32be(val, gdev-base + reg);
+ else
+ iowrite32(val, gdev-base + reg);
}
static ssize_t gisb_arb_get_timeout(struct device *dev,
@@ -300,6 +308,7 @@ static int brcmstb_gisb_arb_probe(struct platform_device
*pdev)
return -EINVAL;
}
gdev-gisb_offsets = of_id-data;
+ gdev-big_endian = of_device_is_big_endian(dn);
err = devm_request_irq(pdev-dev, timeout_irq,
brcmstb_gisb_timeout_handler, 0, pdev-name,
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] USB: ohci-platform: Expose no_big_frame_no and num_ports in DT
On Tue, Oct 14, 2014 at 6:58 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sat, 11 Oct 2014, Kevin Cernekee wrote: These quirks are currently set through platform_data; allow DT-based SoCs to use them too. Signed-off-by: Kevin Cernekee cerne...@gmail.com --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -175,6 +175,12 @@ static int ohci_platform_probe(struct platform_device *dev) if (of_property_read_bool(dev-dev.of_node, big-endian)) ohci-flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC; + if (of_property_read_bool(dev-dev.of_node, no-big-frame-no)) + ohci-flags |= OHCI_QUIRK_FRAME_NO; + + of_property_read_u32(dev-dev.of_node, num-ports, + ohci-num_ports); Does this do the right thing if the property isn't defined? In that case, of_property_read_u32() will just return -EINVAL or -ENODATA without writing anything to ohci-num_ports, leaving the default of 0 in place. -- 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] USB: OHCI: Eliminate platform-specific test in ohci.h
On Sat, Oct 11, 2014 at 7:58 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 10 Oct 2014, Kevin Cernekee wrote: OHCI_QUIRK_FRAME_NO is currently set under either of the following conditions: 1) If a ppc-of-ohci DT node indicates a compatible string of fsl,mpc5200-ohci or mpc5200-ohci 2) If usb_ohci_pdata-no_big_frame_no is set For #1, the affected platforms already enable CONFIG_PPC_MPC52xx. For #2, there are currently no in-tree users. So we can safely remove the #ifdef, and thereby allow OHCI_QUIRK_FRAME_NO to be used by other (non-PPC) platforms that have the same property. bcm63xx and bcm3384 are two such users. Sorry, but I can't understand this patch description. What #ifdef does it refer to? It refers to #ifdef CONFIG_PPC_MPC52xx. This check appears to be redundant: from what I can tell, it isn't ever necessary to gate the workaround logic based on CONFIG_PPC_MPC52xx, because currently OHCI_QUIRK_FRAME_NO is only getting set for (some) mpc52xx chips. So, removing #ifdef CONFIG_PPC_MPC52xx both eliminates PPC-specific code from the generic driver, and allows bcm63xx/bcm3384 to make use of OHCI_QUIRK_FRAME_NO. By comparing the description with the patch, it looks like you _wanted_ to say something along these lines: The bcm63xx and bcm3384 platforms need to set OHCI_QUIRK_FRAME_NO, but they are non-PPC platforms and don't enable CONFIG_PPC_MPC52xx. Therefore this patch changes the code that uses OHCI_QUIRK_FRAME_NO, making it not depend on CONFIG_PPC_MPC52xx. Does that properly describe what you are doing? Yes. -- 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] USB: ohci-platform: Expose no_big_frame_no and num_ports in DT
On Sat, Oct 11, 2014 at 8:09 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 10 Oct 2014, Kevin Cernekee wrote: These quirks are currently set through platform_data; allow DT-based SoCs to use them too. It looks strange to have the platform_data version of the quirks set in one routine and the DT version set in a different routine. Is there any reason not to set all of them in ohci_platform_probe? That would allow us to eliminate ohci_platform_reset. I think it is mostly for historical reasons. In Hauke's original driver submission (commit fa3364b5a2d79), all of the platform_data checks were in ohci_platform_reset(). Prior to commit 928fb68e2357be (make ohci-platform a separate driver) it looks like there were some ordering dependencies involving calls to ohci_hcd_init(): commit 2b16e39ee0a431d6cf6e6ca33bb08ec7dc82073f Author: Florian Fainelli flor...@openwrt.org Date: Mon Oct 8 15:11:26 2012 +0200 USB: ohci: allow platform driver to specify the number of ports This patch modifies the ohci platform driver to accept the num_ports parameter to be set via platform_data. Setting the number of ports must be done after the call to ohci_hcd_init(). But that doesn't seem to be the case anymore, and in my tests with the DT num-ports patch, I never saw ohci-num_ports getting overwritten. Would you like me to submit another patch to move the remaining platform_data tests from ohci_platform_reset() into ohci_platform_probe()? -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] USB: OHCI: Eliminate platform-specific test in ohci.h
The bcm63xx and bcm3384 platforms need to set OHCI_QUIRK_FRAME_NO, but
they are non-PPC platforms and don't enable CONFIG_PPC_MPC52xx.
Therefore this patch changes the code that uses OHCI_QUIRK_FRAME_NO,
making it not depend on CONFIG_PPC_MPC52xx.
Also, rephrase the comments describing OHCI_QUIRK_FRAME_NO and the
related PSW endian swap.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/usb/host/ohci.h | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
v1-v2:
PATCH 1/3: Tweak description; no change to the code.
PATCH 2/3: No changes.
PATCH 3/3: New patch to remove ohci_platform_reset().
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 59f4245..bc46228 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -647,23 +647,22 @@ static inline u32 hc32_to_cpup (const struct ohci_hcd
*ohci, const __hc32 *x)
/*-*/
-/* HCCA frame number is 16 bits, but is accessed as 32 bits since not all
- * hardware handles 16 bit reads. That creates a different confusion on
- * some big-endian SOC implementations. Same thing happens with PSW access.
+/*
+ * The HCCA frame number is 16 bits, but is accessed as 32 bits since not all
+ * hardware handles 16 bit reads. Depending on the SoC implementation, the
+ * frame number can wind up in either bits [31:16] (default) or
+ * [15:0] (OHCI_QUIRK_FRAME_NO) on big endian hosts.
+ *
+ * Somewhat similarly, the 16-bit PSW fields in a transfer descriptor are
+ * reordered on BE.
*/
-#ifdef CONFIG_PPC_MPC52xx
-#define big_endian_frame_no_quirk(ohci)(ohci-flags
OHCI_QUIRK_FRAME_NO)
-#else
-#define big_endian_frame_no_quirk(ohci)0
-#endif
-
static inline u16 ohci_frame_no(const struct ohci_hcd *ohci)
{
u32 tmp;
if (big_endian_desc(ohci)) {
tmp = be32_to_cpup((__force __be32 *)ohci-hcca-frame_no);
- if (!big_endian_frame_no_quirk(ohci))
+ if (!(ohci-flags OHCI_QUIRK_FRAME_NO))
tmp = 16;
} else
tmp = le32_to_cpup((__force __le32 *)ohci-hcca-frame_no);
--
2.1.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 v2 2/3] USB: ohci-platform: Expose no_big_frame_no and num_ports in DT
These quirks are currently set through platform_data; allow DT-based SoCs
to use them too.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
Documentation/devicetree/bindings/usb/usb-ohci.txt | 2 ++
drivers/usb/host/ohci-platform.c | 6 ++
2 files changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt
b/Documentation/devicetree/bindings/usb/usb-ohci.txt
index b968a1a..19233b7 100644
--- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -9,6 +9,8 @@ Optional properties:
- big-endian-regs : boolean, set this for hcds with big-endian registers
- big-endian-desc : boolean, set this for hcds with big-endian descriptors
- big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
+- no-big-frame-no : boolean, set if frame_no lives in bits [15:0] of HCCA
+- num-ports : u32, to override the detected port count
- clocks : a list of phandle + clock specifier pairs
- phys : phandle + phy specifier pair
- phy-names : usb
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 4369299..6fb03f8 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -175,6 +175,12 @@ static int ohci_platform_probe(struct platform_device *dev)
if (of_property_read_bool(dev-dev.of_node, big-endian))
ohci-flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC;
+ if (of_property_read_bool(dev-dev.of_node, no-big-frame-no))
+ ohci-flags |= OHCI_QUIRK_FRAME_NO;
+
+ of_property_read_u32(dev-dev.of_node, num-ports,
+ohci-num_ports);
+
priv-phy = devm_phy_get(dev-dev, usb);
if (IS_ERR(priv-phy)) {
err = PTR_ERR(priv-phy);
--
2.1.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 v2 3/3] USB: ohci-platform: Move platform_data checks into the probe function
This puts all of the platform_data checks in the same place, and removes
the need for a custom drv-reset() callback.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/usb/host/ohci-platform.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 6fb03f8..7793c3c 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -43,20 +43,6 @@ struct ohci_platform_priv {
static const char hcd_name[] = ohci-platform;
-static int ohci_platform_reset(struct usb_hcd *hcd)
-{
- struct platform_device *pdev = to_platform_device(hcd-self.controller);
- struct usb_ohci_pdata *pdata = dev_get_platdata(pdev-dev);
- struct ohci_hcd *ohci = hcd_to_ohci(hcd);
-
- if (pdata-no_big_frame_no)
- ohci-flags |= OHCI_QUIRK_FRAME_NO;
- if (pdata-num_ports)
- ohci-num_ports = pdata-num_ports;
-
- return ohci_setup(hcd);
-}
-
static int ohci_platform_power_on(struct platform_device *dev)
{
struct usb_hcd *hcd = platform_get_drvdata(dev);
@@ -110,7 +96,6 @@ static struct hc_driver __read_mostly
ohci_platform_hc_driver;
static const struct ohci_driver_overrides platform_overrides __initconst = {
.product_desc = Generic Platform OHCI controller,
- .reset =ohci_platform_reset,
.extra_priv_size = sizeof(struct ohci_platform_priv),
};
@@ -218,6 +203,10 @@ static int ohci_platform_probe(struct platform_device *dev)
ohci-flags |= OHCI_QUIRK_BE_DESC;
if (pdata-big_endian_mmio)
ohci-flags |= OHCI_QUIRK_BE_MMIO;
+ if (pdata-no_big_frame_no)
+ ohci-flags |= OHCI_QUIRK_FRAME_NO;
+ if (pdata-num_ports)
+ ohci-num_ports = pdata-num_ports;
#ifndef CONFIG_USB_OHCI_BIG_ENDIAN_MMIO
if (ohci-flags OHCI_QUIRK_BE_MMIO) {
--
2.1.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] USB: ohci-platform: Expose no_big_frame_no and num_ports in DT
These quirks are currently set through platform_data; allow DT-based SoCs
to use them too.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
Documentation/devicetree/bindings/usb/usb-ohci.txt | 2 ++
drivers/usb/host/ohci-platform.c | 6 ++
2 files changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt
b/Documentation/devicetree/bindings/usb/usb-ohci.txt
index b968a1a..19233b7 100644
--- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -9,6 +9,8 @@ Optional properties:
- big-endian-regs : boolean, set this for hcds with big-endian registers
- big-endian-desc : boolean, set this for hcds with big-endian descriptors
- big-endian : boolean, for hcds with big-endian-regs + big-endian-desc
+- no-big-frame-no : boolean, set if frame_no lives in bits [15:0] of HCCA
+- num-ports : u32, to override the detected port count
- clocks : a list of phandle + clock specifier pairs
- phys : phandle + phy specifier pair
- phy-names : usb
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 4369299..6fb03f8 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -175,6 +175,12 @@ static int ohci_platform_probe(struct platform_device *dev)
if (of_property_read_bool(dev-dev.of_node, big-endian))
ohci-flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC;
+ if (of_property_read_bool(dev-dev.of_node, no-big-frame-no))
+ ohci-flags |= OHCI_QUIRK_FRAME_NO;
+
+ of_property_read_u32(dev-dev.of_node, num-ports,
+ohci-num_ports);
+
priv-phy = devm_phy_get(dev-dev, usb);
if (IS_ERR(priv-phy)) {
err = PTR_ERR(priv-phy);
--
2.1.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 1/2] USB: OHCI: Eliminate platform-specific test in ohci.h
OHCI_QUIRK_FRAME_NO is currently set under either of the following
conditions:
1) If a ppc-of-ohci DT node indicates a compatible string of
fsl,mpc5200-ohci or mpc5200-ohci
2) If usb_ohci_pdata-no_big_frame_no is set
For #1, the affected platforms already enable CONFIG_PPC_MPC52xx.
For #2, there are currently no in-tree users.
So we can safely remove the #ifdef, and thereby allow OHCI_QUIRK_FRAME_NO
to be used by other (non-PPC) platforms that have the same property.
bcm63xx and bcm3384 are two such users.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/usb/host/ohci.h | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 59f4245..bc46228 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -647,23 +647,22 @@ static inline u32 hc32_to_cpup (const struct ohci_hcd
*ohci, const __hc32 *x)
/*-*/
-/* HCCA frame number is 16 bits, but is accessed as 32 bits since not all
- * hardware handles 16 bit reads. That creates a different confusion on
- * some big-endian SOC implementations. Same thing happens with PSW access.
+/*
+ * The HCCA frame number is 16 bits, but is accessed as 32 bits since not all
+ * hardware handles 16 bit reads. Depending on the SoC implementation, the
+ * frame number can wind up in either bits [31:16] (default) or
+ * [15:0] (OHCI_QUIRK_FRAME_NO) on big endian hosts.
+ *
+ * Somewhat similarly, the 16-bit PSW fields in a transfer descriptor are
+ * reordered on BE.
*/
-#ifdef CONFIG_PPC_MPC52xx
-#define big_endian_frame_no_quirk(ohci)(ohci-flags
OHCI_QUIRK_FRAME_NO)
-#else
-#define big_endian_frame_no_quirk(ohci)0
-#endif
-
static inline u16 ohci_frame_no(const struct ohci_hcd *ohci)
{
u32 tmp;
if (big_endian_desc(ohci)) {
tmp = be32_to_cpup((__force __be32 *)ohci-hcca-frame_no);
- if (!big_endian_frame_no_quirk(ohci))
+ if (!(ohci-flags OHCI_QUIRK_FRAME_NO))
tmp = 16;
} else
tmp = le32_to_cpup((__force __le32 *)ohci-hcca-frame_no);
--
2.1.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB 3380 using net2280 driver
On Tue, Feb 18, 2014 at 7:12 AM, Felipe Balbi ba...@ti.com wrote: On Mon, Feb 17, 2014 at 04:43:11PM -0800, Amit Uttamchandani wrote: I was looking at the changes for net2280.c and saw your name come up in a few of the more recent changes. I wanted to know, are you aware of support for PLXs USB 338o using this net2280 driver? no, those are two completely different controllers. Linux doesn't support USB 3380 as of today. It might be possible to use this code as a starting point: http://patchwork.openwrt.org/patch/2889/ -- 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] usb: gadget: bcm63xx UDC driver
On Mon, Aug 27, 2012 at 1:46 PM, Sebastian Andrzej Siewior One little question: Felipe suggested to replace the workqueue by a threaded interrupt. You schedule the workqueue in interrupt context and once in ep0 enqueue. The enqueue should be fine by executing one round and waiting for the interrupt. Any reason why you suggested against it? A couple of rounds could pass with no interrupt, e.g. if a SET_CONFIGURATION request and a SET_INTERFACE request are both pending. Also, I ran into deadlocks when trying to invoke the gadget driver's callback from within the UDC enqueue function. I did attempt it; V2 of the patch had the workqueue removed, but I backed out the change for V3 after seeing so many problems. -- 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] usb: gadget: bcm63xx UDC driver
On Wed, Aug 22, 2012 at 12:48 AM, Sebastian Andrzej Siewior sebast...@breakpoint.cc wrote: Just one thing that bit while I was sleeping: The HW acks SetConfig on its own. Once you notice this, you set -ep0_req_set_cfg and set state in bcm63xx_ep0_do_idle() to EP0_IN_FAKE_STATUS_PHASE. This is I guess the workaround for mass_storage's hold with DELAYED_STATUS and continues with a zero packet. EP0_IN_FAKE_STATUS_PHASE is there for the general case of: setup() callback returned = 0 after a spoofed setup packet, and we're waiting for the gadget driver to send the 0-byte status reply so we can silently discard it and move on. When bcm63xx_udc is in EP0_IN_FAKE_STATUS_PHASE, it won't issue any more setup() callbacks until the 0-byte reply arrives from the gadget driver. If the host sends a setup request, the callback will be held off until after the (unused) status reply. This keeps the gadget driver from getting confused by out-of-sequence events. Now two questions: - If a gadget descides not NAK / stall the SetConfig requests. What happens here? If the return value from the setup() callback was negative, bcm63xx_udc should just return to EP0_IDLE as the gadget driver will never send a 0-byte reply. I have added a new check for this condition, verified that it works as intended, and posted V4. I am hoping that these invalid SET_CONFIGURATION / SET_INTERFACE requests are uncommon. In what sorts of situations will a host request a configuration that isn't advertised in the device's descriptors? I had trouble just convincing usb_set_interface() / usb_driver_set_configuration() to send such a request because they honor bInterfaceNumber / bConfigurationValue from the descriptors. - What happens if the host is faster than the UDC. SetConfig returns in usb-storage with DELAYED_STATUS. HW Acks this. Could the Host send another request before the gadget queues the ep0 request? Could you please clarify if this is the sequence of events you are describing: 1) Host sends a valid SET_CONFIGURATION request to a mass storage gadget 2) Hardware instantly auto-acks the request, completing the status phase and allowing the host to proceed with another ep0 request 3) bcm63xx_udc sends a spoofed SET_CONFIGURATION setup packet to the gadget driver 4) setup() callback returns USB_GADGET_DELAYED_STATUS (0x7fff) but doesn't queue up a reply 5) Host sends another setup packet before usb_composite_setup_continue() is called to send the 0-byte status reply If so, the next steps should look like: 6) bcm63xx_udc takes a data IRQ, and sets ep0_req_completed 7) bcm63xx_udc stays in EP0_IN_FAKE_STATUS_PHASE until the 0-byte reply is received from usb_composite_setup_continue() 8) usb_composite_setup_continue() eventually sends the 0-byte reply 9) bcm63xx_udc returns to EP0_IDLE and notices that ep0_req_completed is now set 10) bcm63xx_ep0_do_setup() looks at the new request, and performs the setup() callback for the new setup request 11) Data/status phases are handled as usual -- 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: gadget: bcm63xx UDC driver
On Tue, Aug 21, 2012 at 5:04 AM, Felipe Balbi ba...@ti.com wrote: On Mon, Aug 20, 2012 at 08:48:11PM -0700, Kevin Cernekee wrote: On Mon, Aug 20, 2012 at 12:40 AM, Felipe Balbi ba...@ti.com wrote: no workqueues, please either handle the IRQ here or use threaded_irqs. again, no workqueues. Felipe, I am seeing all sorts of deadlocks now, after removing the workqueue (patch V2). Some have easy fixes, but for others it is not as obvious. The code was much simpler when I could just trigger a deferred worker function. Workqueues are used in at91_udc, lpc32xx_udc, mv_udc_core, and pch_udc. Could you please clarify why it is not OK to use one in bcm63xx_udc? Because threaded_irqs were added in order to drop such workqueues. threaded_irqs also have the highest priority possible (only lower than hardirq handlers), so you'll get scheduled much sooner. Could it be that most of your deadlocks is because you're not setting IRQF_ONESHOT ? The deadlocks involve ep0 processing that is triggered through bcm63xx_udc_queue(). e.g. gadget driver queues a new request, it's a reply to a spoofed SET_CONFIGURATION / SET_INTERFACE transaction, and the UDC driver calls the completion immediately. So, not all of the ep0 work is being done in response to an IRQ from the controller HW, and I think that is where this driver diverges from most of the others. Would it be OK to use a workqueue, or maybe a tasklet, given these circumstances? Thanks. -- 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: gadget: bcm63xx UDC driver
On Tue, Aug 21, 2012 at 1:34 PM, Alan Stern st...@rowland.harvard.edu wrote: It is a silicon feature: the core will intercept SET_CONFIGURATION / SET_INTERFACE requests, store wValue/wIndex in the appropriate USBD_STATUS_REG field (cfg/intf/altintf), send an acknowledgement to the host, and raise a control interrupt. Your explanation is not clear. The operations you listed are exactly what any UDC should do when it receives any control request: It should store the bRequestType, bRequest, wValue, wIndex, and wLength values in appropriate registers, send an ACK back to the host, and generate an IRQ. What's special about Set-Config and Set-Interface? For most control requests (such as GET_DESCRIPTOR), this core writes the raw packet data out to a buffer in DRAM and raises an IUDMA IRQ. The UDC driver passes this data on to the gadget driver and allows it to decide what to send back in subsequent phases. But some requests are handled entirely in hardware, including the status phase: SET_ADDRESS SET_CONFIGURATION SET_INTERFACE SET_FEATURE CLEAR_FEATURE GET_FEATURE Where appropriate, the hardware block will update its registers to indicate the new settings and raise a control IRQ. I haven't found it to be terribly helpful, but I don't know of a way to turn it off. Why would you want to turn this off? Isn't is exactly what you want to have happen? And why do you need a workqueue to handle the request? SET_CONFIGURATION and SET_INTERFACE need to generate setup callbacks for the gadget driver, and the proper ordering of events needs to be enforced. It is possible to receive IRQs for SET_CONFIGURATION has happened, SET_INTERFACE has happened, and there is another control request pending, all at nearly the same time, before the gadget driver has had an opportunity to respond to the first setup request. But if that happened, we would not want to generate three setup callbacks in a row. Example: 1) SET_CONFIGURATION transfer completes (including status phase), all handled by hardware. Control interrupt is raised. 2) Setup packet is sent to USB gadget driver (say, gadgetfs) 3) SET_INTERFACE transfer completes, and another control interrupt is raised 4) Gadget driver eventually gets around to queuing the SET_CONFIGURATION reply. 5) Now we are in the UDC queue function and the gadget driver is probably holding a spinlock. We want to invoke the setup callback with the spoofed SET_INTERFACE packet, but it's not safe to do it from here. To complicate the situation, another setup packet can arrive between steps 3 and 4. But the worker function makes sure everything plays out in the right order. -- 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: gadget: bcm63xx UDC driver
On Tue, Aug 21, 2012 at 2:34 PM, Alan Stern st...@rowland.harvard.edu wrote: But some requests are handled entirely in hardware, including the status phase: SET_ADDRESS SET_CONFIGURATION SET_INTERFACE SET_FEATURE CLEAR_FEATURE GET_FEATURE I assume the features in question are endpoint-halts and remote-wakeup-enable. It's hard to think of any other features that could be handled in hardware. Yes, endpoint-halt is what I've seen in my testing. Not sure on remote wakeup. (BTW, what happens if one of these requests arrives before a previous control IRQ has been acknowledged by the CPU? Does the old data in the registers get overwritten and lost, or does the UDC hold off on carrying out the new request?) There are separate IRQ bits for SET_CONFIGURATION, SET_INTERFACE, and generic setup packet. The driver processes them in order. Consecutive SET_INTERFACE requests could potentially get lost, but the controller will remember the value set in the most recent request. I can't imagine how any gadget driver could hope to handle a mess like this. For instance, suppose a Set-Config operation fails. There's no way for the driver to tell the host, because the hardware has already said that the operation succeeded. Right - but looking on the bright side: 1) At least in my experience, most hosts will send a valid SET_CONFIGURATION request so the error path won't be needed. If it is needed, it won't do anything really bad like hanging the interface. 2) All of this complexity is hidden inside one UDC driver, and did not require hacking up the rest of the gadget layer. 3) After many months of struggling to get things right, the driver can finally pass testusb. Anyway, you don't need to workqueue to manage this -- just a queue of pending control requests. You should be able to do everything in interrupt context, especially since you will ignore any responses the driver submits on ep0 for these transfers. Well, in the example from my last email, I don't get any new interrupts after SET_INTERFACE. So after the gadget driver queues the response to the spoofed SET_CONFIGURATION packet, I need to schedule a workqueue (or tasklet, or timer, or something) to run the next setup callback in a context where the gadget driver isn't holding any spinlocks. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] usb: gadget: bcm63xx UDC driver
On Tue, Aug 21, 2012 at 2:13 PM, Sebastian Andrzej Siewior sebast...@breakpoint.cc wrote: please drop the force all gadget drivers to also be dynamically linked part. We may want to change this one day :) Done in V3 + if (!pd-use_fullspeed !use_fullspeed) so it is a good advice to have pd not set to NULL :) In V3 the probe function will call dev_err() and exit, instead of crashing. I added a comment explaining why it's mandatory: the hardware requires specifying which port is shared between the device core + host core, and if we just assume the wrong value, the port won't pass traffic and it won't be obvious why. +static bool use_fullspeed; +module_param(use_fullspeed, bool, S_IRUGO); +MODULE_PARM_DESC(use_fullspeed, true for fullspeed only); How important is this option? Maybe this should become a generic option? In theory, the hardware core should always be able to support USB 2.0 without any problems, and bcm63xx_udc.c should always be used with gadget drivers that can handle USB 2.0. In practice, there are a number of boards floating around which are only reliable at USB 1.1 speeds, either due to impedance problems or noise. These fall into two categories: 1) Boards that have their own board definition in board_bcm963xx.c, and pass this ID to Linux - in this case use_fullspeed can be specified right in the platform data. 2) Products that copied the generic board name from the reference design. In this case they'll need another way of overriding use_fullspeed, because the reference board supports high speed. Some might just add bcm63xx_udc.use_fullspeed=1 to CONFIG_CMDLINE. Others have to detect the board revision through other means (such as reading a GPIO) and then pass the appropriate option to insmod. use_fullspeed=1 just tells the core not to negotiate USB 2.0 PHY rates. It should be roughly equivalent to plugging the device into a full speed hub. -- 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: gadget: bcm63xx UDC driver
On Mon, Aug 20, 2012 at 12:40 AM, Felipe Balbi ba...@ti.com wrote: no workqueues, please either handle the IRQ here or use threaded_irqs. again, no workqueues. Felipe, I am seeing all sorts of deadlocks now, after removing the workqueue (patch V2). Some have easy fixes, but for others it is not as obvious. The code was much simpler when I could just trigger a deferred worker function. Workqueues are used in at91_udc, lpc32xx_udc, mv_udc_core, and pch_udc. Could you please clarify why it is not OK to use one in bcm63xx_udc? Thanks. -- 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 2/2] usb: gadget: u_ether: Don't change endpoint status in eth_stop()
On Thu, Aug 9, 2012 at 1:26 AM, Felipe Balbi ba...@ti.com wrote: I have applied this one instead: commit a3ab51013e6af38b3e2f9f20bf469cf8c595391b Author: Michael Grzeschik m.grzesc...@pengutronix.de Date: Wed Aug 8 11:48:10 2012 +0200 usb: gadget: u_ether: fix kworker 100% CPU issue with still used interfaces in eth_stop This patch fixes an issue introduced by patch: 72c973d usb: gadget: add usb_endpoint_descriptor to struct usb_ep Without this patch we see a kworker taking 100% CPU, after this sequence: - Connect gadget to a windows host - load g_ether - ifconfig up ip; ifconfig down; ifconfig up - ping windows host The ifconfig down results in calling eth_stop(), which will call usb_ep_disable() and, if the carrier is still ok, usb_ep_enable(): [snip] does it solve the issue you found ? I have verified that it solves the simple up/down/up kworker hang, which was the only problem I actually saw on my board. So for that problem: Tested-by: Kevin Cernekee cerne...@gmail.com I'm not sure if it solves the various other (hypothetical?) races in u_ether between up/down and connect/disconnect. It might be interesting to run ifconfig up/down in a loop overnight, while toggling VBUS every couple of seconds. -- 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: gadget: udc-core: Race between disconnect/unbind and setup
On Mon, Jul 9, 2012 at 7:15 AM, Alan Stern st...@rowland.harvard.edu wrote:
What I see is that usb_gadget_disconnect() does tell the UDC driver to
stop activity on the port, but that only happens after
composite_disconnect() was called. Can you confirm that the order of
the calls in usb_gadget_remove_driver() is correct?
They might need to be changed. It does seems like a bad idea to tell
the gadget driver that a disconnect occurred before the host knows
about it.
If you change the order of those two calls, does that make your posted
patch unnecessary?
Yes, that works too. Here is the change:
-- 8 --
From: Kevin Cernekee cerne...@gmail.com
usb_gadget_remove_driver() runs through a four-step sequence to shut down
the gadget driver. For the case of a composite gadget + at91 UDC, this
would look like:
udc-driver-disconnect(udc-gadget); // composite_disconnect()
usb_gadget_disconnect(udc-gadget);// at91_pullup(gadget, 0)
udc-driver-unbind(udc-gadget); // composite_unbind()
usb_gadget_udc_stop(udc-gadget, udc-driver); // at91_stop()
The UDC driver can receive SETUP packets from the host up until the
point when usb_gadget_disconnect() returns. On rare occasions, the
gadget driver may see this sequence:
udc-driver-disconnect(udc-gadget); // composite_disconnect()
udc-driver-setup(udc-gadget, ctrl);// composite_setup()
udc-driver-unbind(udc-gadget); // composite_unbind()
Some gadget drivers, such as composite, assume this will never happen
and crash as a result.
The fix is to quiesce the UDC hardware (via usb_gadget_disconnect)
before running the gadget driver through the disconnect/unbind sequence.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/usb/gadget/udc-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index e5e44f8..bae243c 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -262,8 +262,8 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
kobject_uevent(udc-dev.kobj, KOBJ_CHANGE);
if (udc_is_newstyle(udc)) {
- udc-driver-disconnect(udc-gadget);
usb_gadget_disconnect(udc-gadget);
+ udc-driver-disconnect(udc-gadget);
udc-driver-unbind(udc-gadget);
usb_gadget_udc_stop(udc-gadget, udc-driver);
} else {
--
1.7.11.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/RFC 2/2] usb: gadget: u_ether: Don't change endpoint status in eth_stop()
This is a follow-up to an earlier thread: gadget: Clearing ep-desc in
struct usb_ep on EP disable?
I took a closer look at u_ether.c and I had some concerns about the
synchronization between USB connection/disconnection vs. netdev
open/close. For instance:
If somebody yanks the USB cable in the middle of eth_open() -
eth_start() - rx_fill(), triggering gether_disconnect() and
usb_ep_disable() on another CPU, will rx_fill() abort gracefully when
its requests start getting rejected? It looks like rx_fill() just
assumes that if rx_submit() returns an error, it is out of memory so it
schedules a worker to try again. That might be a bad idea if the
endpoint was disabled.
If somebody hotplugs the USB cable in the middle of eth_stop(),
triggering gether_connect(), is there a chance that the endpoints might
wind up disabled (and stay that way until eth_stop() executes again)?
Is there any chance of eth_start() never getting called? Both
eth_stop() and gether_connect() acquire dev-lock, but it doesn't
protect the entire usb_ep_enable()...netif_carrier_on() sequence.
Maybe as a first step, it is best to just eliminate the usb_ep_disable()
/ usb_ep_enable() code from eth_stop() entirely, and let the
connect/disconnect functions worry about the endpoint state.
-- 8 --
From: Kevin Cernekee cerne...@gmail.com
eth_stop() calls usb_ep_disable() then usb_ep_enable(), to flush out any
I/O in progress. There are a couple of issues with this:
1) UDC drivers can clear ep-desc on EP disable, causing usb_ep_enable()
to fail. This causes rx_fill() to start looping, and it makes the
system (mostly) non-responsive.
2) gether_connect() / gether_disconnect() also modify the endpoint
state, and they aren't holding dev-lock during ALL of the critical
operations. It may be possible for the endpoints to wind up in an
incorrect state if these events happen in the wrong order.
3) It probably isn't even necessary to flush these transactions unless
the USB link drops, and that case is already covered in
gether_disconnect().
For now, let's just delete these calls.
Signed-off-by: Kevin Cernekee cerne...@gmail.com
---
drivers/usb/gadget/u_ether.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 5b46f02..0a30f23 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -672,23 +672,6 @@ static int eth_stop(struct net_device *net)
if (link-close)
link-close(link);
-
- /* NOTE: we have no abort-queue primitive we could use
-* to cancel all pending I/O. Instead, we disable then
-* reenable the endpoints ... this idiom may leave toggle
-* wrong, but that's a self-correcting error.
-*
-* REVISIT: we *COULD* just let the transfers complete at
-* their own pace; the network stack can handle old packets.
-* For the moment we leave this here, since it works.
-*/
- usb_ep_disable(link-in_ep);
- usb_ep_disable(link-out_ep);
- if (netif_carrier_ok(net)) {
- DBG(dev, host still using in/out endpoints\n);
- usb_ep_enable(link-in_ep);
- usb_ep_enable(link-out_ep);
- }
}
spin_unlock_irqrestore(dev-lock, flags);
--
1.7.11.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] usb: gadget: composite: Race between disconnect/unbind and setup
On Sun, Jul 8, 2012 at 5:33 PM, Alan Stern st...@rowland.harvard.edu wrote: On Sun, 8 Jul 2012, Kevin Cernekee wrote: usb_gadget_remove_driver() runs through a four-step sequence to shut down the gadget driver. For the case of a composite gadget + at91 UDC, this would look like: udc-driver-disconnect(udc-gadget); // composite_disconnect() usb_gadget_disconnect(udc-gadget); // at91_pullup(gadget, 0) udc-driver-unbind(udc-gadget); // composite_unbind() usb_gadget_udc_stop(udc-gadget, udc-driver); // at91_stop() composite_disconnect() says: if (cdev-config) reset_config(cdev); reset_config() sets cdev-config to NULL. composite_unbind() later tests for this: WARN_ON(cdev-config); But SETUP packets may be sent to the composite driver up until the point when usb_gadget_disconnect() returns. That doesn't sound right. A host can't send SETUP packets to a disconnected port. The packets should stop arriving when udc-driver-disconnect returns -- assuming the UDC driver implements a disconnect method. udc-driver-disconnect points to composite_disconnect(). What is composite_disconnect() doing to tell the UDC driver to disconnect the port from the host? What I see is that usb_gadget_disconnect() does tell the UDC driver to stop activity on the port, but that only happens after composite_disconnect() was called. Can you confirm that the order of the calls in usb_gadget_remove_driver() is correct? Thanks. -- 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
