Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
* Bin Liu | 2013-07-23 13:23:57 [-0500]: Hi Sebastian, Hi Liu, On Mon, Jul 22, 2013 at 1:09 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. I guess the EOI register is removed from the TRM because AM33xx does not use it, there is no need to write to it to acknowledge. It does not hurt to write to it though since the register still exists, it just does nothing, I guess. Is it really there or was it never there and it has been added to TRM by accident? But I am not sure if it is a good idea to remove eoi from the musb_dsps driver. If the intension is to merge the support for other SoC, such as AM35xx, AM18xx, then EOI handling might be still needed. I just don't know how those devices use EOI. If one of the architectures gets added which need an EOI then the offset can be 0 and the EOI will happen only if it is != 0. Regards, -Bin. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
Sebastian, On Thu, Jul 25, 2013 at 9:41 AM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: * Bin Liu | 2013-07-23 13:23:57 [-0500]: Hi Sebastian, Hi Liu, On Mon, Jul 22, 2013 at 1:09 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. I guess the EOI register is removed from the TRM because AM33xx does not use it, there is no need to write to it to acknowledge. It does not hurt to write to it though since the register still exists, it just does nothing, I guess. Is it really there or was it never there and it has been added to TRM by accident? The EOI register IS in the USB subsystem of AM33xx, but the SoC does not use it because it uses level triggering for USB interrupt. But I am not sure if it is a good idea to remove eoi from the musb_dsps driver. If the intension is to merge the support for other SoC, such as AM35xx, AM18xx, then EOI handling might be still needed. I just don't know how those devices use EOI. If one of the architectures gets added which need an EOI then the offset can be 0 and the EOI will happen only if it is != 0. This patch cleaned up the use of EOI. Do you mean EOI handling will be added back with condition EOI offset != 0, when the support of new device which uses EIO is added? Regards, -Bin. Regards, -Bin. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
Sebastian, On Thu, Jul 25, 2013 at 10:16 AM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/25/2013 05:12 PM, Bin Liu wrote: Sebastian, Hi Bin, Is it really there or was it never there and it has been added to TRM by accident? The EOI register IS in the USB subsystem of AM33xx, but the SoC does not use it because it uses level triggering for USB interrupt. I see. But I am not sure if it is a good idea to remove eoi from the musb_dsps driver. If the intension is to merge the support for other SoC, such as AM35xx, AM18xx, then EOI handling might be still needed. I just don't know how those devices use EOI. If one of the architectures gets added which need an EOI then the offset can be 0 and the EOI will happen only if it is != 0. This patch cleaned up the use of EOI. Do you mean EOI handling will be added back with condition EOI offset != 0, when the support of new device which uses EIO is added? That is my intention. Then should something like EOI cleanup be added into the commit message for better git log searching experience? I would think the EOI cleanup is more important then variable renaming in this patch. Or even better to separate the changes into two patches. Regards, -Bin. Sebastian Regards, -Bin. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
Hi, On Thu, Jul 25, 2013 at 10:28:37AM -0500, Bin Liu wrote: Sebastian, On Thu, Jul 25, 2013 at 10:16 AM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/25/2013 05:12 PM, Bin Liu wrote: Sebastian, Hi Bin, Is it really there or was it never there and it has been added to TRM by accident? The EOI register IS in the USB subsystem of AM33xx, but the SoC does not use it because it uses level triggering for USB interrupt. I see. But I am not sure if it is a good idea to remove eoi from the musb_dsps driver. If the intension is to merge the support for other SoC, such as AM35xx, AM18xx, then EOI handling might be still needed. I just don't know how those devices use EOI. If one of the architectures gets added which need an EOI then the offset can be 0 and the EOI will happen only if it is != 0. This patch cleaned up the use of EOI. Do you mean EOI handling will be added back with condition EOI offset != 0, when the support of new device which uses EIO is added? That is my intention. Then should something like EOI cleanup be added into the commit message for better git log searching experience? I would think the EOI cleanup is more important then variable renaming in this patch. Or even better to separate the changes into two patches. perhaps two separate patches would be best, indeed. At least it would make it simpler to track regressions. -- balbi signature.asc Description: Digital signature
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
Hi Sebastian, On 7/22/2013 11:39 PM, Sebastian Andrzej Siewior wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. AM335X TRM Section 16.6.5 and 16.7.5 describes about EOI registers. Its at offset 0x24. Or is it that the interrupts are acknowledged even without writing to eoi register? -- -George -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
On 07/23/2013 08:04 AM, George Cherian wrote: Hi Sebastian, On 7/22/2013 11:39 PM, Sebastian Andrzej Siewior wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. AM335X TRM Section 16.6.5 and 16.7.5 describes about EOI registers. Its at offset 0x24. Or is it that the interrupts are acknowledged even without writing to eoi register? I have here Literature Number: SPRUH73H October 2011 Revised April 2013 which calls itself AM335x ARM® CortexTM-A8 Microprocessors (MPUs) Technical Reference Manual. This document ends with 16.5 that means there is no chapter 16.6 or 16.7. The next one is 17. 16.5.2 and 16.5.3 describes the available registers of USB[01]_CTRL REGISTERS which is where the EOI register is accessed. I see here 20h USB0IRQMSTAT 28h USB0IRQSTATRAW0 so no 0x24 at least in my document. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
On 7/23/2013 2:39 PM, Sebastian Andrzej Siewior wrote: On 07/23/2013 08:04 AM, George Cherian wrote: Hi Sebastian, On 7/22/2013 11:39 PM, Sebastian Andrzej Siewior wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. AM335X TRM Section 16.6.5 and 16.7.5 describes about EOI registers. Its at offset 0x24. Or is it that the interrupts are acknowledged even without writing to eoi register? I have here Literature Number: SPRUH73H October 2011 Revised April 2013 which calls itself AM335x ARM® CortexTM-A8 Microprocessors (MPUs) Technical Reference Manual. Looks like I have an old TRM with me, which has EOI register at 24h. I need to update my TRM. ;) This document ends with 16.5 that means there is no chapter 16.6 or 16.7. The next one is 17. 16.5.2 and 16.5.3 describes the available registers of USB[01]_CTRL REGISTERS which is where the EOI register is accessed. I see here 20h USB0IRQMSTAT 28h USB0IRQSTATRAW0 so no 0x24 at least in my document. Sebastian -- -George -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
(re-send, due to last delivery failure to the mailing list...) On Tue, Jul 23, 2013 at 1:23 PM, Bin Liu binml...@gmail.com wrote: Hi Sebastian, On Mon, Jul 22, 2013 at 1:09 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. I guess the EOI register is removed from the TRM because AM33xx does not use it, there is no need to write to it to acknowledge. It does not hurt to write to it though since the register still exists, it just does nothing, I guess. But I am not sure if it is a good idea to remove eoi from the musb_dsps driver. If the intension is to merge the support for other SoC, such as AM35xx, AM18xx, then EOI handling might be still needed. I just don't know how those devices use EOI. Regards, -Bin. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
This patch renames the type struct from ti81xx_driver_data to
am33xx_driver_data since it is not used for ti81xx anymore. The EOI
member is also removed since the am33xx SoC does not have such register.
The interrupt is acknowledged by writting into the stat register.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/usb/musb/musb_dsps.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 37ebcbc..2e45723 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -74,7 +74,6 @@ struct dsps_musb_wrapper {
u16 revision;
u16 control;
u16 status;
- u16 eoi;
u16 epintr_set;
u16 epintr_clear;
u16 epintr_status;
@@ -160,7 +159,6 @@ static void dsps_musb_disable(struct musb *musb)
dsps_writel(reg_base, wrp-epintr_clear,
wrp-txep_bitmap | wrp-rxep_bitmap);
dsps_writeb(musb-mregs, MUSB_DEVCTL, 0);
- dsps_writel(reg_base, wrp-eoi, 0);
}
static void otg_timer(unsigned long _musb)
@@ -271,7 +269,7 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
/* Get usb core interrupts */
usbintr = dsps_readl(reg_base, wrp-coreintr_status);
if (!usbintr !epintr)
- goto eoi;
+ goto out;
musb-int_usb = (usbintr wrp-usb_bitmap) wrp-usb_shift;
if (usbintr)
@@ -339,15 +337,11 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
if (musb-int_tx || musb-int_rx || musb-int_usb)
ret |= musb_interrupt(musb);
- eoi:
- /* EOI needs to be written for the IRQ to be re-asserted. */
- if (ret == IRQ_HANDLED || epintr || usbintr)
- dsps_writel(reg_base, wrp-eoi, 1);
-
/* Poll for ID change */
if (musb-xceiv-state == OTG_STATE_B_IDLE)
mod_timer(glue-timer, jiffies + wrp-poll_seconds * HZ);
+out:
spin_unlock_irqrestore(musb-lock, flags);
return ret;
@@ -395,9 +389,6 @@ static int dsps_musb_init(struct musb *musb)
val = ~(1 wrp-otg_disable);
dsps_writel(musb-ctrl_base, wrp-phy_utmi, val);
- /* clear level interrupt */
- dsps_writel(reg_base, wrp-eoi, 0);
-
return 0;
}
@@ -579,11 +570,10 @@ static int dsps_remove(struct platform_device *pdev)
return 0;
}
-static const struct dsps_musb_wrapper ti81xx_driver_data = {
+static const struct dsps_musb_wrapper am33xx_driver_data = {
.revision = 0x00,
.control= 0x14,
.status = 0x18,
- .eoi= 0x24,
.epintr_set = 0x38,
.epintr_clear = 0x40,
.epintr_status = 0x30,
@@ -610,7 +600,7 @@ static const struct dsps_musb_wrapper ti81xx_driver_data = {
static const struct of_device_id musb_dsps_of_match[] = {
{ .compatible = ti,musb-am33xx,
- .data = (void *) ti81xx_driver_data, },
+ .data = (void *) am33xx_driver_data, },
{ },
};
MODULE_DEVICE_TABLE(of, musb_dsps_of_match);
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
