Hello.
On 03/12/2012 05:05 PM, Manjunathappa, Prakash wrote:
Patch abstracts and moves common USB OHCI platform setup code of da8xx
based boards to mach-davinci/usb.c file, thereby it avoids repetition of
code.
Patch is based on consolidation concern raised by Sekhar Nori.
https://lkml.org/lkml/2011/12/21/48
Signed-off-by: Manjunathappa, Prakash<prakash.pm@xxxxxx>
---
arch/arm/mach-davinci/board-da830-evm.c | 86 +++-----------------
arch/arm/mach-davinci/board-omapl138-hawk.c | 93 +++-------------------
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +
arch/arm/mach-davinci/include/mach/usb.h | 32 +++++++-
arch/arm/mach-davinci/usb.c | 116 +++++++++++++++++++++++++++
drivers/usb/host/ohci-da8xx.c | 15 ++--
6 files changed, 178 insertions(+), 166 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index dc1afe5..edc8277 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -46,59 +46,23 @@ static const short da830_evm_usb11_pins[] = {
[...]
- /* TPS2065 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
Why are you removing this initializer? You don't seem to pass it anywhere
else...
+ .type = GPIO_BASED,
+ .method = {
+ .gpio_method = {
+ .power_control_pin = ON_BD_USB_DRV,
+ .over_current_indicator = ON_BD_USB_OVC,
+ },
+ },
+ .board_ocic_handler = da830_evm_usb_ocic_irq,
};
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 45e8157..9214923 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -184,77 +184,34 @@ mmc_setup_wp_fail:
[...]
static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
- .set_power = hawk_usb_set_power,
- .get_power = hawk_usb_get_power,
- .get_oci = hawk_usb_get_oci,
- .ocic_notify = hawk_usb_ocic_notify,
- /* TPS2087 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
Same question.
diff --git a/arch/arm/mach-davinci/include/mach/usb.h b/arch/arm/mach-davinci/include/mach/usb.h
index e0bc4ab..1a6f211 100644
--- a/arch/arm/mach-davinci/include/mach/usb.h
+++ b/arch/arm/mach-davinci/include/mach/usb.h
[...]
@@ -33,25 +35,47 @@
#define CFGCHIP2_REFFREQ_12MHZ (1<< 0)
#define CFGCHIP2_REFFREQ_24MHZ (2<< 0)
#define CFGCHIP2_REFFREQ_48MHZ (3<< 0)
Empty line needed here.
+enum usb_power_n_ovc_method {
+ GPIO_BASED = 1,
+};
struct da8xx_ohci_root_hub;
typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub,
unsigned port);
+struct gpio_based {
+ u32 power_control_pin;
+ u32 over_current_indicator;
Aren't GPIOs just 'unsigned' in gpiolib, without size?
+};
+
/* Passed as the platform data to the OHCI driver */
struct da8xx_ohci_root_hub {
/* Switch the port power on/off */
- int (*set_power)(unsigned port, int on);
+ int (*set_power)(unsigned port, struct da8xx_ohci_root_hub *hub,
+ int on);
/* Read the port power status */
- int (*get_power)(unsigned port);
+ int (*get_power)(unsigned port, struct da8xx_ohci_root_hub *hub);
/* Read the port over-current indicator */
- int (*get_oci)(unsigned port);
+ int (*get_oci)(unsigned port, struct da8xx_ohci_root_hub *hub);
/* Over-current indicator change notification (pass NULL to disable) */
- int (*ocic_notify)(da8xx_ocic_handler_t handler);
+ int (*ocic_notify)(da8xx_ocic_handler_t handler,
+ struct da8xx_ohci_root_hub *hub);
/* Time from power on to power good (in 2 ms units) */
u8 potpgt;
+
+ /* Power control and over current control method */
+ unsigned int type;
+ union power_n_overcurrent_pins {
+ struct gpio_based gpio_method;
+ /* Add data pertaining other methods here. For example if its
+ * I2C based.
+ */
I thought that you need to first convert the existing GPIO-based method, and
think about the extensions when the need arises, in the next patches. You're
looking too far ahead in this patch...
+ } method;
+
+ /* board specific handler */
+ irq_handler_t board_ocic_handler;
};
void davinci_setup_usb(unsigned mA, unsigned potpgt_ms);
diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
index 23d2b6d..25c3520 100644
--- a/arch/arm/mach-davinci/usb.c
+++ b/arch/arm/mach-davinci/usb.c
@@ -11,12 +12,70 @@
#include<mach/irqs.h>
#include<mach/cputype.h>
#include<mach/usb.h>
+#include<mach/mux.h>
#define DAVINCI_USB_OTG_BASE 0x01c64000
#define DA8XX_USB0_BASE 0x01e00000
#define DA8XX_USB1_BASE 0x01e25000
+static int da8xx_usb_set_power(unsigned port, struct da8xx_ohci_root_hub *hub,
+ int on)
+{
+ struct gpio_based *gpio = (hub->type == GPIO_BASED) ?
+ &hub->method.gpio_method : NULL;
+
+ if (hub->type == GPIO_BASED)
+ gpio_set_value(gpio->power_control_pin, on);
The code doesn't look very flexible. Why not just use:
switch (hub->type) {
case GPIO_BASED:
{
struct gpio_based *gpio = &hub->method.gpio_method;
gpio_set_value(gpio->power_control_pin, on);
}
}
+ return 0;
+}
+
+static int da8xx_usb_get_power(unsigned port, struct da8xx_ohci_root_hub *hub)
+{
+ struct gpio_based *gpio = (hub->type == GPIO_BASED) ?
+ &hub->method.gpio_method : NULL;
+
+ if (hub->type == GPIO_BASED)
+ return gpio_get_value(gpio->power_control_pin);
+ return 0;
+}
+static int da8xx_usb_get_oci(unsigned port, struct da8xx_ohci_root_hub *hub)
+{
+ struct gpio_based *gpio = (hub->type == GPIO_BASED) ?
+ &hub->method.gpio_method : NULL;
+
+ if (hub->type == GPIO_BASED)
+ return !gpio_get_value(gpio->over_current_indicator);
+ return 0;
+}
I see another type of inflexibility in the code: you always assume that
over-current indicator is an inverted signal. Likewise the power control is
always uninverted, in your assumption (more realistic, I admit).
@@ -177,4 +236,61 @@ int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata)
da8xx_usb11_device.dev.platform_data = pdata;
return platform_device_register(&da8xx_usb11_device);
}
+
+void __init da8xx_board_usb_init(const short pins[],
+ struct da8xx_ohci_root_hub *usb11_pdata)
+{
[...]
+ /* TPS2087 switch @ 5V */
+ usb11_pdata->potpgt = (3 + 1) / 2;
This is board specific data, not generic in any way. Leave it were it was.
I can't say I like this generaization result. Certain NAK on implementation
details.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
