| Message ID | 20170813120423.16371-2-abailon@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: > The DA8xx and DSPS platforms don't use the same address for few registers. > On Da8xx, this is causing some issues (e.g. teardown that doesn't work). > Configure the address of the register during the init and use them instead > of constants. > > Reported-by: nsekhar@ti.com > Signed-off-by: Alexandre Bailon <abailon@baylibre.com> > --- > drivers/usb/musb/musb_cppi41.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c > index ba255280a624..dbff0e0a4ff5 100644 > --- a/drivers/usb/musb/musb_cppi41.c > +++ b/drivers/usb/musb/musb_cppi41.c > @@ -26,6 +26,9 @@ > > #define MUSB_DMA_NUM_CHANNELS 15 > > +#define DA8XX_USB_AUTOREQ_REG 0x14 > +#define DA8XX_USB_TEARDOWN_REG 0x1c Nice catch. I noticed that the USB_CTRL_TX_MODE and USB_CTRL_RX_MODE are incorrect for DA8xx too. Perhaps those should be fixed too. Also, since the original code does not use the _REG suffix for register offsets, perhaps the new defines should avoid those as well for consistency. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 14 August 2017 07:01 PM, Sekhar Nori wrote: > Hi, > > On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: >> The DA8xx and DSPS platforms don't use the same address for few registers. >> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). >> Configure the address of the register during the init and use them instead >> of constants. >> >> Reported-by: nsekhar@ti.com Reported-by: Sekhar Nori <nsekhar@ti.com> >> Signed-off-by: Alexandre Bailon <abailon@baylibre.com> >> --- >> drivers/usb/musb/musb_cppi41.c | 23 +++++++++++++++++++---- >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c >> index ba255280a624..dbff0e0a4ff5 100644 >> --- a/drivers/usb/musb/musb_cppi41.c >> +++ b/drivers/usb/musb/musb_cppi41.c >> @@ -26,6 +26,9 @@ >> >> #define MUSB_DMA_NUM_CHANNELS 15 >> >> +#define DA8XX_USB_AUTOREQ_REG 0x14 >> +#define DA8XX_USB_TEARDOWN_REG 0x1c > > Nice catch. I noticed that the USB_CTRL_TX_MODE and USB_CTRL_RX_MODE are > incorrect for DA8xx too. Perhaps those should be fixed too. Ah, just read 2/2 now, and seems like thats exactly what you are handling there. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello! On 8/13/2017 3:04 PM, Alexandre Bailon wrote: > The DA8xx and DSPS platforms don't use the same address for few registers. > On Da8xx, this is causing some issues (e.g. teardown that doesn't work). > Configure the address of the register during the init and use them instead > of constants. > > Reported-by: nsekhar@ti.com > Signed-off-by: Alexandre Bailon <abailon@baylibre.com> > --- > drivers/usb/musb/musb_cppi41.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c > index ba255280a624..dbff0e0a4ff5 100644 > --- a/drivers/usb/musb/musb_cppi41.c > +++ b/drivers/usb/musb/musb_cppi41.c > @@ -26,6 +26,9 @@ > > #define MUSB_DMA_NUM_CHANNELS 15 > > +#define DA8XX_USB_AUTOREQ_REG 0x14 > +#define DA8XX_USB_TEARDOWN_REG 0x1c Why these _REG suffixes suddenly? [...] Other than that looks sane. Need my ACK? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 14 August 2017 07:06 PM, Sekhar Nori wrote: > On Monday 14 August 2017 07:01 PM, Sekhar Nori wrote: >> Hi, >> >> On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: >>> The DA8xx and DSPS platforms don't use the same address for few registers. >>> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). >>> Configure the address of the register during the init and use them instead >>> of constants. >>> >>> Reported-by: nsekhar@ti.com > > Reported-by: Sekhar Nori <nsekhar@ti.com> Tested-by: Sekhar Nori <nsekhar@ti.com> Hi Bin, Do you have any additional comments on this series or are you waiting for v2 to be posted? Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 04, 2017 at 06:32:11PM +0530, Sekhar Nori wrote: > On Monday 14 August 2017 07:06 PM, Sekhar Nori wrote: > > On Monday 14 August 2017 07:01 PM, Sekhar Nori wrote: > >> Hi, > >> > >> On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: > >>> The DA8xx and DSPS platforms don't use the same address for few registers. > >>> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). > >>> Configure the address of the register during the init and use them instead > >>> of constants. > >>> > >>> Reported-by: nsekhar@ti.com > > > > Reported-by: Sekhar Nori <nsekhar@ti.com> > > Tested-by: Sekhar Nori <nsekhar@ti.com> > > Hi Bin, > > Do you have any additional comments on this series or are you waiting > for v2 to be posted? I don't have other comments, just am waiting for v2. Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bin, Sekhar On 09/07/2017 07:16 PM, Bin Liu wrote: > On Mon, Sep 04, 2017 at 06:32:11PM +0530, Sekhar Nori wrote: >> On Monday 14 August 2017 07:06 PM, Sekhar Nori wrote: >>> On Monday 14 August 2017 07:01 PM, Sekhar Nori wrote: >>>> Hi, >>>> >>>> On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: >>>>> The DA8xx and DSPS platforms don't use the same address for few registers. >>>>> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). >>>>> Configure the address of the register during the init and use them instead >>>>> of constants. >>>>> >>>>> Reported-by: nsekhar@ti.com >>> Reported-by: Sekhar Nori <nsekhar@ti.com> >> Tested-by: Sekhar Nori <nsekhar@ti.com> >> >> Hi Bin, >> >> Do you have any additional comments on this series or are you waiting >> for v2 to be posted? > I don't have other comments, just am waiting for v2. My apologize. I was in vacation. I will send the v2 soon. Best Regards, Alexandre > > Regards, > -Bin. > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/15/2017 12:12 PM, Sergei Shtylyov wrote: > Hello! > > On 8/13/2017 3:04 PM, Alexandre Bailon wrote: > >> The DA8xx and DSPS platforms don't use the same address for few >> registers. >> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). >> Configure the address of the register during the init and use them >> instead >> of constants. >> >> Reported-by: nsekhar@ti.com >> Signed-off-by: Alexandre Bailon <abailon@baylibre.com> >> --- >> drivers/usb/musb/musb_cppi41.c | 23 +++++++++++++++++++---- >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/musb/musb_cppi41.c >> b/drivers/usb/musb/musb_cppi41.c >> index ba255280a624..dbff0e0a4ff5 100644 >> --- a/drivers/usb/musb/musb_cppi41.c >> +++ b/drivers/usb/musb/musb_cppi41.c >> @@ -26,6 +26,9 @@ >> #define MUSB_DMA_NUM_CHANNELS 15 >> +#define DA8XX_USB_AUTOREQ_REG 0x14 >> +#define DA8XX_USB_TEARDOWN_REG 0x1c > > Why these _REG suffixes suddenly? I took these defines in da8xx.c. Actually, I think I should make a third patch to remove them from da8xx.c because they are not even used. > > [...] > > Other than that looks sane. Need my ACK? I don't know if that is needed but I guess it would be appreciated. Best Regards, Alexandre > > WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c index ba255280a624..dbff0e0a4ff5 100644 --- a/drivers/usb/musb/musb_cppi41.c +++ b/drivers/usb/musb/musb_cppi41.c @@ -26,6 +26,9 @@ #define MUSB_DMA_NUM_CHANNELS 15 +#define DA8XX_USB_AUTOREQ_REG 0x14 +#define DA8XX_USB_TEARDOWN_REG 0x1c + struct cppi41_dma_controller { struct dma_controller controller; struct cppi41_dma_channel rx_channel[MUSB_DMA_NUM_CHANNELS]; @@ -35,6 +38,9 @@ struct cppi41_dma_controller { u32 rx_mode; u32 tx_mode; u32 auto_req; + + u32 tdown_reg; + u32 autoreq_reg; }; static void save_rx_toggle(struct cppi41_dma_channel *cppi41_channel) @@ -364,8 +370,8 @@ static void cppi41_set_autoreq_mode(struct cppi41_dma_channel *cppi41_channel, if (new_mode == old_mode) return; controller->auto_req = new_mode; - musb_writel(controller->controller.musb->ctrl_base, USB_CTRL_AUTOREQ, - new_mode); + musb_writel(controller->controller.musb->ctrl_base, + controller->autoreq_reg, new_mode); } static bool cppi41_configure_channel(struct dma_channel *channel, @@ -581,12 +587,13 @@ static int cppi41_dma_channel_abort(struct dma_channel *channel) do { if (is_tx) - musb_writel(musb->ctrl_base, USB_TDOWN, tdbit); + musb_writel(musb->ctrl_base, controller->tdown_reg, + tdbit); ret = dmaengine_terminate_all(cppi41_channel->dc); } while (ret == -EAGAIN); if (is_tx) { - musb_writel(musb->ctrl_base, USB_TDOWN, tdbit); + musb_writel(musb->ctrl_base, controller->tdown_reg, tdbit); csr = musb_readw(epio, MUSB_TXCSR); if (csr & MUSB_TXCSR_TXPKTRDY) { @@ -727,6 +734,14 @@ cppi41_dma_controller_create(struct musb *musb, void __iomem *base) controller->controller.is_compatible = cppi41_is_compatible; controller->controller.musb = musb; + if (musb->io.quirks & MUSB_DA8XX) { + controller->tdown_reg = DA8XX_USB_TEARDOWN_REG; + controller->autoreq_reg = DA8XX_USB_AUTOREQ_REG; + } else { + controller->tdown_reg = USB_TDOWN; + controller->autoreq_reg = USB_CTRL_AUTOREQ; + } + ret = cppi41_dma_controller_start(controller); if (ret) goto plat_get_fail;
The DA8xx and DSPS platforms don't use the same address for few registers. On Da8xx, this is causing some issues (e.g. teardown that doesn't work). Configure the address of the register during the init and use them instead of constants. Reported-by: nsekhar@ti.com Signed-off-by: Alexandre Bailon <abailon@baylibre.com> --- drivers/usb/musb/musb_cppi41.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)