Re: [PATCH 2/3] usb: musb: fix bug in musb_start_urb
Hello. David Brownell wrote: Ajay Kumar Gupta wrote: urb->transfer_buffer_length and urb->transfer_buffer should be updated based on urb->actual_length.For a fresh and first time urb, actual_length will be zero but for urbs which has been stopped and restarted (as bulk nak scheme does) actual_length may not be zero. Signed-off-by: Ajay Kumar Gupta NAK, this is not a problem for the current driver since URBs do not ever get restarted. Resolvable by changing the patch description to not say this is a "fix". However, since this is a two line change, I think I'll just merge this with the patch adding the bulk RX retry logic. Yes, that's what should've been done from the start. Also, musb_host_tx() doesn't update urb->actual_length -- please fix it too. That would be a good point if the retry patch touched any TX paths. But it doesn't. If one teaches musb_start_urb() to restart, that one should at least be consistent I think. Also, you must not clear qh->iso_idx when restarting -- you must not start ISO transfer all over again too. Also, you should not set musb->ep0_state to MUSB_EP0_START again in this case (I agree that control transfers will remain not restartable from an arbitatry place even then). But the [3/3] patch only adds NAK timeout support for bulk RX. And ISO transfers can't NAK in the first place. Plus, as noted in a comment you could see in this patch, this only touches (re)start for bulk/interrupt transfers. Not ISO; not control. All I was asking for was a bit of consistency. Note that I have already done all the changes that I requested for and can post them. URB restart is also going to be used for the interrupt transfers BTW. - Dave WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: musb: fix bug in musb_start_urb
Ajay Kumar Gupta wrote: urb->transfer_buffer_length and urb->transfer_buffer should be updated based on urb->actual_length.For a fresh and first time urb, actual_length will be zero but for urbs which has been stopped and restarted (as bulk nak scheme does) actual_length may not be zero. Signed-off-by: Ajay Kumar Gupta NAK, this is not a problem for the current driver since URBs do not ever get restarted. Also, musb_host_tx() doesn't update urb->actual_length -- please fix it too. Also, you must not clear qh->iso_idx when restarting -- you must not start ISO transfer all over again too. Also, you should not set musb->ep0_state to MUSB_EP0_START again in this case (I agree that control transfers will remain not restartable from an arbitatry place even then). If you're trying to make musb_start_urb() able to re-start, please be consistent. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/8] musb: Add structure to get board specific data
Hello.
Ajay Kumar Gupta wrote:
Adding 'musb_hdrc_board_data' which will have all the board specific
parameters such as; mA power, potpgt, extvbus, gpios etc.
Currently only 'power' and 'potpgt' is being moved from existing
'musb_hdrc_platform_data' to 'musb_hdrc_board_data' but any further
board specific functions or parameter can be added to this structure
later.
Signed-off-by: Ajay Kumar Gupta
---
include/linux/usb/musb.h | 18 --
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index d437556..6e1426c 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -27,6 +27,15 @@ struct musb_hdrc_eps_bits {
u8 bits;
};
+/* MUSB board-specific details */
+struct musb_hdrc_board_data {
+ /* power (mA/2) sourcing capability */
+ u8 power;
+ /* (HOST or OTG) msec/2 after VBUS on till power good */
+ u8 potpgt;
Uneven indentation, either too many or too little tabs here.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/8] musb: Get power (mA) from board data
Hello. Ajay Kumar Gupta wrote: Different board may have different power sourcing capability and now with 'struct musb_hdrc_board_data' in place; pass this data from board files and also modify musb_core.c to get 'power' data from 'plat->board_data'. This should be part of the patch 1/8 to keep the code compiling. Signed-off-by: Ajay Kumar Gupta diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 3a61ddb..818ccda 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -2033,7 +2033,9 @@ bad_config: if (is_otg_enabled(musb)) hcd->self.otg_port = 1; musb->xceiv->host = &hcd->self; - hcd->power_budget = 2 * (plat->power ? : 250); + if (plat->board_data) + hcd->power_budget = + 2 * (plat->board_data->power ? : 250); Shouldn't it be: + + hcd->power_budget = 2 * (plat->board_data && +plat->board_data->power ? +plat->board_data->power : 250); WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] musb: Update setup_usb() call for all Davinci boards
Ajay Kumar Gupta wrote:
setup_usb() has been modified to pass board specific data so updating
this function call from all Davinci based boards.
Added "struct device;" to fix below compilation warning for Davinci boards.
"musb.h: struct device, defined within parameter list"
You should fix the missing #include in the musb.h, not band-aid it here...
Signed-off-by: Ajay Kumar Gupta
diff --git a/arch/arm/mach-davinci/board-dm355-evm.c
b/arch/arm/mach-davinci/board-dm355-evm.c
index 77e8067..31c5741 100644
--- a/arch/arm/mach-davinci/board-dm355-evm.c
+++ b/arch/arm/mach-davinci/board-dm355-evm.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -317,6 +318,12 @@ static struct spi_board_info dm355_evm_spi_info[]
__initconst = {
},
};
+/* musb board specific data */
+static struct musb_hdrc_board_data musb_bdata __initdata = {
+ .power = 250, /* (power in mA)/2 */
+ .potpgt = 4,/* (potpgt in msec)/2 */
+};
+
static __init void dm355_evm_init(void)
{
struct clk *aemif;
@@ -344,7 +351,7 @@ static __init void dm355_evm_init(void)
gpio_request(2, "usb_id_toggle");
gpio_direction_output(2, USB_ID_VALUE);
/* irlml6401 switches over 1A in under 8 msec */
- setup_usb(500, 8);
+ setup_usb(&musb_bdata);
Unfortunately, this will conflict with a patch queued for 2.6.33 in
linux-davinci. Though in fact, it will render the part of this patch
useless... :-/
diff --git a/arch/arm/mach-davinci/include/mach/common.h
b/arch/arm/mach-davinci/include/mach/common.h
index 1fd3917..dab784c 100644
--- a/arch/arm/mach-davinci/include/mach/common.h
+++ b/arch/arm/mach-davinci/include/mach/common.h
@@ -20,11 +20,14 @@ extern void davinci_irq_init(void);
extern void __iomem *davinci_intc_base;
extern int davinci_intc_type;
+struct device;
NAK. should be fixed instead.
+#include
+
/* parameters describe VBUS sourcing for host mode */
-extern void setup_usb(unsigned mA, unsigned potpgt_msec);
+extern void setup_usb(struct musb_hdrc_board_data *board_data);
/* parameters describe VBUS sourcing for host mode */
-extern void setup_usb(unsigned mA, unsigned potpgt_msec);
+extern void setup_usb(struct musb_hdrc_board_data *board_data);
Don't you see -- these are duplicate? You could kill the second one. :-)
BTW, the mentioned linux-davinci patch moved the declaration to
(and renamed the function too).
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/8] musb: Add structure to get board specific data
Ajay Kumar Gupta wrote:
Adding 'musb_hdrc_board_data' which will have all the board specific
parameters such as; mA power, potpgt, extvbus, gpios etc.
Currently only 'power' and 'potpgt' is being moved from existing
'musb_hdrc_platform_data' to 'musb_hdrc_board_data' but any further
board specific functions or parameter can be added to this structure
later.
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index d437556..6e1426c 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
[...]
@@ -67,15 +76,9 @@ struct musb_hdrc_platform_data {
/* (HOST or OTG) switch VBUS on/off */
int (*set_vbus)(struct device *dev, int is_on);
- /* (HOST or OTG) mA/2 power supplied on (default = 8mA) */
- u8 power;
-
Oh, this will break compilation of the existing code! This patch
shouldn't be separated from patch 2/8.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] musb: set 'extvbus = 0' for OMAP3 boards
Ajay Kumar Gupta wrote: Default value of 'extvbus' is being set as '0' to maintain the current programming state of all OMAP3 musb boards. This flag should be set to '1' for boards using external vbus supply such as, OMAP3EVM Rev >=E. This patch is rather pointless as the struct fieds not explictily initialized will be default to 0 anyway. You should only need to explicitly init to 1. Signed-off-by: Ajay Kumar Gupta WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] musb: set 'extvbus = 0' for Davinci boards
Ajay Kumar Gupta wrote: Default value of 'extvbus' is being set as '0' to maintain the current programming state of all Davinci musb boards. Signed-off-by: Ajay Kumar Gupta Pointless patch again... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] musb: Update setup_usb() call for all Davinci boards
Ajay Kumar Gupta wrote:
setup_usb() has been modified to pass board specific data so updating
this function call from all Davinci based boards.
Added "struct device;" to fix below compilation warning for Davinci boards.
"musb.h: struct device, defined within parameter list"
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/arch/arm/mach-davinci/board-sffsdr.c
b/arch/arm/mach-davinci/board-sffsdr.c
index 7acdfd8..e61d7d7 100644
--- a/arch/arm/mach-davinci/board-sffsdr.c
+++ b/arch/arm/mach-davinci/board-sffsdr.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
@@ -149,6 +150,12 @@ static struct davinci_uart_config uart_config __initdata =
{
.enabled_uarts = (1 << 0),
};
+/* musb board specific data */
+static struct musb_hdrc_board_data musb_bdata __initdata = {
+ .power = 0, /* (power in mA)/2 */
+ .potpgt = 0,/* (potpgt in msec)/2 */
You can leave the structure unixitialized here, it will default to all
zeros anyway.
+};
+
static void __init davinci_sffsdr_map_io(void)
{
dm644x_init();
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] musb: Update setup_usb() call for all Davinci boards
Ajay Kumar Gupta wrote:
setup_usb() has been modified to pass board specific data so updating
this function call from all Davinci based boards.
Added "struct device;" to fix below compilation warning for Davinci boards.
"musb.h: struct device, defined within parameter list"
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
index 06f5593..1b164dc 100644
--- a/arch/arm/mach-davinci/usb.c
+++ b/arch/arm/mach-davinci/usb.c
@@ -85,10 +85,10 @@ static struct platform_device usb_dev = {
.num_resources = ARRAY_SIZE(usb_resources),
};
-void __init setup_usb(unsigned mA, unsigned potpgt_msec)
+void __init setup_usb(struct musb_hdrc_board_data *board_data)
{
- usb_data.power = mA / 2;
- usb_data.potpgt = potpgt_msec / 2;
Hm, again, you can't separate this patch from patch 1/8 to keep the code
compiling between the patches. It looks like you have no choice but lump
most of your patches together (and get rid of some others :-). Either that
or better separate your change to the struct musb_hdrc_platfrpom_data and
your change to setup_usb() prototype.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] musb: set 'extvbus = 0' for Blackfin boards
Ajay Kumar Gupta wrote: Default value of 'extvbus' is being set as '0' to maintain the current programming state of all Blackfin musb boards. Again, you could keep the structures unitialized so that they default to all zeros (which they do anyway for the 'power' and 'potpgt' fields with your patch). Signed-off-by: Ajay Kumar Gupta WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] musb: Update musb_init() call for all OMAP3 boards
Ajay Kumar Gupta wrote:
musb_init() has been modified to pass board specific data so updating
this function call from all OMAP3 boards.
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 1145a25..0e9380c 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -124,12 +124,6 @@ static struct musb_hdrc_platform_data musb_plat = {
/* .clock is set dynamically */
.set_clock = musb_set_clock,
.config = &musb_config,
-
- /* REVISIT charge pump on TWL4030 can supply up to
-* 100 mA ... but this value is board-specific, like
-* "mode", and should be passed to usb_musb_init().
-*/
- .power = 50, /* up to 100 mA */
That should obviously be a part of patch 1/8...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/8] musb: Add structure 'musb_hdrc_board_data'
Hello. Ajay Kumar Gupta wrote: This patch set adds a new structure 'musb_hdrc_board_data' to get all board specific data from board files. It is actually done to accomodate ULPI_VBUSCONTROL programming required for OMAP3EVM Rev >=E which uses external Vbus supply to support 500mA. It's not clear why it was necessary to create yet another structure. You could your new field to 'struct musb_hdrc_platfrom_data'... Regards, Ajay WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] usb: musb: fail unaligned DMA transfers on v1.8 and above
Hello. On 31-10-2010 8:25, Anand Gadiyar wrote: The Inventra DMA engine in version 1.8 and later of the MUSB controller cannot handle DMA addresses that are not aligned to a 4 byte boundary. It ends up ignoring the last two bits programmed in the DMA_ADDR register. This is a deliberate design change in the controller and is documented in the programming guide. Earlier versions of the controller could handle these accesses just fine. Fail dma_channel_program if we see an unaligned address when using the newer controllers, so that the caller can carry out the transfer using PIO mode. (Current callers already have this backup path in place). Signed-off-by: Anand Gadiyar Cc: Felipe Balbi Cc: Ming Lei Cc: Ajay Kumar Gupta Cc: Mike Frysinger [...] Index: mainline/drivers/usb/musb/musbhsdma.c === --- mainline.orig/drivers/usb/musb/musbhsdma.c +++ mainline/drivers/usb/musb/musbhsdma.c [...] @@ -167,6 +169,18 @@ static int dma_channel_program(struct dm BUG_ON(channel->status == MUSB_DMA_STATUS_UNKNOWN || channel->status == MUSB_DMA_STATUS_BUSY); + /* +* The DMA engine in RTL1.8 and above cannot handle +* DMA addresses that are not aligned to a 4 byte boundary. +* It ends up masking the last two bits of the address +* programmed in DMA_ADDR. +* +* Fail such DMA transfers, so that the backup PIO mode +* can carry out the transfer +*/ + if ((musb->hwvers >= MUSB_HWVERS_1800) && (dma_addr %4)) Also need space after %. + return false; + WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] omap:mailbox-fix checkpatch warnings
Hello. Hari Kanigeri wrote: Fix the checkpatch warnings observed in mailbox module Signed-off-by: Hari Kanigeri --- arch/arm/plat-omap/mailbox.c |9 + 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 9ce3570..ed960c1 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c [...] @@ -394,7 +394,8 @@ static int __init omap_mbox_init(void) /* kfifo size sanity check: alignment and minimal size */ mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t)); - mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(mbox_msg_t)); + mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, + sizeof(mbox_msg_t)); This line is over-indented a bit. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix IGEPv2 second MMC channel power supply
Hello. On 14-11-2010 14:34, Marc Zyngier wrote: Commit 72f381ba056 removed an unused regulator entry, but left Linus has aksed to also specify the commit summary in parentheses. Signed-off-by: Marc Zyngier Cc: Enric Balletbo i Serra Cc: Tony Lindgren WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] OMAP: wd_timer: remove old, dead probing code
Hello. On 16-11-2010 13:25, Paul Walmsley wrote: After commit f2ce62312650211f6cf665cd6dc519c334c4071e, watchdog Linus has asked to also specify the commit summary in parens... probing is handled by files in mach-omap1/ and mach-omap2/, and the plat-omap/devices.c probing code is no longer used. Remove the dead code in plat-omap/devices.c. Signed-off-by: Paul Walmsley Cc: Charulatha Varadarajan WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v1] AM35xx: Craneboard: Add USB EHCI support
Hello.
On 19-11-2010 19:07, srin...@mistralsolutions.com wrote:
From: Srinath
AM3517/05 Craneboard has one EHCI interface on board using port1.
GPIO35 is used as power enable.
GPIO38 is used as port1 PHY reset.
Signed-off-by: Srinath
---
arch/arm/mach-omap2/board-am3517crane.c | 21 +
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-am3517crane.c
b/arch/arm/mach-omap2/board-am3517crane.c
index 13ead33..0e1a806 100644
--- a/arch/arm/mach-omap2/board-am3517crane.c
+++ b/arch/arm/mach-omap2/board-am3517crane.c
[...]
@@ -53,10 +55,29 @@ static void __init am3517_crane_init_irq(void)
omap_gpio_init();
}
+static struct ehci_hcd_omap_platform_data ehci_pdata __initdata = {
+ .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
+ .port_mode[1] = EHCI_HCD_OMAP_MODE_UNKNOWN,
+ .port_mode[2] = EHCI_HCD_OMAP_MODE_UNKNOWN,
+
+ .phy_reset = true,
+ .reset_gpio_port[0] = 38,
+ .reset_gpio_port[1] = -EINVAL,
+ .reset_gpio_port[2] = -EINVAL
+};
+
static void __init am3517_crane_init(void)
{
omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
omap_serial_init();
+
+ /* Configure GPIO for EHCI port */
+ omap_mux_init_gpio(35, OMAP_PIN_OUTPUT);
+ gpio_request(35, "usb_ehci_enable");
+ gpio_direction_output(35, 1);
+ gpio_set_value(35, 1);
There's no need to call gpio_set_value(), as gpio_direction_output() has
already set the GPIO's value.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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 6/8] usb: ehci-omap: add helpers for checking port mode
Hello.
On 29-11-2010 20:25, Anand Gadiyar wrote:
Introduce helper functions to test port mode. These checks are
performed in several places in the driver, and these helpers
improve readability.
Signed-off-by: Anand Gadiyar
[...]
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 7a4682c..dd9d5c1 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
[...]
@@ -387,27 +390,27 @@ static int omap_start_ehc(struct ehci_hcd_omap *omap,
struct usb_hcd *hcd)
/* Bypass the TLL module for PHY mode operation */
if (cpu_is_omap3430()&& (omap_rev()<= OMAP3430_REV_ES2_1)) {
dev_dbg(omap->dev, "OMAP3 ES version<= ES2.1\n");
- if ((omap->port_mode[0] == EHCI_HCD_OMAP_MODE_PHY) ||
- (omap->port_mode[1] == EHCI_HCD_OMAP_MODE_PHY) ||
- (omap->port_mode[2] == EHCI_HCD_OMAP_MODE_PHY))
+ if (is_ehci_phy_mode(omap->port_mode[0]) ||
+ is_ehci_phy_mode(omap->port_mode[1]) ||
+ is_ehci_phy_mode(omap->port_mode[2]))
Why there's another indentation level whenre there shouldn't be one?
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: am35x: fix compile error due to control apis
Hello.
Ajay Kumar Gupta wrote:
As the control.h have been moved to new location and it's
uses are not allowed to drivers directly so moving the phy
control, interrupt clear and reset functionality to board
files.
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 7260558..1f32fdb 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -33,6 +33,82 @@
#ifdef CONFIG_USB_MUSB_SOC
+static void am35x_musb_phy_power(u8 on)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(100);
+ u32 devconf2;
+
+ if (on) {
+ /*
+* Start the on-chip PHY and its PLL.
+*/
+ devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
+
+ devconf2 &= ~(CONF2_RESET | CONF2_PHYPWRDN | CONF2_OTGPWRDN);
+ devconf2 |= CONF2_PHY_PLLON;
+
+ omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2);
+
+ printk(KERN_INFO "Waiting for PHY clock good...\n");
pr_info().
+ while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2)
+ & CONF2_PHYCLKGD)) {
+ cpu_relax();
+
+ if (time_after(jiffies, timeout)) {
+ printk(KERN_ERR "musb PHY clock good timed
out\n");
pr_err().
diff --git a/arch/arm/plat-omap/include/plat/usb.h
b/arch/arm/plat-omap/include/plat/usb.h
index 59c7fe7..4299097 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -69,6 +69,9 @@ struct omap_musb_board_data {
u8 mode;
u16 power;
unsigned extvbus:1;
+ void(*set_phy_power) (u8 on);
+ void(*clear_irq) (void);
+ void(*set_mode) (u8 mode);
Should be no spaces between ) and (. scripts/checkpatch.pl used to complain
about such things, now it's silent though...
@@ -364,37 +324,21 @@ eoi:
int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
{
- u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
+ struct device *dev = musb->controller;
+ struct musb_hdrc_platform_data *plat = dev->platform_data;
+ struct omap_musb_board_data *data = plat->board_data;
- devconf2 &= ~CONF2_OTGMODE;
- switch (musb_mode) {
-#ifdef CONFIG_USB_MUSB_HDRC_HCD
- case MUSB_HOST: /* Force VBUS valid, ID = 0 */
- devconf2 |= CONF2_FORCE_HOST;
- break;
-#endif
-#ifdef CONFIG_USB_GADGET_MUSB_HDRC
- case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */
- devconf2 |= CONF2_FORCE_DEVICE;
- break;
-#endif
-#ifdef CONFIG_USB_MUSB_OTG
- case MUSB_OTG: /* Don't override the VBUS/ID comparators */
- devconf2 |= CONF2_NO_OVERRIDE;
- break;
-#endif
- default:
- DBG(2, "Trying to set unsupported mode %u\n", musb_mode);
- }
+ if (data->set_mode)
+ data->set_mode(musb_mode);
You should return -EIO if data->set_mode is NULL.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] musb: am35x: fix compile error due to control apis
Hello.
On 06-12-2010 11:13, Ajay Kumar Gupta wrote:
As the control.h have been moved to new location and it's
uses are not allowed to drivers directly so moving the phy
control, interrupt clear and reset functionality to board
files.
I'm not fond of the whole approach. I'm not sure why accesses to the
control registers are such a no-no, taking into account that one needs to
access such regisyter to clear the interrupt...
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 7260558..8c1d121 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -30,9 +30,102 @@
#include
#include
#include
+#include "control.h"
#ifdef CONFIG_USB_MUSB_SOC
+static void am35x_musb_reset(void)
+{
+ u32 regval;
+
+ /* Reset the musb interface */
+ regval = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET);
+
+ regval |= AM35XX_USBOTGSS_SW_RST;
+ omap_ctrl_writel(regval, AM35XX_CONTROL_IP_SW_RESET);
+
+ regval&= ~AM35XX_USBOTGSS_SW_RST;
+ omap_ctrl_writel(regval, AM35XX_CONTROL_IP_SW_RESET);
+
+ regval = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET);
Why read it and ignore the result?
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] musb: am35x: fix compile error due to control apis
Hello. Gupta, Ajay Kumar wrote: As the control.h have been moved to new location and it's uses are not allowed to drivers directly so moving the phy control, interrupt clear and reset functionality to board files. I'm not fond of the whole approach. I'm not sure why accesses to the control registers are such a no-no, taking into account that one needs to access such regisyter to clear the interrupt... I think Tony is the right person to answer this :) Yeah, I thought he must be reading linux-omap... Signed-off-by: Ajay Kumar Gupta [...] diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c [...] + regval&= ~AM35XX_USBOTGSS_SW_RST; + omap_ctrl_writel(regval, AM35XX_CONTROL_IP_SW_RESET); + + regval = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET); Why read it and ignore the result? This is due to recommendation for OMAPs as discussed at below Link, http://markmail.org/message/s3lp7xlyq7zxnjtc#query:+page:1+mid:kfia4ld4xgzek6kq+state:results There is recommendation to read back the interrupt status register, which this register isn't. Anyway, a comment wouldn't hurt... Thanks, Ajay WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] musb: am35x: fix compile error due to control apis
Hello. On 06-12-2010 20:54, Tony Lindgren wrote: As the control.h have been moved to new location and it's uses are not allowed to drivers directly so moving the phy control, interrupt clear and reset functionality to board files. I'm not fond of the whole approach. I'm not sure why accesses to the control registers are such a no-no, taking into account that one needs to access such regisyter to clear the interrupt... Because drivers should arch independent and any tinkering of the platform level registers will lead into pains later on that I end up having to deal with. To me it seem you just init to separate out the transceiver, right? We also have to install a hook to clear the MUSB level interrupt via the control register -- that's a thing that makes me dubious about not allowing drivers to access the control registers. Tony WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] usb: musb: TWL6030: Selecting TWL6030_USB transceiver
Hello. Hema HK wrote: Selecting the twl6030-usb for OMAP4430SDP and OMAP4 PANDA board and adding OMAP4 internal phy code for compilation Signed-off-by: Hema HK Cc: Felipe Balbi Cc: Tony Lindgren [...] Index: linux-2.6/drivers/usb/musb/Kconfig === --- linux-2.6.orig/drivers/usb/musb/Kconfig +++ linux-2.6/drivers/usb/musb/Kconfig @@ -12,6 +12,7 @@ config USB_MUSB_HDRC depends on (ARM || (BF54x && !BF544) || (BF52x && !BF522 && !BF523)) select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN) select TWL4030_USB if MACH_OMAP_3430SDP + select TWL6030_USB if (MACH_OMAP_3430SDP || MACH_OMAP4_PANDA) Parens are not necessary. Though the previous code uses them... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] usb: otg: Kconfig: Add Kconfig option for TWL6030 transceiver.
Hello. Hema HK wrote: Added the TWL6030-usb transceiver option in the Kconfig Signed-off-by: Hema HK Cc: Felipe Balbi Cc: David Brownell [...] Index: linux-2.6/drivers/usb/otg/Kconfig === --- linux-2.6.orig/drivers/usb/otg/Kconfig +++ linux-2.6/drivers/usb/otg/Kconfig @@ -59,6 +59,18 @@ config TWL4030_USB This transceiver supports high and full speed devices plus, in host mode, low speed. +config TWL6030_USB + tristate "TWL6030 USB Transceiver Driver" + depends on TWL4030_CORE + select USB_OTG_UTILS + help + Enable this to support the USB OTG transceiver on TWL6030 + family chips. This TWL6030 transceiver has the VBUS and ID GND + and OTG SRP events capabilities. For all other transceiver functionality + UTMI PHY is embedded in OMAP4430.The internal PHY configurations APIs ^^ Space missing after period. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mfd: TWL6030: OMAP4: Registering the TWL6030-usb device
Hello. On 08-12-2010 19:01, Hema HK wrote: Registering the twl6030-usb transceiver device as a child to twl6030 core. Removed the NOP transceiver init call from board file. Populated twl4030_usb_data platform data structure with the function pointers for OMAP4430 internal PHY operation to be used twl630-usb driver. ^ by? Signed-off-by: Hema HK Cc: Samuel Ortiz Cc: Felipe Balbi Cc: Tony Lindgren [...] Index: linux-2.6/drivers/mfd/twl-core.c === --- linux-2.6.orig/drivers/mfd/twl-core.c +++ linux-2.6/drivers/mfd/twl-core.c [...] @@ -682,6 +683,43 @@ add_children(struct twl4030_platform_dat [...] + child = add_child(0, "twl6030_usb", + pdata->usb, sizeof(*pdata->usb), + true, + /* irq1 = VBUS_PRES, irq0 = USB ID*/ You forgot space before */. Index: linux-2.6/arch/arm/mach-omap2/board-omap4panda.c === --- linux-2.6.orig/arch/arm/mach-omap2/board-omap4panda.c +++ linux-2.6/arch/arm/mach-omap2/board-omap4panda.c Your patch is not sorted. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: musb: Adding names for IRQs in resource structure
Hello.
Kevin Hilman wrote:
The resource data is getting automatically populated from a set of data
generated from TI's hardware database for the OMAP platform,
While we could hack in some exceptions to that tool to generate resources
in a specific order, it seems less fragile to use the resource name
instead.That way, no matter what order the resources are generated, the
driver still work.
Modified the OMAP,Blackfin and Davinci architecture files to add the name of
the IRQs
in the resource structures and musb driver to use the get_irq_byname() api to
get the device and dma irq numbers instead of using the index.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
Cc: Kevin Hilman
Cc: Cousson, Benoit
Cc: Paul Walmsley
Cc: Mike Frysinger
---
For the davinci changes:
Acked-by: Kevin Hilman
Kevin
[...]
Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
===
--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
{
.start = IRQ_USBINT,
.flags = IORESOURCE_IRQ,
+ .name = "mc"
},
{
/* placeholder for the dedicated CPPI IRQ */
.flags = IORESOURCE_IRQ,
+ .name = "dma"
},
};
Argh! This failed to also modify da8xx_usb20_resources[]... :-(
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: musb: Adding names for IRQs in resource structure
Hello.
On 10-12-2010 7:46, Kalliguddi, Hema wrote:
The resource data is getting automatically populated from a set of data
generated from TI's hardware database for the OMAP platform, While we
could hack in some exceptions to that tool to generate resources
in a specific order, it seems less fragile to use the resource name
instead.That way, no matter what order the resources are generated, the
driver still work.
Modified the OMAP,Blackfin and Davinci architecture files to add the name
of the IRQs
in the resource structures and musb driver to use the get_irq_byname()
api to
get the device and dma irq numbers instead of using the index.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
Cc: Kevin Hilman
Cc: Cousson, Benoit
Cc: Paul Walmsley
Cc: Mike Frysinger
---
For the davinci changes:
Acked-by: Kevin Hilman
Kevin
[...]
Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
===
--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
{
.start = IRQ_USBINT,
.flags = IORESOURCE_IRQ,
+ .name = "mc"
},
{
/* placeholder for the dedicated CPPI IRQ */
.flags = IORESOURCE_IRQ,
+ .name = "dma"
},
};
Argh! This failed to also modify da8xx_usb20_resources[]... :-(
I think when I posted these patch, da8xx support was not there in mainline.
No, it was added long ago, before the glue layer itself.
I will send patch to add this.
I will care about this myself now.
Regards,
Hema
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8 V3] usb: otg: Kconfig: Add Kconfig option for TWL6030 transceiver.
Hello. On 10-12-2010 15:38, Hema HK wrote: Added the TWL6030-usb transceiver option in the Kconfig Signed-off-by: Hema HK Cc: Felipe Balbi Cc: David Brownell [...] Index: usb/drivers/usb/otg/Kconfig === --- usb.orig/drivers/usb/otg/Kconfig +++ usb/drivers/usb/otg/Kconfig @@ -59,6 +59,18 @@ config TWL4030_USB This transceiver supports high and full speed devices plus, in host mode, low speed. +config TWL6030_USB + tristate "TWL6030 USB Transceiver Driver" + depends on TWL4030_CORE + select USB_OTG_UTILS + help + Enable this to support the USB OTG transceiver on TWL6030 + family chips. This TWL6030 transceiver has the VBUS and ID GND + and OTG SRP events capabilities. For all other transceiver functionality + UTMI PHY is embedded in OMAP4430. The internal PHY configurations APIs + are hooked to this driver through platform_data structure. + The definition of internal PHY APIs are in the mach-omap2 layer. + Why not do it in the patch 2, together with the driver itself? And where are you adding the driver to drivers/usb/otg/Makefile? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8 V3] usb: otg: TWL6030: Add twl6030_usb file for compilation
Hello. On 10-12-2010 15:40, Hema HK wrote: Add the twl6030_usb transceiver file for compilation. Signed-off-by: Hema HK Cc: Felipe Balbi Index: usb/drivers/usb/otg/Makefile === --- usb.orig/drivers/usb/otg/Makefile +++ usb/drivers/usb/otg/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_USB_OTG_UTILS) += otg.o obj-$(CONFIG_USB_GPIO_VBUS) += gpio_vbus.o obj-$(CONFIG_ISP1301_OMAP)+= isp1301_omap.o obj-$(CONFIG_TWL4030_USB) += twl4030-usb.o +obj-$(CONFIG_TWL6030_USB) += twl6030-usb.o Why not do it in the patch 3, along with adding the Kconfig entry?! Or even in patch 2, along with the driver itself? Why "smear" these simple things it over N patches?! WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: musb: Adding names for IRQs in resource structure
Hello.
On 10-12-2010 16:20, Felipe Balbi wrote:
The resource data is getting automatically populated from a set of data
generated from TI's hardware database for the OMAP platform, While we
could hack in some exceptions to that tool to generate resources
in a specific order, it seems less fragile to use the resource name
instead.That way, no matter what order the resources are generated, the
driver still work.
Modified the OMAP,Blackfin and Davinci architecture files to add the name
of the IRQs
in the resource structures and musb driver to use the get_irq_byname()
api to
get the device and dma irq numbers instead of using the index.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
Cc: Kevin Hilman
Cc: Cousson, Benoit
Cc: Paul Walmsley
Cc: Mike Frysinger
---
For the davinci changes:
Acked-by: Kevin Hilman
Kevin
[...]
Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
===
--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
{
.start = IRQ_USBINT,
.flags = IORESOURCE_IRQ,
+ .name = "mc"
},
{
/* placeholder for the dedicated CPPI IRQ */
.flags = IORESOURCE_IRQ,
+ .name = "dma"
},
};
Argh! This failed to also modify da8xx_usb20_resources[]... :-(
I think when I posted these patch, da8xx support was not there in mainline.
No, it was added long ago, before the glue layer itself.
I will send patch to add this.
I will care about this myself now.
Thanks, send me the patch and I'll put to the same branch.
I was thinking about pushing this thru the DaVinci tree. Well, I'll post
to both lists and let you figure out who will apply it... :-)
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: musb: Adding names for IRQs in resource structure
Hello.
Felipe Balbi wrote:
Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
===
--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
{
.start = IRQ_USBINT,
.flags = IORESOURCE_IRQ,
+ .name = "mc"
},
{
/* placeholder for the dedicated CPPI IRQ */
.flags = IORESOURCE_IRQ,
+ .name = "dma"
},
};
Argh! This failed to also modify da8xx_usb20_resources[]... :-(
I think when I posted these patch, da8xx support was not there in
mainline.
No, it was added long ago, before the glue layer itself.
I will send patch to add this.
I will care about this myself now.
Thanks, send me the patch and I'll put to the same branch.
I was thinking about pushing this thru the DaVinci tree. Well, I'll
post to both lists and let you figure out who will apply it... :-)
If Kevin picks it up, fine by me :-)
Hm, I thought this patch has already been applied but it's only in Greg's
usb-next branch yet. I guess it's too late to update it now, so a separate patch
should still be created? However, it's not a hot fix now as I thought, and
doesn't apply to linux-davinci.git...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/9 v3] usb: musb: Remove board_data parameter from musb_platform_init()
Hello. Kevin Hilman wrote: Removed the board_data parameter being passed to musb_platform_init function as board data can be extracted from device structure which is already member of musb structure. Signed-off-by: Hema HK Cc: Felipe Balbi Cc: Tony Lindgren Cc: Kevin Hilman Cc: Cousson, Benoit Cc: Paul Walmsley For the davinci changes: Acked-by: Kevin Hilman Kevin --- drivers/usb/musb/blackfin.c |2 +- drivers/usb/musb/davinci.c |2 +- drivers/usb/musb/musb_core.c |2 +- drivers/usb/musb/musb_core.h |2 +- drivers/usb/musb/omap2430.c |6 -- drivers/usb/musb/tusb6010.c |2 +- 6 files changed, 9 insertions(+), 7 deletions(-) Grr. This misses changes to da8xx.c and am35x.c -- which breaks the compilation for them! Greg, could you drop it from your usb-next branch? Or should we send a patch adding these glue layers? WBR. Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/9 v3] usb: musb: Remove board_data parameter from musb_platform_init()
Hello. Greg KH wrote: Removed the board_data parameter being passed to musb_platform_init function as board data can be extracted from device structure which is already member of musb structure. Signed-off-by: Hema HK Cc: Felipe Balbi Cc: Tony Lindgren Cc: Kevin Hilman Cc: Cousson, Benoit Cc: Paul Walmsley For the davinci changes: Acked-by: Kevin Hilman Kevin --- drivers/usb/musb/blackfin.c |2 +- drivers/usb/musb/davinci.c |2 +- drivers/usb/musb/musb_core.c |2 +- drivers/usb/musb/musb_core.h |2 +- drivers/usb/musb/omap2430.c |6 -- drivers/usb/musb/tusb6010.c |2 +- 6 files changed, 9 insertions(+), 7 deletions(-) Grr. This misses changes to da8xx.c and am35x.c -- which breaks the compilation for them! Greg, could you drop it from your usb-next branch? Or should we send a patch adding these glue layers? I can't drop patches from a git branch, sorry, it doesn't work that way anymore. That's why I really prefer quilt to git. :-) greg k-h WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/9 v3] usb: musb: Remove board_data parameter from musb_platform_init()
Hello. On 11-12-2010 20:43, Greg KH wrote: Removed the board_data parameter being passed to musb_platform_init function as board data can be extracted from device structure which is already member of musb structure. Signed-off-by: Hema HK Cc: Felipe Balbi Cc: Tony Lindgren Cc: Kevin Hilman Cc: Cousson, Benoit Cc: Paul Walmsley For the davinci changes: Acked-by: Kevin Hilman Kevin --- drivers/usb/musb/blackfin.c |2 +- drivers/usb/musb/davinci.c |2 +- drivers/usb/musb/musb_core.c |2 +- drivers/usb/musb/musb_core.h |2 +- drivers/usb/musb/omap2430.c |6 -- drivers/usb/musb/tusb6010.c |2 +- 6 files changed, 9 insertions(+), 7 deletions(-) Grr. This misses changes to da8xx.c and am35x.c -- which breaks the compilation for them! Greg, could you drop it from your usb-next branch? Or should we send a patch adding these glue layers? I can't drop patches from a git branch, sorry, it doesn't work that way anymore. OTOH, you could revert it (as breaking the build). Dunno if that makes sense... Again, I trust the musb maintainer here to handle this type of thing, not me. So take it up with him. Done already. And he's on the CC here too... thanks, greg k-h WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1
Hello. Ming Lei wrote: Which codebase is this patch meant for? I don't see the hb_mult in current mainline code? Are there any patches which this one depends on? This depends on the two iso for device mode patches: http://marc.info/?l=linux-usb&m=128076716001885&w=2 http://marc.info/?l=linux-usb&m=128076716901924&w=2 I plan to resend the two patches above and the double buffer patches later. I recommend that you change the ordering of patches so that fixes come first, and new features last. I.e. this patch should precede the second one of those two above. thanks, WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello.
tom.leim...@gmail.com wrote:
From: Ming Lei
Complete the current request only if the data transfer is over.
Signed-off-by: Ming Lei
Cc: David Brownell
Cc: Felipe Balbi
Cc: Anand Gadiyar
Cc: Mike Frysinger
Cc: Sergei Shtylyov
---
drivers/usb/musb/musb_gadget.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index f3ee04f..fa826f9 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -501,14 +501,14 @@ void musb_g_tx(struct musb *musb, u8 epnum)
request->zero = 0;
}
- /* ... or if not, then complete it. */
- musb_g_giveback(musb_ep, request, 0);
-
- request = musb_ep->desc ? next_request(musb_ep) : NULL;
- if (!request) {
- DBG(4, "%s idle now\n",
- musb_ep->end_point.name);
- return;
+ if (request->actual == request->length) {
But why not modify the conditional above all that code, just excluding
'is_dma' from it. This conditional already includes (request->actual ==
request->length) check. Please recast this patch.
+ musb_g_giveback(musb_ep, request, 0);
+ request = musb_ep->desc ? next_request(musb_ep)
: NULL;
+ if (!request) {
+ DBG(4, "%s idle now\n",
+ musb_ep->end_point.name);
+ return;
+ }
}
}
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello. Ming Lei wrote: But why not modify the conditional above all that code, just excluding 'is_dma' from it. This conditional already includes (request->actual == request->length) check. Please recast this patch. The two condition is OR relation, not and, so we can't exclude 'is_dma' simply. Yes, we can. You're clearly handling only the DMA case with your added check, the PIO case was already handled. Anyway, the change is not wrong, right? Not wrong, but the check is duplicate. thanks, WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello. I wrote: But why not modify the conditional above all that code, just excluding 'is_dma' from it. This conditional already includes (request->actual == request->length) check. Please recast this patch. The two condition is OR relation, not and, so we can't exclude 'is_dma' simply. Yes, we can. You're clearly handling only the DMA case with your added check, the PIO case was already handled. Anyway, the change is not wrong, right? Not wrong, but the check is duplicate. Oops, I've been too fast and haven't realized that the check done here _is_ actually wrong. It causes ZLP send to trigger too fast in the DMA case. So please fix this patch. Felipe, please drop it for now. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello.
On 14-09-2010 10:56, Felipe Balbi wrote:
Oops, I've been too fast and haven't realized that the check done here
_is_ actually wrong. It causes ZLP send to trigger too fast in the DMA
case. So please fix this patch. Felipe, please drop it for now.
patch is not even touching that part of the code,
Yeah, and that's the problem.
not even
reading/writing TXCSR_TXPKTRDY bit care to explain please.
If a DMA interrupt comes when the whole transfer is not yet complete (and
other Ming Lei's patches are making this possible), it will pass due to the
'Ãs_dma' condition above the patched code:
if (is_dma || request->actual == request->length) {
and then it will hit the code sending the final ZLP (above this patched code
too):
/*
* First, maybe a terminating short packet. Some DMA
* engines might handle this by themselves.
*/
if ((request->zero && request->length
&& request->length % musb_ep->packet_sz == 0)
#ifdef CONFIG_USB_INVENTRA_DMA
|| (is_dma && (!dma->desired_mode ||
(request->actual &
(musb_ep->packet_sz - 1
#endif
) {
before the transfer is complete while it should only be hit when and only when
the whole transfer is complete. The current code doesn't look correct as well
though, all due to this 'Ãs_dma' condition. Surely this needs fixing.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 6/6] USB: musb-gadget: fix dma length in txstate
Hello.
tom.leim...@gmail.com wrote:
From: Ming Lei
DMA length should not go beyond the availabe space of request buffer,
so fix it.
Signed-off-by: Ming Lei
Cc: David Brownell
Cc: Felipe Balbi
Cc: Anand Gadiyar
Cc: Mike Frysinger
Cc: Sergei Shtylyov
[...]
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index fa826f9..cacae96 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -300,6 +300,11 @@ static void txstate(struct musb *musb, struct musb_request
*req)
#ifndefCONFIG_MUSB_PIO_ONLY
if (is_dma_capable() && musb_ep->dma) {
struct dma_controller *c = musb->dma_controller;
+ size_t request_size;
+
+ /* setup DMA, then program endpoint CSR */
+ request_size = min_t(size_t, request->length - request->actual,
+ musb_ep->dma->max_len);
Er, you're moving this from under #ifdef CONFIG_USB_INVENTRA_DMA to the
common code, right? Do you know that not all DMA drivers initialize max_len? For
example CPPI driver doesn't, so it's left at zero. You're going to break DMA for
CPPI. Please extend your patch, adding cppi_dma.c to it.
@@ -307,11 +312,6 @@ static void txstate(struct musb *musb, struct musb_request
*req)
#ifdef CONFIG_USB_INVENTRA_DMA
{
- size_t request_size;
-
- /* setup DMA, then program endpoint CSR */
- request_size = min_t(size_t, request->length,
- musb_ep->dma->max_len);
if (request_size < musb_ep->packet_sz)
musb_ep->dma->desired_mode = 0;
else
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello.
Felipe Balbi wrote:
If a DMA interrupt comes when the whole transfer is not yet
complete (and
other Ming Lei's patches are making this possible),
Oh, here I mixed some other patch with Ming Lei's ones...
it will pass due to the
than this is the actual problem, no ? If we're using mode1 dma (as we
are on tx path), we should only get dma interrupt when the whole
transfer has been completed.
The Inventra DMA controller has serious DMA length limitation, so the whole
transfer may take more than one DMA.
'Ãs_dma' condition above the patched code:
if (is_dma || request->actual == request->length) {
and then it will hit the code sending the final ZLP (above this
patched code too):
but this was already there before the patch.
Yes, and here lies the problem.
/*
* First, maybe a terminating short packet.
Some DMA
* engines might handle this by themselves.
*/
if ((request->zero && request->length
&& request->length %
musb_ep->packet_sz == 0)
#ifdef CONFIG_USB_INVENTRA_DMA
|| (is_dma && (!dma->desired_mode ||
(request->actual &
(musb_ep->packet_sz -
1
#endif
) {
before the transfer is complete while it should only be hit when and
only when
the whole transfer is complete. The current code doesn't look correct
as well
though, all due to this 'Ãs_dma' condition. Surely this needs fixing.
likewise, this was there before the patch. I don't think the real
problem lies with this patch, it's been there for a while, don't you
agree ?
Then what problem this patch fixes, if not this one?
the problem is not on the extra if () added below the quoted code,
it's on the quoted code itself, which wasn't changed in any way.
Let me repeat: in the PIO mode the added check is just duplicate, in the DMA
mode it's added too late in the code, after ZLP/short packet send is triggered.
We should modify the check at the top of that code instead.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello. On 15-09-2010 10:53, Felipe Balbi wrote: If a DMA interrupt comes when the whole transfer is not yet complete (and other Ming Lei's patches are making this possible), Oh, here I mixed some other patch with Ming Lei's ones... it will pass due to the than this is the actual problem, no ? If we're using mode1 dma (as we are on tx path), we should only get dma interrupt when the whole transfer has been completed. The Inventra DMA controller has serious DMA length limitation, so the whole transfer may take more than one DMA. When I said 'whole transfer' I meant the transfer size you programmed dma to transfer, see that we have (not actual code): if (transfer_size > dma->max_len) transfer_size = dma->max_len; dma->channel_program(...,..., transfer_size,...); Ah, I didn't mean the same thing -- I meant the transfer size as specified by 'struct usb_request'. with mode1, we will only get irq when dma has transferred transfer_size bytes. Sure. likewise, this was there before the patch. I don't think the real problem lies with this patch, it's been there for a while, don't you agree ? Then what problem this patch fixes, if not this one? if request->length == 1MB and dma->max_len = 128KB, when is_dma is true, request->actual will be different from request->length for 7 'iterations', then only on the 8th it will be the same. I believe that's what Ming is trying to fix. I repeat once again: it's too late to check this where Ming is inserting the check. Ming, am I correct ? Let me repeat: in the PIO mode the added check is just duplicate, in the DMA it is duplicate for PIO, but not for DMA. I didn't say it was duplicate for DMA, just too late. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello. On 15-09-2010 14:05, Felipe Balbi wrote: I didn't say it was duplicate for DMA, just too late. how come ? we need to send ZLP before giving back the request. Well, look at the code ionce again. We need to send ZLP *after* request->actual == request->length, but as the check is inserted after the ZLP send, ZLP *may* be sent once the first DMA completes, not the last. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello. On 15-09-2010 14:14, Ming Lei wrote: I didn't say it was duplicate for DMA, just too late. how come ? we need to send ZLP before giving back the request. Well, look at the code ionce again. We need to send ZLP *after* request->actual == request->length, but as the check is inserted after the ZLP send, ZLP *may* be sent once the first DMA completes, not the last. Yes, it is really a problem, as said by balbi. And the problem should be in the check for zlp or the 'is_dma' condition. But this patch is not addressed for the zlp problem, and is is only for completing the request only if the data transfer in usb_request is over, as explained before, right? I don't see why we should fix only this problem, while it's obvious tha the fix is incomplete and leaves the other problem exposed. Please recast the patch. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
On 15.09.2010 14:22, Felipe Balbi wrote: I don't see why we should fix only this problem, while it's obvious tha the fix is incomplete and leaves the other problem exposed. Please recast the patch. IMO, the ZLP fix is *another* fix and as such subject to a different patch. IMHO, this fix as it is now is quite stupid. It's clear that the check is misplaced and will be removed once the ZLP fix is done. So why not do it once and for all? Is it so hard to do? FWIW, I NAK this patch as it is now. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Hello. On 15-09-2010 14:31, Felipe Balbi wrote: IMHO, this fix as it is now is quite stupid. It's clear that the check is misplaced and will be removed once the ZLP fix is done. So why not do it once your forgetting the fact that not always you need to send ZLP after completing a packet_size-aligned transfer, So what? so that check will have to stay where it is. I don't see how these two are connected at all. >> and for all? Is it so hard to do? FWIW, I NAK this patch as it is now. > ok, but I don't. Well, you're the maintainer. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND/PATCH 5/6] USB: musb-gadget: complete request only if data is transfered over
Felipe Balbi wrote:
your forgetting the fact that not always you need to send ZLP after
completing a packet_size-aligned transfer,
So what?
what you're saying is that when we change the ZLP handling, the
request->actual == request->lenght check will have to be removed, but
that's not true because ZLP is only needed if request->zero is set.
So the final code would be something like (pseudo-code):
if (request->lenght % musb_ep->packet_sz)
set_last_packet_is_short_flag(musb_request);
if (request->zero || last_packet_is_short(request)) {
set_txpktrdy(musb);
set_musb_request_completed_flag(musb_request);
return;
}
if (request->acual == request->length)
musb_g_giveback(musb, request);
restart_ep_if_more_requests(musb_ep);
No, this code will still send ZLP before the whole requested transfer is
done. The (request->actual == request->length) check is needed before we even
check that request->zero is set (and it is there but not for the DMA case). I
thought that this was quite obvious, and I was surprised that this caused such a
lengthy discussion.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Discussion] USB: musb-gadget: how to fix ZLP issue in musb_g_tx
Ming Lei wrote:
Hi All,
In another thread, Sergei pointed out there is a ZLP issue in musb_g_tx:
Sergei Shtylyov wrote:
Hello.
On 15-09-2010 14:05, Felipe Balbi wrote:
I didn't say it was duplicate for DMA, just too late.
how come ? we need to send ZLP before giving back the request.
Well, look at the code ionce again. We need to send ZLP *after*
request->actual == request->length, but as the check is inserted
after the ZLP send, ZLP *may* be sent once the first DMA completes,
not the last.
WBR, Sergei
balbi also confirmed that is is really a problem.
I also have two related questions below for the problem:
1), why is the check for "is_dma" needed here?
if (is_dma || request->actual == request->length) {
}
I'm not sure -- it seems erratic.
2), why is a zlp needed in the case below?
#ifdef CONFIG_USB_INVENTRA_DMA
|| (is_dma && (!dma->desired_mode ||
(request->actual & (musb_ep->packet_sz - 1
#endif
Seems no request->zero is set to ask for zlp explicitly in
the case above.
This is not for ZLP -- this is here to set TXPktRdy for the last short
packet in the Inventra DMA mode 0 that doesn't set TXPktRdy in such case.
IMO, it is not difficult to give a good fix for the ZLP problem
if the two questions are clear.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Discussion] USB: musb-gadget: how to fix ZLP issue in musb_g_tx
Hello. Felipe Balbi wrote: On Wed, Sep 15, 2010 at 06:02:22AM -0500, Ming Lei wrote: If so, once the dma interrupt comes, will request->actual be same with request->length in musb_g_tx? And if it is true, could we remove the check for 'is_dma'? see that is_dma is set to true by just checking if dma in enabled in txcsr, it might be that dma didn't complete everything and you need to write txpktrdy by hand to send last short packet. So to remove that you would need to re-work a bit more code. I don't see what to rework. The last short packet should still satisfy (request->actual == request->length) condition, no? You need to know when this is a dma IRQ or an endpoint IRQ. We know that -- but why check it there, before (request->actual == request->length)? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Discussion] USB: musb-gadget: how to fix ZLP issue in musb_g_tx
Hello. On 16-09-2010 10:05, Felipe Balbi wrote: I don't see what to rework. The last short packet should still satisfy (request->actual == request->length) condition, no? of course not, it's short not zero. so the last short packet can be anything from 1 to 511 bytes. Sigh. Where have I said anything different? What I meant is that the last short packet is already counted in request->actual, otherwise the condition (request->actual & (musb_ep->packet_sz - 1)) wouldn't work. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] OMAP: hwmod: Enable module wakeup if in smartidle
Hello. On 22-09-2010 4:19, Paul Walmsley wrote: From: Rajendra Nayak If a module's OCP slave port is programmed to be in smartidle, its also necessary that they have module level wakeup enabled. Update _sysc_enable in hwmod framework to do this. The thread "[PATCH 7/8] : Hwmod api changes" archived here: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34212.html has additional technical information on the rationale of this patch. Signed-off-by: Rajendra Nayak Signed-off-by: Partha Basak Signed-off-by: Benoît Cousson [p...@pwsan.com: revised patch description] Signed-off-by: Paul Walmsley Cc: Kevin Hilman --- arch/arm/mach-omap2/omap_hwmod.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index f320cfb..3f3d61a 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c [...] @@ -703,6 +701,10 @@ static void _sysc_enable(struct omap_hwmod *oh) _set_clockactivity(oh, oh->class->sysc->clockact,&v); _write_sysconfig(v, oh); + + /* If slave is in SMARTIDLE, also enable wakeup */ + if ((sf & SYSC_HAS_SIDLEMODE) && !(oh->flags & HWMOD_SWSUP_SIDLE)) + _enable_wakeup(oh); This line is overindented... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/9 v3] OMAP: Hwmod api changes
Hello. Hema HK wrote: OMAP USBOTG modules has a requirement to set the auto idle bit only after setting smart idle bit. Modified the _sys_enable api to set the smart idle first and then the autoidle bit. Setting this will not have any impact on the other modules. Signed-off-by: Hema HK Signed-off-by: Basak, Partha Cc: Felipe Balbi Cc: Tony Lindgren Cc: Kevin Hilman Cc: Cousson, Benoit Cc: Paul Walmsley --- arch/arm/mach-omap2/omap_hwmod.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c === --- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c +++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c [...] @@ -672,6 +666,19 @@ static void _sysc_enable(struct omap_hwm _set_clockactivity(oh, oh->class->sysc->clockact, &v); _write_sysconfig(v, oh); + + /* +* Set the auto idle bit only after setting the smartidle bit +* as this is requirement for some modules like USBOTG Need period at the end of sentense, and the next sentense should satrt with capital letter. Sorry for the grammar nitpicking. :-) +* setting this will not have any impact on the other modues. +*/ WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] omap4 hsmmc: Fix the init if CONFIG_MMC_OMAP_HS is not set
Hello.
kishore kadiyala wrote:
From: Benoit Cousson
Avoid possible crash if CONFIG_MMC_OMAP_HS is not set
Signed-off-by: Benoit Cousson
Signed-off-by: Kishore Kadiyala
---
arch/arm/mach-omap2/board-4430sdp.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/board-4430sdp.c
b/arch/arm/mach-omap2/board-4430sdp.c
index a49f285..ac8541c 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -240,8 +240,14 @@ static int omap4_twl6030_hsmmc_late_init(struct device
*dev)
static __init void omap4_twl6030_hsmmc_set_late_init(struct device *dev)
{
- struct omap_mmc_platform_data *pdata = dev->platform_data;
+ struct omap_mmc_platform_data *pdata;
+ /* dev can be null if CONFIG_MMC_OMAP_HS is not set */
+ if (!dev) {
+ pr_err("Failed omap4_twl6030_hsmmc_set_late_init\n");
pr_err("Failed %s\n", __func__);
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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 v3] usb : musb: Offmode fix for idle path
Hello.
On 23-09-2010 9:50, Maulik wrote:
-Original Message-
From: Hema HK [mailto:hem...@ti.com]
Sent: Thursday, September 23, 2010 6:01 AM
To: linux-omap@vger.kernel.org; linux-...@vger.kernel.org
Cc: Hema HK; Maulik Mankad; Felipe Balbi; Tony Lindgren; Kevin Hilman;
Cousson, Benoit; Paul Walmsley
Subject: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
With OMAP core-off support musb was not functional as context was getting
lost after wakeup from core-off. And also musb was blocking the core-off
after loading the gadget driver even with no cable connected sometimes.
Added the idle and wakeup APIs in the platform layer which will
be called in the idle and wakeup path.
Used the pm_runtime_put_sysc API to configure the
musb to force idle/standby modes, saving the context and disable the clk
in
while idling if there is no activity on the usb bus.
Used the pm_runtime_get_sync API to configure the musb to
no idle/standby modes, enable the clock and restore the context
after wakeup when there is no activity on the usb bus.
Signed-off-by: Hema HK
Signed-off-by: Maulik Mankad
Cc: Felipe Balbi
Cc: Tony Lindgren
Cc: Kevin Hilman
Cc: Cousson, Benoit
Cc: Paul Walmsley
[...]
Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
===
--- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
+++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
[...]
@@ -115,8 +126,101 @@ void __init usb_musb_init(struct omap_mu
put_device(dev);
}
+void musb_prepare_for_idle()
+{
+ int core_next_state;
+ struct omap_hwmod *oh = oh_p;
+ struct omap_device *od;
+ struct platform_device *pdev;
+ struct musb_hdrc_platform_data *pdata;
+ struct device *dev;
+
+ if (!core_pwrdm)
+ return;
+
+ core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+ if (core_next_state>= PWRDM_POWER_INACTIVE)
+ return;
+ if (!oh)
+ return;
+
+ od = oh->od;
+ pdev =&od->pdev;
+
+ if (!pdev)
+ return;
+ dev =&pdev->dev;
+
+ if (dev->driver) {
+ pdata = dev->platform_data;
+
+ if (pdata->is_usb_active)
Don't you need a start brace here?
No if the *if* only embraces one statement as here. But the next *if* can
be collapsed into this one.
Also a tab is required if this if condition is under if (dev->driver)
Yeah.
+ if (!pdata->is_usb_active(dev)) {
Brace not necessary.
+ if (core_next_state == PWRDM_POWER_OFF) {
+ pdata->save_context = 1;
+ pm_runtime_put_sync(dev);
+ } else if (core_next_state == PWRDM_POWER_RET) {
+ pdata->save_context = 0;
+ pm_runtime_put_sync(dev);
+ }
+ }
+ }
+}
+
+void musb_wakeup_from_idle()
+{
+ int core_next_state;
+ int core_prev_state;
+ struct omap_hwmod *oh = oh_p;
+ struct omap_device *od;
+ struct platform_device *pdev;
+ struct device *dev;
+ struct musb_hdrc_platform_data *pdata;
+
+ if (!core_pwrdm)
+ return;
+
+ core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
+
+ if (core_next_state>= PWRDM_POWER_INACTIVE)
+ return;
+core_prev_state = pwrdm_read_prev_pwrst(core_pwrdm);
+
+if (!oh)
+ return;
+od = oh->od;
+pdev =&od->pdev;
+
+if (!pdev)
+ return;
+
+dev =&pdev->dev;
+
+if (dev->driver) {
+ pdata = dev->platform_data;
+
+ if (pdata->is_usb_active)
Braces needed for this if condition?
No. But the next *if* can be collapsed into the previous.
+ if (!pdata->is_usb_active(dev)) {
+ if (core_prev_state == PWRDM_POWER_OFF) {
+ pdata->restore_context = 1;
+pm_runtime_get_sync(dev);
+ } else {
+pdata->restore_context = 0;
+pm_runtime_get_sync(dev);
+ }
+}
+}
+}
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Hello. Felipe Balbi wrote: I guess that's Felipe's call, but I don't like that option. I think it's cleaner to have the ->set_clock hook be a noop on OMAP and the runtime hooks be noops on the other platforms. Agreed. We should focus on removing ->set_clock for .38 actually. Is DaVinci already using clkdev, Kevin ? Sure. But DaVinci doesn't use ->set_clock(). WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Hello. On 27-09-2010 10:07, Felipe Balbi wrote: I guess that's Felipe's call, but I don't like that option. I think it's cleaner to have the ->set_clock hook be a noop on OMAP and the runtime hooks be noops on the other platforms. Agreed. We should focus on removing ->set_clock for .38 actually. Is DaVinci already using clkdev, Kevin ? Sure. But DaVinci doesn't use ->set_clock(). Ok, so seems like no-one is actually using that. I thought OMAP does... but I'm seeing now that it doesn't. We can already start patching to remove that thing, later on, we need to remove the clock name via platform_data. As I've said already, there are cases where two clocks is needed by MUSB (like AM3517) or where USB 2.0 clock is also needed by the OHCI driver (DA8xx), hence the name seems needed still... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] leds: pwm: add a new element 'name' to platform data
Hello.
On 28-09-2010 14:35, Arun Murthy wrote:
A new element 'name' is added to pwm led platform data structure.
This is required to identify the pwm device.
Signed-off-by: Arun Murthy
Acked-by: Linus Walleij
[...]
diff --git a/include/linux/leds_pwm.h b/include/linux/leds_pwm.h
index 33a0711..7a847a0 100644
--- a/include/linux/leds_pwm.h
+++ b/include/linux/leds_pwm.h
@@ -16,6 +16,7 @@ struct led_pwm {
struct led_pwm_platform_data {
int num_leds;
struct led_pwm *leds;
+ char *name;
};
Shouldn't '*name'be aligned, at least with '*leds'?
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3 v4] musb: AM35x: Workaround for fifo read issue
Hello.
Ajay Kumar Gupta wrote:
AM35x supports only 32bit read operations so we need to have
workaround for 8bit and 16bit read operations.
Signed-off-by: Ajay Kumar Gupta
drivers/usb/musb/am35x.c | 30 ++
drivers/usb/musb/musb_core.c |2 ++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index ee0c104..dce99fc 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -508,3 +508,33 @@ void musb_platform_restore_context(struct musb *musb,
phy_on();
}
#endif
+
+/* AM35x supports only 32bit read operation */
+void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst)
+{
+ void __iomem *fifo = hw_ep->fifo;
+ u32 val;
+ int i;
+
+ /* Read for 32bit-aligned destination address */
+ if (likely((0x03 & (unsigned long) dst) == 0) && len >= 4) {
+ readsl(fifo, dst, len >> 2);
+ dst += len & ~0x03;
+ len &= 0x03;
+ }
+ /*
+* Now read the rest 1 to 3 bytes or complete length if
s/rest/remaining/
+* unaligned address.
+*/
+ if (len > 4) {
+ for (i = 0; i < (len >> 2); i++) {
+ *(u32 *) dst = musb_readl(fifo, 0);
+ dst += 4;
+ }
+ len %= 4;
Why not 'len &= 0x03' like above?
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v4] musb: add musb support for AM35x
Hello.
Ajay Kumar Gupta wrote:
AM35x has musb interface and uses CPPI4.1 DMA engine.
Current patch supports only PIO mode. DMA support can be
added later once basic CPPI4.1 DMA patch is accepted.
Also added USB_MUSB_AM35X which is required to differentiate musb ips
between OMAP3x and AM35x. This config would be used to for below
purposes,
- Select am35x.c instead of omap2430.c for compilation
at drivers/usb/musb directory. Please note there are
significant differneces in these two files as musb ip
in quite different on AM35x.
- Select workaround codes applicable for AM35x musb issues.
one such workaround is for bytewise read issue on AM35x.
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 1dd21c2..0941a32 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -60,6 +60,17 @@ comment "OMAP 44xx high speed USB support"
comment "Blackfin high speed USB Support"
depends on USB_MUSB_HDRC && ((BF54x && !BF544) || (BF52x && !BF522 &&
!BF523))
+config USB_MUSB_AM35X
+ boolean "AM35X MUSB support"
+ depends on USB_MUSB_HDRC && MACH_OMAP3517EVM
As I've already said, depending on the board type won't scale... :-(
+ select NOP_USB_XCEIV
+ default y
+ help
+ Select this option if your platform is based on AM35x. As
+ AM35x has an updated MUSB with CPPI4.1 DMA so this config
+ is introduced to differentiate musb ip between OMAP3x and
+ AM35x platforms.
OK, but why it's made visible?
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
new file mode 100644
index 000..ee0c104
--- /dev/null
+++ b/drivers/usb/musb/am35x.c
@@ -0,0 +1,510 @@
+/*
+ * Texas Instruments AM35x "glue layer"
+ *
+ * Copyright (c) 2010, by Texas Instruments
+ *
+ * Based on the DA8xx "glue layer" code.
+ * Copyright (C) 2005-2006 by Texas Instruments
There's no code by TI in the DA8xx layer -- that copyright comes from the
DaVinci code.
+ * Copyright (c) 2008, MontaVista Software, Inc.
I've extended it to 2008-2009 (and should have to 2010 :-).
+/* USB interrupt register bits */
+#define USB_INTR_USB_SHIFT 16
+#define USB_INTR_USB_MASK (0x1ff << USB_INTR_USB_SHIFT)
Don't seem like good names (USB_ repeated twice)...
[...]
+int __init musb_platform_init(struct musb *musb, void *board_data)
+{
+ void __iomem *reg_base = musb->ctrl_base;
+ struct clk *otg_fck;
+ u32 rev, lvl_intr, sw_reset;
+ int status;
+
+ musb->mregs += USB_MENTOR_CORE_OFFSET;
+
+ if (musb->set_clock)
+ musb->set_clock(musb->clock, 1);
OMAP doesn't use plat->set_clock anymore, does it?
+ else
+ clk_enable(musb->clock);
+ DBG(2, "usbotg_ck=%lud\n", clk_get_rate(musb->clock));
+
+ otg_fck = clk_get(musb->controller, "fck");
+ if (IS_ERR(otg_fck)) {
+ status = PTR_ERR(otg_fck);
+ otg_fck = NULL;
This assignment does not seem necessary.
+ goto exit0;
+ }
[...]
+exit1:
+ clk_disable(otg_fck);
+exit0:
+ clk_disable(musb->clock);
+ return status;
+}
+
+int musb_platform_exit(struct musb *musb)
+{
+ struct clk *otg_fck;
+
+ if (is_host_enabled(musb))
+ del_timer_sync(&otg_workaround);
+
+ phy_off();
+
+ otg_put_transceiver(musb->xceiv);
+ usb_nop_xceiv_unregister();
+
+ if (musb->set_clock)
+ musb->set_clock(musb->clock, 0);
Looks like it may be dropped...
+ else
+ clk_disable(musb->clock);
+
+ otg_fck = clk_get(musb->controller, "fck");
Isn't it better to store this in some static variable? I don't think there's
or there'll be multiple instances of MUSB on AM35x...
+ if (IS_ERR(otg_fck)) {
+ DBG(2, "clk_get() failed for otg_fck.\n");
+ } else {
+ clk_put(otg_fck);
+ clk_put(otg_fck);
+ clk_disable(otg_fck);
+ }
+
+ return 0;
+}
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v4] musb: add musb support for AM35x
Hello.
Igor Grinberg wrote:
AM35x has musb interface and uses CPPI4.1 DMA engine.
Current patch supports only PIO mode. DMA support can be
added later once basic CPPI4.1 DMA patch is accepted.
Also added USB_MUSB_AM35X which is required to differentiate musb ips
between OMAP3x and AM35x. This config would be used to for below
purposes,
- Select am35x.c instead of omap2430.c for compilation
at drivers/usb/musb directory. Please note there are
significant differneces in these two files as musb ip
in quite different on AM35x.
- Select workaround codes applicable for AM35x musb issues.
one such workaround is for bytewise read issue on AM35x.
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
new file mode 100644
index 000..ee0c104
--- /dev/null
+++ b/drivers/usb/musb/am35x.c
@@ -0,0 +1,510 @@
[...]
+int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
+{
+ u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
+
+ devconf2 &= ~CONF2_OTGMODE;
+ switch (musb_mode) {
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
+ case MUSB_HOST: /* Force VBUS valid, ID = 0 */
+ devconf2 |= CONF2_FORCE_HOST;
+ break;
+#endif
+#ifdef CONFIG_USB_GADGET_MUSB_HDRC
+ case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */
+ devconf2 |= CONF2_FORCE_DEVICE;
+ break;
+#endif
+#ifdef CONFIG_USB_MUSB_OTG
+ case MUSB_OTG: /* Don't override the VBUS/ID comparators */
+ devconf2 |= CONF2_NO_OVERRIDE;
This does nothing, can be removed...
Well, I think you should let it live -- for completeness...
+int musb_platform_exit(struct musb *musb)
+{
+ struct clk *otg_fck;
+
+ if (is_host_enabled(musb))
+ del_timer_sync(&otg_workaround);
+
+ phy_off();
+
+ otg_put_transceiver(musb->xceiv);
+ usb_nop_xceiv_unregister();
+
+ if (musb->set_clock)
+ musb->set_clock(musb->clock, 0);
+ else
+ clk_disable(musb->clock);
+
+ otg_fck = clk_get(musb->controller, "fck");
+ if (IS_ERR(otg_fck)) {
+ DBG(2, "clk_get() failed for otg_fck.\n");
+ } else {
+ clk_put(otg_fck);
+ clk_put(otg_fck);
+ clk_disable(otg_fck);
I think the order should be:
clk_disable(...);
clk_put(...);
Right...
And of course, it should be put only once... ;)
clk_get() is called twice, here and in musb_platform_init(), and so is
clk_put().
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] OMAP: split plat-omap/common.c
Hello. On 04-10-2010 23:15, Paul Walmsley wrote: --- /dev/null +++ b/arch/arm/plat-omap/32ksynctimer.c @@ -0,0 +1,184 @@ +/* + * linux/arch/arm/plat-omap/clocksource.c What name do you want to use? 32ksynctimer.c or clocksource.c? Thanks, that's a bug. And also note that filenames in heading comments have been long discouraged. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] AM35xx: Craneboard: Add USB EHCI support
Hello.
On 16-12-2010 18:25, srin...@mistralsolutions.com wrote:
From: Srinath
AM3517/05 Craneboard has one EHCI interface on board using port1.
GPIO35 is used as power enable.
GPIO38 is used as port1 PHY reset.
History:
http://marc.info/?l=linux-omap&w=2&r=1&s=Craneboard%3A+Add+USB+EHCI+support&q=t
Signed-off-by: Srinath
---
arch/arm/mach-omap2/board-am3517crane.c | 53 +++
1 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-am3517crane.c
b/arch/arm/mach-omap2/board-am3517crane.c
index 8ba4047..1a80175 100644
--- a/arch/arm/mach-omap2/board-am3517crane.c
+++ b/arch/arm/mach-omap2/board-am3517crane.c
[...]
@@ -51,10 +58,56 @@ static void __init am3517_crane_init_irq(void)
[...]
static void __init am3517_crane_init(void)
{
+ int ret;
+
omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
omap_serial_init();
+
+ /* Configure GPIO for EHCI port */
+ if (omap_mux_init_gpio(GPIO_USB_NRESET, OMAP_PIN_OUTPUT)) {
+ pr_err("Can not cofigure mux for GPIO_USB_NRESET %d\n",
+ GPIO_USB_NRESET);
+ return;
+ }
+
+ if (omap_mux_init_gpio(GPIO_USB_POWER, OMAP_PIN_OUTPUT)) {
+ pr_err("Can not cofigure mux for GPIO_USB_POWER %d\n",
+
Empty line not needed here...
+ GPIO_USB_POWER);
+ return;
+ }
+
+ ret = gpio_request(GPIO_USB_POWER, "usb_ehci_enable");
+ if (ret < 0) {
+ pr_err("Cannot request GPIO %d\n", GPIO_USB_POWER);
+ return;
+ }
+
+ ret = gpio_direction_output(GPIO_USB_POWER, 1);
+ if (ret < 0)
+ goto err;
+
+
Too many empty lines here...
+ usb_ehci_init(&ehci_pdata);
+ return;
+
+err:
There's no need for *goto* and label.
+ gpio_free(GPIO_USB_POWER);
+ pr_err("Unable to initialize EHCI power\n");
+ return;
}
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: am35x: fix compile error due to control apis
Hello. On 30-12-2010 15:27, Felipe Balbi wrote: >Is this change slated to go into 2.6.37? As it stands it looks like >2.6.37 will be released with completely broken musb support on many boards >(including the BeagleBoard). yes, it is. Are you sure? We are currently at -rc8 and yet this patch has yet to make it into my tree. Yes. The patch is waiting for the merge window, check linux-next or Greg's usb-next branch. And yet he was asking you about 2.6.37. :-) WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: am35x: fix compile error due to control apis
Hello. Felipe Balbi wrote: Yes. The patch is waiting for the merge window, check linux-next or Greg's usb-next branch. So I guess no working USB in 2.6.37 in that case? For am35x, I guess not. All in all, it was a change to omap's arch code which "broke" am35x in the first place. Well, the driver shouldn't be using that header anyway. This was a dubious decision, IMHO. It would have been better to accept the initial patch to make things compile in 2.6.37, and then do the big change for 2.6.38... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/3] ARM: add CPPI 4.1 DMA support
Hello. On 03-01-2011 20:01, Gupta, Ajay Kumar wrote: Add support for Texas Instuments Communication Port Programming Interface 4.1 (CPPI 4.1) used on OMAP-L1x/DA8xx and AM35x. At this moment, only the DMA controller and queue manager are supported. Support for the buffer manager is lacking but these chips don't have it anyway. Signed-off-by: Sergei Shtylyov Signed-off-by: Sekhar Nori Russell, you have recently discarded this patch from your patch system. Can we know the reason? It was recently discussed with TI, and various people raised concerns about it. I don't remember the exact details, and I wish they'd raise them with the patch author directly. It may have been that it's inventing its own API rather than using something like the DMA engine API. Adding linux-omap to try to get a response there. Sergei, This issue was discussed recently at TI and proposal was to place it to drivers/dma folder. Note that I have neither time nor inclination to do this work (not do I think it's even feasible), so it will have to fall on TI's shoulders... Moreover, even Felipe also seems to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma. Frankly speaking, I doubt that drivers/dma/ will have place for the purely MUSB specific DMA engines such as the named ones (there's no TUSB DMA BTW -- it uses OMAP DMA). Regards, Ajay WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/3] ARM: add CPPI 4.1 DMA support
Hello. On 03-01-2011 23:44, Felipe Balbi wrote: > Moreover, even Felipe also seems to move other musb > DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma. Frankly speaking, I doubt that drivers/dma/ will have place for the purely MUSB specific DMA engines such as the named ones (there's no TUSB DMA BTW it uses OMAP DMA). I think we will get more clarity once we start on this activity. I agree, but I personally don't see that many limiting factors. dmaengine is just a generic API for doing DMA transfers. If it's not enough for us currently, we extend it. Putting MUSB DMA enignes into drivers/dma/ is the same as taking *any* chip capable of bus-mastering DMA, "separating" its bus mastering related code from its driver and putting this code into drivers/dma/. This doesn't make sense, in my opinion. drivers/dma/ is for the dedicated DMA controllers (which can *optionally* serve the slave devices). WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/3] ARM: add CPPI 4.1 DMA support
Hello. Felipe Balbi wrote: I think we will get more clarity once we start on this activity. I agree, but I personally don't see that many limiting factors. dmaengine is just a generic API for doing DMA transfers. If it's not enough for us currently, we extend it. Putting MUSB DMA enignes into drivers/dma/ is the same as taking *any* chip capable of bus-mastering DMA, "separating" its bus mastering related code from its driver and putting this code into drivers/dma/. This doesn't make sense, in my opinion. drivers/dma/ is for the dedicated DMA controllers (which can *optionally* serve the slave devices). Do I really have to spell it out ? Really ? Yes, I'm dense. :-) Especially after Ajay claiming that Mentor and CPPI 3.0 DMA will be moved to drivers/dma/... You don't need to physically move the part of the code to drivers/dma, but it has to use the API. The mentor DMA is internal to MUSB. tusb6010_omap.c isn't. Yes, that's what I've already noted in this thread. Where it makes sense to move the code under drivers/dma, it will be Surely OMAP DMA needs to be moved under drivers/dma/, not the TUSB code interfacing it. done, where it doesn't, it won't be done, but it will use the same API. That's all. I don't quite see how DMA engine API is beneficial to what we currently have... The end goal is just to drop all these ad-hoc "APIs" for accessing DMA on musb code. The "ad-hoc" API is well suited for use with MUSB, while DMA engine API is more abstract, I think. The "ad-hoc" API takes into account some things that the DMA engine API just can't -- like the transfer mode and packet size... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/3] ARM: add CPPI 4.1 DMA support
Hello. Felipe Balbi wrote: On Tue, Jan 04, 2011 at 11:41:05PM +0800, Ming Lei wrote: OMAP GPIOs have many usages or use cases, so we can use gpiolib to simplify access to GPIOs. If GPIOs has only one usage or use case, it is not necessary to access GPIOs by gpiolib. Now this kind of DMA controllers are only used by MUSB or only for MUSB, so it doesn't matter to access them by dmaengine or not. Not entirely true. TUSB uses OMAP system DMA and AFAICT CPPI is used also for ethernet. Yes, but the CPPI registers are not compatible between MUSB and EMAC, only the descriptors are, AFAIK. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: mach-omap2: potential NULL dereference
Hello.
On 17-01-2011 13:08, Vasiliy Kulikov wrote:
kzalloc() may fail, if so return -ENOMEM.
Signed-off-by: Vasiliy Kulikov
[...]
diff --git a/arch/arm/mach-omap2/smartreflex.c
b/arch/arm/mach-omap2/smartreflex.c
index 77ecebf..871bca9 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -260,7 +260,10 @@ static int sr_late_init(struct omap_sr *sr_info)
if (sr_class->class_type == SR_CLASS2&&
sr_class->notify_flags&& sr_info->irq) {
+ ret = -ENOMEM;
name = kzalloc(SMARTREFLEX_NAME_LEN + 1, GFP_KERNEL);
+ if (name == NULL)
+ goto error;
Why not:
if (name == NULL) {
ret = -ENOMEM;
goto error;
}
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/2] OMAP3: Devkit8000: Change lcd power pin
Hello. Thomas Weber wrote: The reset_gpio pin for lcd is connected with TWL4030 LedA. TWL4030 GPIO.1 has a not connected resistor. Fix indention issue. Indentation. The comment line uses 8 whitespaces. Replaced with one tabulator. Reported-by: Daniel Morsing Signed-off-by: Thomas Weber WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 3/4] OMAP3: Devkit8000: Check return value of gpio_request
Hello.
On 19-01-2011 11:19, Thomas Weber wrote:
The return value of gpio_request is ignored.
This patch adds the check of the return value of gpio_request.
Signed-off-by: Thomas Weber
---
arch/arm/mach-omap2/board-devkit8000.c | 16 ++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/board-devkit8000.c
b/arch/arm/mach-omap2/board-devkit8000.c
index 9fb416b..4ddd81c 100644
--- a/arch/arm/mach-omap2/board-devkit8000.c
+++ b/arch/arm/mach-omap2/board-devkit8000.c
[...]
@@ -244,13 +246,23 @@ static int devkit8000_twl_gpio_setup(struct device *dev,
/* TWL4030_GPIO_MAX + 0 is "LCD_PWREN" (out, active high) */
devkit8000_lcd_device.reset_gpio = gpio + TWL4030_GPIO_MAX + 0;
- gpio_request(devkit8000_lcd_device.reset_gpio, "LCD_PWREN");
+ ret = gpio_request(devkit8000_lcd_device.reset_gpio, "LCD_PWREN");
+ if (ret < 0) {
+ printk(KERN_ERR "Failed to request GPIO for LCD_PWRN\n");
+ return ret;
+ }
+
/* Disable until needed */
gpio_direction_output(devkit8000_lcd_device.reset_gpio, 0);
/* gpio + 7 is "DVI_PD" (out, active low) */
devkit8000_dvi_device.reset_gpio = gpio + 7;
- gpio_request(devkit8000_dvi_device.reset_gpio, "DVI PowerDown");
+ ret = gpio_request(devkit8000_dvi_device.reset_gpio, "DVI PowerDown");
+ if (ret < 0) {
+ printk(KERN_ERR "Failed to request GPIO for DVI PowerDown\n");
You forgot to call:
gpio_free(devkit8000_lcd_device.reset_gpio);
+ return ret;
+ }
+
/* Disable until needed */
gpio_direction_output(devkit8000_dvi_device.reset_gpio, 0);
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: oprofile: Always allow backtraces
Hello. On 19-01-2011 23:54, Ari Kauppi wrote: Always allow backtrace when using oprofile on ARM, even if a PMU isn't present. Restores functionality originally introduced in 1b7b56982fdcd9d85effd76f3928cf5d6eb26155 by Richard Purdie. You're lacking the commit summary specified in parens which Linus asked to add. Signed-off-by: Ari Kauppi Cc: Richard Purdie Cc: Will Deacon Cc: Matt Fleming WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 18/18] OMAP2,3: DSS2: Get DSS IRQ from platform device
Hello.
On 22-01-2011 13:05, Sumit Semwal wrote:
From: Senthilvadivu Guruswamy
DSS IRQ number can be obtained from platform_get_irq(). This API in turn
picks the right IRQ number belonging to HW IP from the hwmod database.
So hardcoding of IRQ number could be removed.
Reviewed-by: Paul Walmsley
Reviewed-by: Kevin Hilman
Tested-by: Kevin Hilman
Signed-off-by: Senthilvadivu Guruswamy
Signed-off-by: Sumit Semwal
[...]
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 4d7a816..22690e9 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
[...]
@@ -609,15 +609,18 @@ static int dss_init(bool skip_init)
REG_FLD_MOD(DSS_CONTROL, 0, 2, 2); /* venc clock mode = normal */
#endif
- r = request_irq(INT_24XX_DSS_IRQ,
+ dss_irq = platform_get_irq(dss.pdev, 0);
+ if (dss_irq != -ENXIO) {
Better just 'dss_irq > 0', IMHO.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] usb: otg: OMAP4430: Add phy_suspend function pointer to twl4030_usb_data
Hello. On 03-02-2011 12:49, Hema HK wrote: Introduce the .phy_suspend function pointer to twl4030_usb_data structure. assign the function to it for both sdp board and panda boards. This will be used by the twl6030-usb transceiver driver. Signed-off-by: Hema HK Cc: Felipe Balbi Cc: Tony Lindgren Index: linux-2.6/arch/arm/plat-omap/include/plat/usb.h === --- linux-2.6.orig/arch/arm/plat-omap/include/plat/usb.h +++ linux-2.6/arch/arm/plat-omap/include/plat/usb.h @@ -88,6 +88,7 @@ extern int omap4430_phy_power(struct dev extern int omap4430_phy_set_clk(struct device *dev, int on); extern int omap4430_phy_init(struct device *dev); extern int omap4430_phy_exit(struct device *dev); +extern int omap4430_phy_suspend(struct device *dev, int suspend); And where it it defined? Why this doesn't happen in this patch? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] usb: otg: OMAP4430: Add phy_suspend function pointer to twl4030_usb_data
Hello.
On 03-02-2011 12:49, Hema HK wrote:
Introduce the .phy_suspend function pointer to twl4030_usb_data structure.
assign the function to it for both sdp board and panda boards.
This will be used by the twl6030-usb transceiver driver.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
[...]
Index: linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-4430sdp.c
+++ linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
@@ -272,6 +272,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-omap4panda.c
+++ linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
@@ -153,6 +153,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/plat-omap/include/plat/usb.h
===
--- linux-2.6.orig/arch/arm/plat-omap/include/plat/usb.h
+++ linux-2.6/arch/arm/plat-omap/include/plat/usb.h
@@ -88,6 +88,7 @@ extern int omap4430_phy_power(struct dev
extern int omap4430_phy_set_clk(struct device *dev, int on);
extern int omap4430_phy_init(struct device *dev);
extern int omap4430_phy_exit(struct device *dev);
+extern int omap4430_phy_suspend(struct device *dev, int suspend);
#endif
Index: linux-2.6/include/linux/i2c/twl.h
===
--- linux-2.6.orig/include/linux/i2c/twl.h
+++ linux-2.6/include/linux/i2c/twl.h
@@ -600,6 +600,8 @@ struct twl4030_usb_data {
int (*phy_power)(struct device *dev, int iD, int on);
/* enable/disable phy clocks */
int (*phy_set_clock)(struct device *dev, int on);
+ /* suspend/resume of phy */
+ int (*phy_suspend)(struct device *dev, int suspend);
};
I think this patch should contain only this change, and the initializers
and 'extern' declaration should be merged to the previous patch.
Also, where do you call this method? Ah, I see, in the next patch. I think
it should be merged to this one then.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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: mach-omap2: board-rm680: fix rm680_vemmc regulator constraints
Hello. On 01-02-2011 18:36, Aaro Koskinen wrote: With the commit 757902513019e6ee469791ff76f954b19ca8d036 fixed voltage Please also specify the commit summary in parens, as asked by Linus. regulator setup will fail if there are voltage constraints defined. This made MMC unusable on this board. Fix by just deleting those redundant constraints. Signed-off-by: Aaro Koskinen WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [4/7 v2] usb: otg: OMAP4430: Add phy_suspend function pointer to
Hello.
On 15-02-2011 12:42, Hema HK wrote:
The subject seems incomplete.
From: Kalliguddi, Hema
Introduce the .phy_suspend function pointer to twl4030_usb_data structure.
assign the function to it for both sdp board and panda boards.
This will be used by the twl6030-usb transceiver driver.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
[...]
Index: linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-4430sdp.c
+++ linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
@@ -272,6 +272,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-omap4panda.c
+++ linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
@@ -153,6 +153,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/plat-omap/include/plat/usb.h
===
--- linux-2.6.orig/arch/arm/plat-omap/include/plat/usb.h
+++ linux-2.6/arch/arm/plat-omap/include/plat/usb.h
@@ -88,6 +88,7 @@ extern int omap4430_phy_power(struct dev
extern int omap4430_phy_set_clk(struct device *dev, int on);
extern int omap4430_phy_init(struct device *dev);
extern int omap4430_phy_exit(struct device *dev);
+extern int omap4430_phy_suspend(struct device *dev, int suspend);
I think this *extern* declaration should be a part of the previous patch.
#endif
Index: linux-2.6/include/linux/i2c/twl.h
===
--- linux-2.6.orig/include/linux/i2c/twl.h
+++ linux-2.6/include/linux/i2c/twl.h
@@ -600,6 +600,8 @@ struct twl4030_usb_data {
int (*phy_power)(struct device *dev, int iD, int on);
/* enable/disable phy clocks */
int (*phy_set_clock)(struct device *dev, int on);
+ /* suspend/resume of phy */
+ int (*phy_suspend)(struct device *dev, int suspend);
};
I'd make the above the only change in this patch, and add all the other
changes into the previous patch (which then I'd change places with that one).
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [5/7 v2] usb: otg: TWL6030: Introduce the twl6030_phy_suspend function.
Hello.
On 15-02-2011 12:42, Hema HK wrote:
From: Kalliguddi, Hema
Introduce the twl6030_phy_suspend function and assign to otg.set_suspend
function pointer.
This function is used by the musb-omap2430 platform driver
during suspend/resume.
Signed-off-by: Hema HK
Cc: Felipe Balbi
[...]
Index: linux-2.6/drivers/usb/otg/twl6030-usb.c
===
--- linux-2.6.orig/drivers/usb/otg/twl6030-usb.c
+++ linux-2.6/drivers/usb/otg/twl6030-usb.c
@@ -177,6 +177,20 @@ static void twl6030_phy_shutdown(struct
pdata->phy_power(twl->dev, 0, 0);
}
+static int twl6030_phy_suspend(struct otg_transceiver *x, int suspend)
+{
+ struct twl6030_usb *twl;
+ struct device *dev;
+ struct twl4030_usb_data *pdata;
+
+ twl = xceiv_to_twl(x);
+ dev = twl->dev;
+ pdata = dev->platform_data;
Why not do all the above in the intializers?
+ pdata->phy_suspend(twl->dev, suspend);
+
+ return 0;
+}
+
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
Hello.
On 15-02-2011 16:20, David Cohen wrote:
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohen
---
arch/arm/mach-omap2/iommu2.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32
*ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
+ dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
for (i = 0; i< ARRAY_SIZE(err_msg); i++) {
if (stat & (1<< i))
- printk("%s ", err_msg[i]);
+ printk(KERN_DEBUG "%s ", err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
}
- printk("\n");
+ printk(KERN_DEBUG "\n");
Here too... Although wait, it should be KERN_CONT instead! Debug levels
are only attributed to the whole lines.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On 15-02-2011 16:44, David Cohen wrote:
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohen
---
arch/arm/mach-omap2/iommu2.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
+ dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
That's my plan.
for (i = 0; i < ARRAY_SIZE(err_msg); i++) {
if (stat& (1<
Re: [4/7 v2] usb: otg: OMAP4430: Add phy_suspend function pointer to
Hello.
On 16-02-2011 8:14, Hema Kalliguddi wrote:
From: Kalliguddi, Hema
Introduce the .phy_suspend function pointer to
twl4030_usb_data structure.
assign the function to it for both sdp board and panda boards.
This will be used by the twl6030-usb transceiver driver.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
[...]
Index: linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-4430sdp.c
+++ linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
@@ -272,6 +272,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-omap4panda.c
+++ linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
@@ -153,6 +153,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/plat-omap/include/plat/usb.h
===
--- linux-2.6.orig/arch/arm/plat-omap/include/plat/usb.h
+++ linux-2.6/arch/arm/plat-omap/include/plat/usb.h
@@ -88,6 +88,7 @@ extern int omap4430_phy_power(struct dev
extern int omap4430_phy_set_clk(struct device *dev, int on);
extern int omap4430_phy_init(struct device *dev);
extern int omap4430_phy_exit(struct device *dev);
+extern int omap4430_phy_suspend(struct device *dev, int suspend);
I think this *extern* declaration should be a part of the
previous patch.
What is the problem if extern when using?
I think 'extern' declaration should be in the same place where this
function is defined, else you're just defining an unused function. Same goes
for the 'phy_suspend' initializers...
#endif
Index: linux-2.6/include/linux/i2c/twl.h
===
--- linux-2.6.orig/include/linux/i2c/twl.h
+++ linux-2.6/include/linux/i2c/twl.h
@@ -600,6 +600,8 @@ struct twl4030_usb_data {
int (*phy_power)(struct device *dev, int iD, int on);
/* enable/disable phy clocks */
int (*phy_set_clock)(struct device *dev, int on);
+ /* suspend/resume of phy */
+ int (*phy_suspend)(struct device *dev, int suspend);
};
I'd make the above the only change in this patch, and add
all the other
changes into the previous patch (which then I'd change places
with that one).
No. if I do that git bisect fails.
How in the world it will fail?
Initializer does not make any sense without function poointer declaration.
I said "which then I'd changed places with that one", i.e. put this patch
first and the previous patch second. Otherwise, you're just doing things
beckwards: first you define the method implementation, and then you declare
the method itself... it should be vice versa.
Regards,
Hema
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [4/7 v3] usb: otg: OMAP4430: Add phy_suspend function pointer to twl4030_usb_data
On 16-02-2011 14:50, Hema HK wrote:
From: Kalliguddi, Hema
Introduce the .phy_suspend function pointer to twl4030_usb_data structure.
assign the function to it for both sdp board and panda boards.
This will be used by the twl6030-usb transceiver driver.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
---
arch/arm/mach-omap2/board-4430sdp.c|1 +
arch/arm/mach-omap2/board-omap4panda.c |1 +
arch/arm/plat-omap/include/plat/usb.h |1 +
include/linux/i2c/twl.h|2 ++
4 files changed, 5 insertions(+)
Index: linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-4430sdp.c
+++ linux-2.6/arch/arm/mach-omap2/board-4430sdp.c
@@ -272,6 +272,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/board-omap4panda.c
+++ linux-2.6/arch/arm/mach-omap2/board-omap4panda.c
@@ -153,6 +153,7 @@ static struct twl4030_usb_data omap4_usb
.phy_exit = omap4430_phy_exit,
.phy_power = omap4430_phy_power,
.phy_set_clock = omap4430_phy_set_clk,
+ .phy_suspend= omap4430_phy_suspend,
};
static struct omap2_hsmmc_info mmc[] = {
Index: linux-2.6/include/linux/i2c/twl.h
===
--- linux-2.6.orig/include/linux/i2c/twl.h
+++ linux-2.6/include/linux/i2c/twl.h
@@ -600,6 +600,8 @@ struct twl4030_usb_data {
int (*phy_power)(struct device *dev, int iD, int on);
/* enable/disable phy clocks */
int (*phy_set_clock)(struct device *dev, int on);
+ /* suspend/resume of phy */
+ int (*phy_suspend)(struct device *dev, int suspend);
};
struct twl4030_ins {
You're still doing things backwards. Sigh...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On 16-02-2011 14:53, Hema HK wrote: From: Hema HK Moved all the board specific internal PHY functions out of usb_musb.c file as this file is shared between the OMAP2+ and AM35xx platforms. There exists a file which has the functions specific to internal PHY used for OMAP4 platform. Moved all phy specific functions to this file and passing these functions through board data in the board file. Signed-off-by: Hema HK Index: linux-2.6/arch/arm/mach-omap2/board-am3517evm.c === --- linux-2.6.orig/arch/arm/mach-omap2/board-am3517evm.c +++ linux-2.6/arch/arm/mach-omap2/board-am3517evm.c @@ -409,6 +409,10 @@ static struct omap_musb_board_data musb_ .interface_type = MUSB_INTERFACE_ULPI, .mode = MUSB_OTG, .power = 500, + .set_phy_power = am35x_musb_phy_power, + .clear_irq = am35x_musb_clear_irq, + .set_mode = am35x_musb_set_mode, + .reset = am35x_musb_reset, }; static __init void am3517_evm_musb_init(void) [...] Index: linux-2.6/arch/arm/mach-omap2/Makefile === --- linux-2.6.orig/arch/arm/mach-omap2/Makefile +++ linux-2.6/arch/arm/mach-omap2/Makefile @@ -218,7 +218,8 @@ obj-$(CONFIG_MACH_OMAP4_PANDA) += board hsmmc.o \ omap_phy_internal.o -obj-$(CONFIG_MACH_OMAP3517EVM) += board-am3517evm.o +obj-$(CONFIG_MACH_OMAP3517EVM) += board-am3517evm.o \ + omap_phy_internal.o \ obj-$(CONFIG_MACH_CRANEBOARD) += board-am3517crane.o Doesn't the above board needs board data modified as well? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] OMAP2+: hwmod: Avoid setup if clock lookup failed
Hello.
On 16.02.2011 15:11, Rajendra Nayak wrote:
Add a hwmod state check in the _setup function
to avoid setting up hwmods' for which clock
lookup has failed.
Signed-off-by: Rajendra Nayak
[...]
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e282e35..cd9dcde 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1362,6 +1362,12 @@ static int _setup(struct omap_hwmod *oh, void *data)
int i, r;
u8 postsetup_state;
+ if (oh->_state != _HWMOD_STATE_CLKS_INITED) {
+ WARN(1, "omap_hwmod: %s: _setup failed as one or more"
You forgot space bafore " -- "moreclock" will be printed.
+"clock lookups' have failed\n", oh->name);
Why there's apostrophe here?
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] OMAP2+: musb: hwmod adaptation for musb registration
Hello.
On 16-02-2011 15:28, Hema HK wrote:
Using omap_device_build API instead of platform_device_register for
OMAP2430,OMAP3xxx, OMAP4430 and AM35x musb device registration.
The device specific resources defined in centralized
database will be used.
Signed-off-by: Hema HK
Cc: Felipe Balbi
Cc: Tony Lindgren
Cc: Kevin Hilman
Cc: Cousson, Benoit
Cc: Paul Walmsley
[...]
Index: linux-2.6/arch/arm/mach-omap2/usb-musb.c
===
--- linux-2.6.orig/arch/arm/mach-omap2/usb-musb.c
+++ linux-2.6/arch/arm/mach-omap2/usb-musb.c
[...]
@@ -115,8 +88,35 @@ void __init usb_musb_init(struct omap_mu
musb_plat.mode = board_data->mode;
musb_plat.extvbus = board_data->extvbus;
- if (platform_device_register(&musb_device)< 0)
- printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
+ if (cpu_is_omap3517() || cpu_is_omap3505()) {
+ oh_name = "am35x_otg_hs";
+ name = "musb-am35x";
+ } else {
+ oh_name = "usb_otg_hs";
+ name = "musb-omap2430";
+ }
The following code is over-indented...
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh) {
+ pr_err("Could not look up %s\n", oh_name);
+ return;
+ }
+
+ od = omap_device_build(name, bus_id, oh, &musb_plat,
+ sizeof(*pdata), omap_musb_latency,
+ ARRAY_SIZE(omap_musb_latency), false);
+ if (IS_ERR(od)) {
+ pr_err("Could not build omap_device for %s %s\n",
+ name, oh_name);
+ return;
+
Empty line not needed here...
+ }
+ pdev =&od->pdev;
+ dev =&pdev->dev;
+ get_device(dev);
+ dev->dma_mask = &musb_dmamask;
+ dev->coherent_dma_mask = musb_dmamask;
+ put_device(dev);
+
And here...
}
#else
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello.
Ajay Kumar Gupta wrote:
Isochronous Tx DMA is getting programmed but never getting started
for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
That's not true.
Fixing it by starting DMAs using musb_h_tx_dma_start().
Signed-off-by: Swaminathan S
Signed-off-by: Babu Ravi
Signed-off-by: Ajay Kumar Gupta
NAK.
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index cf94511..e3ab40a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
- offset, length))
+ offset, length)) {
+ if (is_cppi_enabled() || tusb_dma_omap())
+ musb_h_tx_dma_start(hw_ep);
return;
It will have been already started by this moment -- from
musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
made sure it *does* work.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello, I wrote: Isochronous Tx DMA is getting programmed but never getting started for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work. That's not true. Well, this is only true iff URB_ISO_ASAP flag is *not* set for an URB. In this case, PIO is also not being started, so you should fix this too. Signed-off-by: Swaminathan S Signed-off-by: Babu Ravi Signed-off-by: Ajay Kumar Gupta Fixing it by starting DMAs using musb_h_tx_dma_start(). NAK. Still NAK... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello.
Gupta, Ajay Kumar wrote:
-Original Message-
From: Sergei Shtylyov [mailto:sshtyl...@ru.mvista.com]
Sent: Friday, August 28, 2009 2:53 PM
To: Gupta, Ajay Kumar
Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
felipe.ba...@nokia.com; davi...@pacbell.net; Subbrathnam, Swaminathan; B,
Ravi
Subject: Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello.
Ajay Kumar Gupta wrote:
Isochronous Tx DMA is getting programmed but never getting started
for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
That's not true.
We have tested and it doesn't work with current driver. Have you tested it and
was it working for you ?
Not with the current driver, I must admit. I'll try it today...
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index cf94511..e3ab40a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
- offset, length))
+ offset, length)) {
+ if (is_cppi_enabled() || tusb_dma_omap())
+ musb_h_tx_dma_start(hw_ep);
return;
It will have been already started by this moment -- from
musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
made sure it *does* work.
This part is being done at musb_host_rx()
You're doing it in musb_host_tx() actually. Although musb_host_rx()
is also broken WRT the isochronous transfers.
doing next packet programming within same urb and *not* starting next urb.
Thus musb_start_urb() doesn't come into this path.
What? Read the code, please -- musb_start_urb() call should always
precede musb_host_tx() which handles the DMA interrupt. Unless something
clears DMAReqEnab after musb_start_urb() call, setting it only once
should work.
So it wouldn't start the DMAs.
How musb_host_tx() can be called without musb_start_urb()?
In case of PIO, it does load the FIFO and sets the TXPKTREADY.
Only is URB_ISO_ASAP is not set.
WBR, Sergei
-Ajay
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello, I wrote: In case of PIO, it does load the FIFO and sets the TXPKTREADY. Only is URB_ISO_ASAP is not set. This should read "only if URB_ISO_ASAP is set". :-/ WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello.
Gupta, Ajay Kumar wrote:
diff --git a/drivers/usb/musb/musb_host.c
b/drivers/usb/musb/musb_host.c
index cf94511..e3ab40a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
- offset, length))
+ offset, length)) {
+ if (is_cppi_enabled() || tusb_dma_omap())
+ musb_h_tx_dma_start(hw_ep);
return;
It will have been already started by this moment -- from
musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
made sure it *does* work.
This part is being done at musb_host_rx()
You're doing it in musb_host_tx() actually. Although musb_host_rx()
is also broken WRT the isochronous transfers.
doing next packet programming within same urb and *not* starting next
urb. Thus musb_start_urb() doesn't come into this path.
What? Read the code, please -- musb_start_urb() call should always
precede musb_host_tx() which handles the DMA interrupt. Unless something
clears DMAReqEnab after musb_start_urb() call, setting it only once
should work.
musb_start_urb() call does precede musb_host_tx() but when urb is *completed*.
check the 'done' flag and musb_advance_schedule getting called in the path.
if (done) {
/* set status */
urb->status = status;
urb->actual_length = qh->offset;
musb_advance_schedule(musb, urb, hw_ep, USB_DIR_OUT);
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
offset, length)) {
if (is_cppi_enabled() || tusb_dma_omap()
|| is_cppi41_enabled())
musb_h_tx_dma_start(hw_ep);
return;
}
Sigh, that will be musb_start_urb() for the *next* URB after a
completed one... Someone must have called it for the *current* URB when
starting an ISO transfer.
This call to musb_tx_dma_program() is only done for the 2nd and
subsequent data fragments of an URB -- the call for the 1st fragment
happens elsewhere, from musb_ep_program().
There are basically two Tx URB starting paths within the driver:
1) musb_urb_enqueue() -> musb_schedule() -> musb_start_urb() ->
musb_h_tx_dma_start();
2) musb_host_tx() -> musb_advance_schedule() -> musb_start_urb() ->
musb_h_tx_dma_start().
Transfer of the 1st fragment is started on either of these patch,
depending on whether there was URBs already queued at the time of
submitting the new URB; after that DMAReqMode/DMAReqEnab bits should
remain set after the first musb_h_tx_dma_start() call, so that calling
musb_tx_dma_program() should be enough for the subsequent fragments. So
your statement that "Isochronous Tx DMA is getting programmed but never
getting started for CPPI and TUSB DMAs" is an overstatement in any case
-- first fragment must be properly started.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Gupta, Ajay Kumar wrote: This part is being done at musb_host_rx() You're doing it in musb_host_tx() actually. Although musb_host_rx() is also broken WRT the isochronous transfers. doing next packet programming within same urb and *not* starting next urb. Thus musb_start_urb() doesn't come into this path. What? Read the code, please -- musb_start_urb() call should always precede musb_host_tx() which handles the DMA interrupt. Unless something clears DMAReqEnab after musb_start_urb() call, setting it only once should work. musb_start_urb() call does precede musb_host_tx() but when urb is *completed*. I think you are aware that there are multiple packets within same isochronous urb and musb_start_urb() programs only for first packet. case USB_ENDPOINT_XFER_ISOC: qh->iso_idx = 0; qh->frame = 0; offset = urb->iso_frame_desc[0].offset; len = urb->iso_frame_desc[0].length; Sure. What I'm still not aware of is where and how the TXCSR DMA bits are cleared after the first fragment. Hopefully, testing will reveal this... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello, I wrote: This part is being done at musb_host_rx() You're doing it in musb_host_tx() actually. Although musb_host_rx() is also broken WRT the isochronous transfers. doing next packet programming within same urb and *not* starting next urb. Thus musb_start_urb() doesn't come into this path. What? Read the code, please -- musb_start_urb() call should always precede musb_host_tx() which handles the DMA interrupt. Unless something clears DMAReqEnab after musb_start_urb() call, setting it only once should work. musb_start_urb() call does precede musb_host_tx() but when urb is *completed*. I think you are aware that there are multiple packets within same isochronous urb and musb_start_urb() programs only for first packet. case USB_ENDPOINT_XFER_ISOC: qh->iso_idx = 0; qh->frame = 0; offset = urb->iso_frame_desc[0].offset; len = urb->iso_frame_desc[0].length; Sure. What I'm still not aware of is where and how the TXCSR DMA bits are cleared after the first fragment. Hopefully, testing will reveal this... I really should have stared at the code a bit more myself. Now that I have the sad truth has dawned on me... :-/ My patch "USB: musb: bugfixes for multi-packet TXDMA support" (commit c7bbc056a92476b3b3d70a8df7cc746ac5d56de7) inevitably causes the DMA bits to be cleared because we're using DMA mode 1 regardless of whether this is isochronous transfer or no. (We could really use mode 0 for isochronous transfer and save the hassle when filtering out DMA completion interrupt in musb_host_tx().) So, I now have to ACK your patch (which could use a better description though) and strew my head with ashes. All this also must mean that I have managed to break ISO Tx DMA in the internal tree, where I added the abovementioned patch after the one that fixed it (the order of the patches in the community tree is reverse). I probably did not retest USB audio back then... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello, I wrote: You're doing it in musb_host_tx() actually. Although musb_host_rx() is also broken WRT the isochronous transfers. doing next packet programming within same urb and *not* starting next urb. Thus musb_start_urb() doesn't come into this path. What? Read the code, please -- musb_start_urb() call should always precede musb_host_tx() which handles the DMA interrupt. Unless something clears DMAReqEnab after musb_start_urb() call, setting it only once should work. musb_start_urb() call does precede musb_host_tx() but when urb is *completed*. I think you are aware that there are multiple packets within same isochronous urb and musb_start_urb() programs only for first packet. case USB_ENDPOINT_XFER_ISOC: qh->iso_idx = 0; qh->frame = 0; offset = urb->iso_frame_desc[0].offset; len = urb->iso_frame_desc[0].length; Sure. What I'm still not aware of is where and how the TXCSR DMA bits are cleared after the first fragment. Hopefully, testing will reveal this... I really should have stared at the code a bit more myself. Now that I have the sad truth has dawned on me... :-/ My patch "USB: musb: bugfixes for multi-packet TXDMA support" (commit c7bbc056a92476b3b3d70a8df7cc746ac5d56de7) inevitably causes the DMA bits You can also refer to this patch'es brokenness in your patch description. Looks like I now have the complete picture of what happened back then when the patches were (re)submitted. The DaVinci case was competely broken when I recast the patch "USB: musb: fix isochronous TXDMA (take 2)" to fix the failure to transfer the whole ISO URB in DMA mode for the Mentor's own DMA case; before that, DaVinci case could (likewise) complete the URB transfer in PIO mode after transferring the first fragment via DMA... So, I now have to ACK your patch (which could use a better description though) and strew my head with ashes. All this also must mean that I have managed to break ISO Tx DMA in the internal tree, where I added the abovementioned patch after the one that fixed it (the order of the patches in the community tree is reverse). I probably did not retest USB audio back then... No, looks like in the internal tree transfer is completed in PIO mode since the internal version of patch lacks that Mentor DMA fix (luckily :-). Should still check to make sure... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello. Subbrathnam, Swaminathan wrote: Sergei, Pl. do the required testing with and without the patch on the current tree for ISO transfers in Tx direction. As Ajay indicated we have done the same and found it not working and hence the fix. Sigh, I'm now seeing it even witout testing... So, I'm sorry for all the noise. It was a result of my 2 patches clashing. ISO Rx is also broken and the patch for fixing the same is on the way. That's good to hear... musb_host_rx() was further broken WRT ISO trabnsers while the Mentor DMA case was being fixed. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Ajay Kumar Gupta wrote:
Isochronous Tx DMA is getting programmed but never getting started
for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
Fixing it by starting DMAs using musb_h_tx_dma_start().
Signed-off-by: Swaminathan S
Signed-off-by: Babu Ravi
Signed-off-by: Ajay Kumar Gupta
Acked-by: Sergei Shtylyov
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index cf94511..e3ab40a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
- offset, length))
+ offset, length)) {
+ if (is_cppi_enabled() || tusb_dma_omap())
The latter check shouldn't really be needed, as in the TUSB DMA mode
only DMA mode 0 is used, in which case the DMA interrupt filtering code
above in this function shouldn't clear the TXCSR.DMAReEnab bit.
+ musb_h_tx_dma_start(hw_ep);
return;
+ }
} else if (tx_csr & MUSB_TXCSR_DMAENAB) {
DBG(1, "not complete, but DMA enabled?\n");
return;
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] musb: fix compilation warning in host only mode
Hello.
Ajay Kumar Gupta wrote:
Fixes below compilation warning when host only configuration is
selected.
drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
Also removed definition of 'mbase' from multiple places to only
at function top.
AFAIR, it was intentionally removed from the function top and declared in
the multiple plcase instead by the former Felipe's patch [1] to fix exactly
the same issue, if I don't mistake. So, it hasn't worked out?
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index f0ff893..7cc8398 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
u8 devctl, u8 power)
{
irqreturn_t handled = IRQ_NONE;
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
+ void __iomem*mbase = musb->mregs;
+#endif
I'd rather see it declared multiple times...
@@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
if (int_usb & MUSB_INTR_CONNECT) {
struct usb_hcd *hcd = musb_to_hcd(musb);
- void __iomem *mbase = musb->mregs;
We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block.
handled = IRQ_HANDLED;
musb->is_active = 1;
[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=aa4714560b4ea359bb7830188ebd06bce71bcdea
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode
Hello. Ajay Kumar Gupta wrote: From: Maulik Mankad As a part of aligning the ISR code for MUSB with the specs, the ISR code was re-written. See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle irqs in the order dictated by programming guide) With this the suspend interrupt came accidently under CONFIG_USB_MUSB_HDRC_HCD. The fix brings suspend interrupt handling outside CONFIG_USB_MUSB_HDRC_HCD. Signed-off-by: Maulik Mankad Acked-by: Felipe Balbi Cc: David Brownell Signed-off-by: Ajay Kumar Gupta [...] diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 4f43db7..64b08f9 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, handled = IRQ_HANDLED; } - +#endif Could you move #endif one line up, so that it closely embraces the *if* statement? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] musb: fix compilation warning in host only mode
Gupta, Ajay Kumar wrote:
Fixes below compilation warning when host only configuration is
selected.
drivers/usb/musb/musb_core.c: In function 'musb_stage0_irq':
drivers/usb/musb/musb_core.c:711: warning: unused variable 'mbase'
Also removed definition of 'mbase' from multiple places to only
at function top.
AFAIR, it was intentionally removed from the function top and declared
in the multiple plcase instead by the former Felipe's patch [1] to fix
exactly the same issue, if I don't mistake. So, it hasn't worked out?
Yes, it was removed by Felipe's below patch but it introduced
compilation warning issue as reported.
---
commit aa4714560b4ea359bb7830188ebd06bce71bcdea
usb: musb: core: declare mbase only where it's used
... and avoid a compilation if we disable host side
of musb.
--
Signed-off-by: Ajay Kumar Gupta
[...]
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index f0ff893..7cc8398 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -455,6 +455,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
u8 devctl, u8 power)
{
irqreturn_t handled = IRQ_NONE;
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
+ void __iomem*mbase = musb->mregs;
+#endif
I'd rather see it declared multiple times...
I was thinking it's better to have it at one place and avoid multiple
#ifdefs.
You'd only need one #ifdef: in the placae where the warning was
reported. And you can open a block under #ifdef CONFIG_USB_MUSB_OTG to
declare it in. Other declarations are already covered by #ifdef's,
aren't they?
-Ajay
@@ -703,7 +700,6 @@ static irqreturn_t musb_stage0_irq(struct musb
*musb, u8 int_usb,
if (int_usb & MUSB_INTR_CONNECT) {
struct usb_hcd *hcd = musb_to_hcd(musb);
- void __iomem *mbase = musb->mregs;
We could also move this into the #ifdef CONFIG_USB_MUSB_OTG block.
Yeah, the block where 'mbase' is actually used.
BTW, your patch didn't cover the declaration in #if 0'ed out SOF
handler.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode
Hello. Maulik wrote: -Original Message- From: Sergei Shtylyov [mailto:sshtyl...@mvista.com] Sent: Thursday, June 17, 2010 4:57 PM To: Ajay Kumar Gupta Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; felipe.ba...@nokia.com; gre...@suse.de; Maulik Mankad; David Brownell Subject: Re: [PATCH 8/8] usb: musb: Fix suspend interrupt issue in device mode Hello. Ajay Kumar Gupta wrote: From: Maulik Mankad As a part of aligning the ISR code for MUSB with the specs, the ISR code was re-written. See Commit 1c25fda4a09e8229800979986ef399401053b46e (usb: musb: handle irqs in the order dictated by programming guide) With this the suspend interrupt came accidently under CONFIG_USB_MUSB_HDRC_HCD. The fix brings suspend interrupt handling outside CONFIG_USB_MUSB_HDRC_HCD. Signed-off-by: Maulik Mankad Acked-by: Felipe Balbi Cc: David Brownell Signed-off-by: Ajay Kumar Gupta [...] diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 4f43db7..64b08f9 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -635,7 +635,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, handled = IRQ_HANDLED; } - +#endif Could you move #endif one line up, so that it closely embraces the *if* statement? The patch is already in Greg's queue. Hm, how come it ended up there before the patches submitted earlier -- namely before the patch that's needed for the suspend interrupt to be handled at all? :-/ Felipe, could you also ACK the other pending patches, not this single one? Greg, Do you want me to resend this patch? Thanks, Maulik WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
