[PATCH v3 1/7] usb: musb: dma: Correct parameter passed to IRQ handler

[PATCH v3 1/7] usb: musb: dma: Correct parameter passed to IRQ handler

[Paul Cercueil]

The IRQ handler was passed a pointer to a struct dma_controller, but the
argument was then casted to a pointer to a struct musb_dma_controller.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v2: Rebase on 5.4-rc4
    v3: Rebase on 5.5-rc1

 drivers/usb/musb/musbhsdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 5fc6825745f2..2d3751d885b4 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -425,7 +425,7 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
 	controller->controller.channel_abort = dma_channel_abort;
 
 	if (request_irq(irq, dma_controller_irq, 0,
-			dev_name(musb->controller), &controller->controller)) {
+			dev_name(musb->controller), controller)) {
 		dev_err(dev, "request_irq %d failed!\n", irq);
 		musb_dma_controller_destroy(&controller->controller);
 
-- 
2.24.0

[PATCH v3 2/7] usb: musb: dma: Add support for shared IRQ

[Paul Cercueil]

The implementation of the Inventra IP in some of the Ingenic JZ47xx SoCs
does not use a separate IRQ line for DMA transfers.

Allow these SoCs to be supported by adding a flag 'dma_share_usb_irq'
in the struct musb. If set, no extra IRQ line is required, and the musb
glue will need to call the API function musbhs_dma_controller_irq()
within its interrupt handler.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v2: Rebase on 5.4-rc4
    v3: Rebase on 5.5-rc1

 drivers/usb/musb/musb_core.h |  2 ++
 drivers/usb/musb/musb_dma.h  |  1 +
 drivers/usb/musb/musbhsdma.c | 27 ++++++++++++++++++++++-----
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 04203b7126d5..b7c31c717800 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -392,6 +392,8 @@ struct musb {
 
 	unsigned		flush_irq_work:1;
 
+	unsigned		dma_share_usb_irq:1;
+
 	u8			address;
 	u8			test_mode_nr;
 	u16			ackpend;		/* ep0 */
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
index 8f60271c0a9d..b3f65016a972 100644
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -191,6 +191,7 @@ extern void (*musb_dma_controller_destroy)(struct dma_controller *);
 extern struct dma_controller *
 musbhs_dma_controller_create(struct musb *musb, void __iomem *base);
 extern void musbhs_dma_controller_destroy(struct dma_controller *c);
+extern irqreturn_t musbhs_dma_controller_irq(struct dma_controller *c);
 
 extern struct dma_controller *
 tusb_dma_controller_create(struct musb *musb, void __iomem *base);
diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 2d3751d885b4..898856e9974e 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -268,7 +268,7 @@ static int dma_channel_abort(struct dma_channel *channel)
 	return 0;
 }
 
-static irqreturn_t dma_controller_irq(int irq, void *private_data)
+static irqreturn_t dma_controller_irq_cb(int irq, void *private_data)
 {
 	struct musb_dma_controller *controller = private_data;
 	struct musb *musb = controller->private_data;
@@ -292,6 +292,9 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data)
 	int_hsdma = musb_readb(mbase, MUSB_HSDMA_INTR);
 
 	if (!int_hsdma) {
+		if (musb->dma_share_usb_irq)
+			goto done;
+
 		musb_dbg(musb, "spurious DMA irq");
 
 		for (bchannel = 0; bchannel < MUSB_HSDMA_CHANNELS; bchannel++) {
@@ -384,6 +387,15 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data)
 	return retval;
 }
 
+irqreturn_t musbhs_dma_controller_irq(struct dma_controller *c)
+{
+	struct musb_dma_controller *controller = container_of(c,
+			struct musb_dma_controller, controller);
+
+	return dma_controller_irq_cb(controller->irq, controller);
+}
+EXPORT_SYMBOL_GPL(musbhs_dma_controller_irq);
+
 void musbhs_dma_controller_destroy(struct dma_controller *c)
 {
 	struct musb_dma_controller *controller = container_of(c,
@@ -404,9 +416,14 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
 	struct musb_dma_controller *controller;
 	struct device *dev = musb->controller;
 	struct platform_device *pdev = to_platform_device(dev);
-	int irq = platform_get_irq_byname(pdev, "dma");
+	int irq;
+
+	if (musb->dma_share_usb_irq)
+		irq = 0;
+	else
+		irq = platform_get_irq_byname(pdev, "dma");
 
-	if (irq <= 0) {
+	if (irq < 0) {
 		dev_err(dev, "No DMA interrupt line!\n");
 		return NULL;
 	}
@@ -424,8 +441,8 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
 	controller->controller.channel_program = dma_channel_program;
 	controller->controller.channel_abort = dma_channel_abort;
 
-	if (request_irq(irq, dma_controller_irq, 0,
-			dev_name(musb->controller), controller)) {
+	if (irq > 0 && request_irq(irq, dma_controller_irq_cb, 0,
+				   dev_name(musb->controller), controller)) {
 		dev_err(dev, "request_irq %d failed!\n", irq);
 		musb_dma_controller_destroy(&controller->controller);
 
-- 
2.24.0

Re: [PATCH v3 2/7] usb: musb: dma: Add support for shared IRQ

[Bin Liu]

On Tue, Dec 10, 2019 at 06:11:05PM +0100, Paul Cercueil wrote:
> The implementation of the Inventra IP in some of the Ingenic JZ47xx SoCs
> does not use a separate IRQ line for DMA transfers.
> 
> Allow these SoCs to be supported by adding a flag 'dma_share_usb_irq'
> in the struct musb. If set, no extra IRQ line is required, and the musb
> glue will need to call the API function musbhs_dma_controller_irq()
> within its interrupt handler.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>
> ---
 
Queued for usb-next. Thanks.

-Bin.

Re: [PATCH v3 2/7] usb: musb: dma: Add support for shared IRQ

[Bin Liu]

Hi Paul,

On Wed, Dec 11, 2019 at 12:52:24PM -0600, Bin Liu wrote:
> On Tue, Dec 10, 2019 at 06:11:05PM +0100, Paul Cercueil wrote:
> > The implementation of the Inventra IP in some of the Ingenic JZ47xx SoCs
> > does not use a separate IRQ line for DMA transfers.
> > 
> > Allow these SoCs to be supported by adding a flag 'dma_share_usb_irq'
> > in the struct musb. If set, no extra IRQ line is required, and the musb
> > glue will need to call the API function musbhs_dma_controller_irq()
> > within its interrupt handler.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > Tested-by: Artur Rojek <contact@artur-rojek.eu>
> > ---
>  
> Queued for usb-next. Thanks.

I removed this and the next patch [3/7] in this series from my queue.
Sorry. Ming Guo has posted a series "Add MediaTek MUSB Controller
Driver" which has done the similar implementation [1] but without adding
the flag in struct musb. Can you please check if you can use Ming's
implementation instead? The patch of his musb glue driver which uses the
implementation is [2], just for your reference.

[1] https://marc.info/?l=linux-usb&m=157602930627195&w=2
[2] https://marc.info/?l=linux-usb&m=157602932427210&w=2

Thanks,
-Bin.

Re: [PATCH v3 2/7] usb: musb: dma: Add support for shared IRQ

[Paul Cercueil]

Hi Bin,


Le mar., déc. 17, 2019 at 08:42, Bin Liu <b-liu@ti.com> a écrit :
> Hi Paul,
> 
> On Wed, Dec 11, 2019 at 12:52:24PM -0600, Bin Liu wrote:
>>  On Tue, Dec 10, 2019 at 06:11:05PM +0100, Paul Cercueil wrote:
>>  > The implementation of the Inventra IP in some of the Ingenic 
>> JZ47xx SoCs
>>  > does not use a separate IRQ line for DMA transfers.
>>  >
>>  > Allow these SoCs to be supported by adding a flag 
>> 'dma_share_usb_irq'
>>  > in the struct musb. If set, no extra IRQ line is required, and 
>> the musb
>>  > glue will need to call the API function 
>> musbhs_dma_controller_irq()
>>  > within its interrupt handler.
>>  >
>>  > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  > Tested-by: Artur Rojek <contact@artur-rojek.eu>
>>  > ---
>> 
>>  Queued for usb-next. Thanks.
> 
> I removed this and the next patch [3/7] in this series from my queue.
> Sorry. Ming Guo has posted a series "Add MediaTek MUSB Controller
> Driver" which has done the similar implementation [1] but without 
> adding
> the flag in struct musb. Can you please check if you can use Ming's
> implementation instead? The patch of his musb glue driver which uses 
> the
> implementation is [2], just for your reference.

Sure. Were these patches merged? What tree do they apply to? They don't 
apply cleanly on -rc2.

-Paul


> 
> [1] https://marc.info/?l=linux-usb&m=157602930627195&w=2
> [2] https://marc.info/?l=linux-usb&m=157602932427210&w=2
> 
> Thanks,
> -Bin.

Re: [PATCH v3 2/7] usb: musb: dma: Add support for shared IRQ

[Bin Liu]

On Tue, Dec 17, 2019 at 10:04:38PM +0100, Paul Cercueil wrote:
> Hi Bin,
> 
> 
> Le mar., déc. 17, 2019 at 08:42, Bin Liu <b-liu@ti.com> a écrit :
> > Hi Paul,
> > 
> > On Wed, Dec 11, 2019 at 12:52:24PM -0600, Bin Liu wrote:
> > >  On Tue, Dec 10, 2019 at 06:11:05PM +0100, Paul Cercueil wrote:
> > >  > The implementation of the Inventra IP in some of the Ingenic
> > > JZ47xx SoCs
> > >  > does not use a separate IRQ line for DMA transfers.
> > >  >
> > >  > Allow these SoCs to be supported by adding a flag
> > > 'dma_share_usb_irq'
> > >  > in the struct musb. If set, no extra IRQ line is required, and
> > > the musb
> > >  > glue will need to call the API function
> > > musbhs_dma_controller_irq()
> > >  > within its interrupt handler.
> > >  >
> > >  > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > >  > Tested-by: Artur Rojek <contact@artur-rojek.eu>
> > >  > ---
> > > 
> > >  Queued for usb-next. Thanks.
> > 
> > I removed this and the next patch [3/7] in this series from my queue.
> > Sorry. Ming Guo has posted a series "Add MediaTek MUSB Controller
> > Driver" which has done the similar implementation [1] but without adding
> > the flag in struct musb. Can you please check if you can use Ming's
> > implementation instead? The patch of his musb glue driver which uses the
> > implementation is [2], just for your reference.
> 
> Sure. Were these patches merged? What tree do they apply to? They don't
> apply cleanly on -rc2.

I am currently doing the last round review and test on those patches,
applying them to Greg's -next branch. Yes, they don't apply cleanly on
-rc2, but manual apply doesn't have any issue.

-Bin.

[PATCH v3 3/7] usb: musb: jz4740: Add support for DMA

[Paul Cercueil]

Add support for using the DMA channels built into the Inventra IP.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v2: Rebase on 5.4-rc4
    v3: Rebase on 5.5-rc1

 drivers/usb/musb/Kconfig  |  2 +-
 drivers/usb/musb/jz4740.c | 21 +++++++++++++++------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 52f8e2b57ad5..210e4844c92a 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -142,7 +142,7 @@ config USB_UX500_DMA
 
 config USB_INVENTRA_DMA
 	bool 'Inventra'
-	depends on USB_MUSB_OMAP2PLUS
+	depends on USB_MUSB_OMAP2PLUS || USB_MUSB_JZ4740
 	help
 	  Enable DMA transfers using Mentor's engine.
 
diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
index 5261f8dfedec..bbecfdee6ea1 100644
--- a/drivers/usb/musb/jz4740.c
+++ b/drivers/usb/musb/jz4740.c
@@ -25,11 +25,14 @@ struct jz4740_glue {
 static irqreturn_t jz4740_musb_interrupt(int irq, void *__hci)
 {
 	unsigned long   flags;
-	irqreturn_t     retval = IRQ_NONE;
+	irqreturn_t     retval = IRQ_NONE, retval_dma = IRQ_NONE;
 	struct musb     *musb = __hci;
 
 	spin_lock_irqsave(&musb->lock, flags);
 
+	if (IS_ENABLED(CONFIG_USB_INVENTRA_DMA) && musb->dma_controller)
+		retval_dma = musbhs_dma_controller_irq(musb->dma_controller);
+
 	musb->int_usb = musb_readb(musb->mregs, MUSB_INTRUSB);
 	musb->int_tx = musb_readw(musb->mregs, MUSB_INTRTX);
 	musb->int_rx = musb_readw(musb->mregs, MUSB_INTRRX);
@@ -47,7 +50,10 @@ static irqreturn_t jz4740_musb_interrupt(int irq, void *__hci)
 
 	spin_unlock_irqrestore(&musb->lock, flags);
 
-	return retval;
+	if (retval == IRQ_HANDLED || retval_dma == IRQ_HANDLED)
+		return IRQ_HANDLED;
+
+	return IRQ_NONE;
 }
 
 static struct musb_fifo_cfg jz4740_musb_fifo_cfg[] = {
@@ -91,18 +97,19 @@ static int jz4740_musb_init(struct musb *musb)
 	musb->dyn_fifo = true;
 
 	musb->isr = jz4740_musb_interrupt;
+	musb->dma_share_usb_irq = true;
 
 	return 0;
 }
 
-/*
- * DMA has not been confirmed to work with CONFIG_USB_INVENTRA_DMA,
- * so let's not set up the dma function pointers yet.
- */
 static const struct musb_platform_ops jz4740_musb_ops = {
 	.quirks		= MUSB_DMA_INVENTRA | MUSB_INDEXED_EP,
 	.fifo_mode	= 2,
 	.init		= jz4740_musb_init,
+#ifdef CONFIG_USB_INVENTRA_DMA
+	.dma_init	= musbhs_dma_controller_create,
+	.dma_exit	= musbhs_dma_controller_destroy,
+#endif
 };
 
 static int jz4740_probe(struct platform_device *pdev)
@@ -137,6 +144,8 @@ static int jz4740_probe(struct platform_device *pdev)
 	}
 
 	musb->dev.parent		= &pdev->dev;
+	musb->dev.dma_mask		= &musb->dev.coherent_dma_mask;
+	musb->dev.coherent_dma_mask	= DMA_BIT_MASK(32);
 
 	glue->dev			= &pdev->dev;
 	glue->musb			= musb;
-- 
2.24.0

Re: [PATCH v3 3/7] usb: musb: jz4740: Add support for DMA

[Bin Liu]

On Tue, Dec 10, 2019 at 06:11:06PM +0100, Paul Cercueil wrote:
> Add support for using the DMA channels built into the Inventra IP.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>
> ---

Queued for usb-next. Thanks.

-Bin.

[PATCH v3 4/7] usb: musb: jz4740: Drop dependency on NOP_USB_XCEIV

[Paul Cercueil]

The driver does not depend directly on the NOP transceiver. It can
compile and work just fine without it.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: New patch
    v3: Rebase on 5.5-rc1

 drivers/usb/musb/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 210e4844c92a..56ccba3c8444 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -111,7 +111,6 @@ config USB_MUSB_UX500
 
 config USB_MUSB_JZ4740
 	tristate "JZ4740"
-	depends on NOP_USB_XCEIV
 	depends on MIPS || COMPILE_TEST
 	depends on USB_MUSB_GADGET
 	depends on USB=n || USB_OTG_BLACKLIST_HUB
-- 
2.24.0

Re: [PATCH v3 4/7] usb: musb: jz4740: Drop dependency on NOP_USB_XCEIV

[Bin Liu]

On Tue, Dec 10, 2019 at 06:11:07PM +0100, Paul Cercueil wrote:
> The driver does not depend directly on the NOP transceiver. It can
> compile and work just fine without it.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Queued for usb-next. Thanks.

-Bin.

[PATCH v3 5/7] usb: musb: jz4740: Silence error if code is -EPROBE_DEFER

[Paul Cercueil]

Avoid printing any error message if the error code is -EPROBE_DEFER.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: New patch
    v3: Rebase on 5.5-rc1

 drivers/usb/musb/jz4740.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
index bbecfdee6ea1..5e885fa26829 100644
--- a/drivers/usb/musb/jz4740.c
+++ b/drivers/usb/musb/jz4740.c
@@ -81,14 +81,17 @@ static struct musb_hdrc_platform_data jz4740_musb_platform_data = {
 static int jz4740_musb_init(struct musb *musb)
 {
 	struct device *dev = musb->controller->parent;
+	int err;
 
 	if (dev->of_node)
 		musb->xceiv = devm_usb_get_phy_by_phandle(dev, "phys", 0);
 	else
 		musb->xceiv = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
 	if (IS_ERR(musb->xceiv)) {
-		dev_err(dev, "No transceiver configured\n");
-		return PTR_ERR(musb->xceiv);
+		err = PTR_ERR(musb->xceiv);
+		if (err != -EPROBE_DEFER)
+			dev_err(dev, "No transceiver configured: %d", err);
+		return err;
 	}
 
 	/* Silicon does not implement ConfigData register.
-- 
2.24.0

Re: [PATCH v3 5/7] usb: musb: jz4740: Silence error if code is -EPROBE_DEFER

[Bin Liu]

On Tue, Dec 10, 2019 at 06:11:08PM +0100, Paul Cercueil wrote:
> Avoid printing any error message if the error code is -EPROBE_DEFER.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Queued for next v5.5 -rc. Thanks.

-Bin.

[PATCH v3 6/7] usb: musb: jz4740: Code cleanup

[Paul Cercueil]

Just some code maintenance; no functional change.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2: New patch
    v3: Rebase on 5.5-rc1

 drivers/usb/musb/jz4740.c | 75 +++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
index 5e885fa26829..f948eca654f3 100644
--- a/drivers/usb/musb/jz4740.c
+++ b/drivers/usb/musb/jz4740.c
@@ -17,16 +17,15 @@
 #include "musb_core.h"
 
 struct jz4740_glue {
-	struct device           *dev;
-	struct platform_device  *musb;
+	struct platform_device	*pdev;
 	struct clk		*clk;
 };
 
 static irqreturn_t jz4740_musb_interrupt(int irq, void *__hci)
 {
-	unsigned long   flags;
-	irqreturn_t     retval = IRQ_NONE, retval_dma = IRQ_NONE;
-	struct musb     *musb = __hci;
+	unsigned long	flags;
+	irqreturn_t	retval = IRQ_NONE, retval_dma = IRQ_NONE;
+	struct musb	*musb = __hci;
 
 	spin_lock_irqsave(&musb->lock, flags);
 
@@ -43,7 +42,7 @@ static irqreturn_t jz4740_musb_interrupt(int irq, void *__hci)
 	 * never see them set
 	 */
 	musb->int_usb &= MUSB_INTR_SUSPEND | MUSB_INTR_RESUME |
-	    MUSB_INTR_RESET | MUSB_INTR_SOF;
+			 MUSB_INTR_RESET | MUSB_INTR_SOF;
 
 	if (musb->int_usb || musb->int_tx || musb->int_rx)
 		retval = musb_interrupt(musb);
@@ -57,25 +56,20 @@ static irqreturn_t jz4740_musb_interrupt(int irq, void *__hci)
 }
 
 static struct musb_fifo_cfg jz4740_musb_fifo_cfg[] = {
-{ .hw_ep_num = 1, .style = FIFO_TX, .maxpacket = 512, },
-{ .hw_ep_num = 1, .style = FIFO_RX, .maxpacket = 512, },
-{ .hw_ep_num = 2, .style = FIFO_TX, .maxpacket = 64, },
+	{ .hw_ep_num = 1, .style = FIFO_TX, .maxpacket = 512, },
+	{ .hw_ep_num = 1, .style = FIFO_RX, .maxpacket = 512, },
+	{ .hw_ep_num = 2, .style = FIFO_TX, .maxpacket = 64, },
 };
 
 static const struct musb_hdrc_config jz4740_musb_config = {
 	/* Silicon does not implement USB OTG. */
-	.multipoint = 0,
+	.multipoint	= 0,
 	/* Max EPs scanned, driver will decide which EP can be used. */
-	.num_eps    = 4,
+	.num_eps	= 4,
 	/* RAMbits needed to configure EPs from table */
-	.ram_bits   = 9,
-	.fifo_cfg = jz4740_musb_fifo_cfg,
-	.fifo_cfg_size = ARRAY_SIZE(jz4740_musb_fifo_cfg),
-};
-
-static struct musb_hdrc_platform_data jz4740_musb_platform_data = {
-	.mode   = MUSB_PERIPHERAL,
-	.config = &jz4740_musb_config,
+	.ram_bits	= 9,
+	.fifo_cfg	= jz4740_musb_fifo_cfg,
+	.fifo_cfg_size	= ARRAY_SIZE(jz4740_musb_fifo_cfg),
 };
 
 static int jz4740_musb_init(struct musb *musb)
@@ -94,7 +88,8 @@ static int jz4740_musb_init(struct musb *musb)
 		return err;
 	}
 
-	/* Silicon does not implement ConfigData register.
+	/*
+	 * Silicon does not implement ConfigData register.
 	 * Set dyn_fifo to avoid reading EP config from hardware.
 	 */
 	musb->dyn_fifo = true;
@@ -115,65 +110,69 @@ static const struct musb_platform_ops jz4740_musb_ops = {
 #endif
 };
 
+static const struct musb_hdrc_platform_data jz4740_musb_pdata = {
+	.mode		= MUSB_PERIPHERAL,
+	.config		= &jz4740_musb_config,
+	.platform_ops	= &jz4740_musb_ops,
+};
+
 static int jz4740_probe(struct platform_device *pdev)
 {
-	struct musb_hdrc_platform_data	*pdata = &jz4740_musb_platform_data;
+	struct device			*dev = &pdev->dev;
+	const struct musb_hdrc_platform_data *pdata = &jz4740_musb_pdata;
 	struct platform_device		*musb;
 	struct jz4740_glue		*glue;
-	struct clk                      *clk;
+	struct clk			*clk;
 	int				ret;
 
-	glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
+	glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL);
 	if (!glue)
 		return -ENOMEM;
 
 	musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO);
 	if (!musb) {
-		dev_err(&pdev->dev, "failed to allocate musb device\n");
+		dev_err(dev, "failed to allocate musb device");
 		return -ENOMEM;
 	}
 
-	clk = devm_clk_get(&pdev->dev, "udc");
+	clk = devm_clk_get(dev, "udc");
 	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "failed to get clock\n");
+		dev_err(dev, "failed to get clock");
 		ret = PTR_ERR(clk);
 		goto err_platform_device_put;
 	}
 
 	ret = clk_prepare_enable(clk);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to enable clock\n");
+		dev_err(dev, "failed to enable clock");
 		goto err_platform_device_put;
 	}
 
-	musb->dev.parent		= &pdev->dev;
+	musb->dev.parent		= dev;
 	musb->dev.dma_mask		= &musb->dev.coherent_dma_mask;
 	musb->dev.coherent_dma_mask	= DMA_BIT_MASK(32);
 
-	glue->dev			= &pdev->dev;
-	glue->musb			= musb;
+	glue->pdev			= musb;
 	glue->clk			= clk;
 
-	pdata->platform_ops		= &jz4740_musb_ops;
-
 	platform_set_drvdata(pdev, glue);
 
 	ret = platform_device_add_resources(musb, pdev->resource,
 					    pdev->num_resources);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to add resources\n");
+		dev_err(dev, "failed to add resources");
 		goto err_clk_disable;
 	}
 
 	ret = platform_device_add_data(musb, pdata, sizeof(*pdata));
 	if (ret) {
-		dev_err(&pdev->dev, "failed to add platform_data\n");
+		dev_err(dev, "failed to add platform_data");
 		goto err_clk_disable;
 	}
 
 	ret = platform_device_add(musb);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to register musb device\n");
+		dev_err(dev, "failed to register musb device");
 		goto err_clk_disable;
 	}
 
@@ -188,9 +187,9 @@ static int jz4740_probe(struct platform_device *pdev)
 
 static int jz4740_remove(struct platform_device *pdev)
 {
-	struct jz4740_glue	*glue = platform_get_drvdata(pdev);
+	struct jz4740_glue *glue = platform_get_drvdata(pdev);
 
-	platform_device_unregister(glue->musb);
+	platform_device_unregister(glue->pdev);
 	clk_disable_unprepare(glue->clk);
 
 	return 0;
@@ -199,7 +198,7 @@ static int jz4740_remove(struct platform_device *pdev)
 #ifdef CONFIG_OF
 static const struct of_device_id jz4740_musb_of_match[] = {
 	{ .compatible = "ingenic,jz4740-musb" },
-	{},
+	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, jz4740_musb_of_match);
 #endif
-- 
2.24.0

Re: [PATCH v3 6/7] usb: musb: jz4740: Code cleanup

[Bin Liu]

Hi,

On Tue, Dec 10, 2019 at 06:11:09PM +0100, Paul Cercueil wrote:
> Just some code maintenance; no functional change.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

It seems the patch has a few different types of cleanup, coding style,
indentation, data structure change, variable name change...

Please break the changes into multiple patches, each serves one type of
cleanup.

-Bin.

Re: [PATCH v3 6/7] usb: musb: jz4740: Code cleanup

[Greg Kroah-Hartman]

On Tue, Dec 10, 2019 at 06:11:09PM +0100, Paul Cercueil wrote:
> Just some code maintenance; no functional change.
> 

Be very specific as to what you are doing here, and why.  "code
maintenance" is very vague and not descriptive at all.

thanks,

greg k-h

[PATCH v3 7/7] usb: musb: jz4740: Disable pullup at init

[Paul Cercueil]

The pullup may be already enabled before the driver is initialized.
It has to be disabled at init time, as we cannot guarantee that a gadget
driver will be bound to the UDC.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v3: New patch

 drivers/usb/musb/jz4740.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
index f948eca654f3..1af9e4053312 100644
--- a/drivers/usb/musb/jz4740.c
+++ b/drivers/usb/musb/jz4740.c
@@ -75,6 +75,7 @@ static const struct musb_hdrc_config jz4740_musb_config = {
 static int jz4740_musb_init(struct musb *musb)
 {
 	struct device *dev = musb->controller->parent;
+	u8 power;
 	int err;
 
 	if (dev->of_node)
@@ -97,6 +98,14 @@ static int jz4740_musb_init(struct musb *musb)
 	musb->isr = jz4740_musb_interrupt;
 	musb->dma_share_usb_irq = true;
 
+	/*
+	 * If the SoC booted from USB the pullup might still be set.
+	 * Disable it until a gadget is bound.
+	 */
+	power = musb_readb(musb->mregs, MUSB_POWER);
+	power &= ~MUSB_POWER_SOFTCONN;
+	musb_writeb(musb->mregs, MUSB_POWER, power);
+
 	return 0;
 }
 
-- 
2.24.0

Re: [PATCH v3 7/7] usb: musb: jz4740: Disable pullup at init

[Bin Liu]

Hi,

On Tue, Dec 10, 2019 at 06:11:10PM +0100, Paul Cercueil wrote:
> The pullup may be already enabled before the driver is initialized.
> It has to be disabled at init time, as we cannot guarantee that a gadget
> driver will be bound to the UDC.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Notes:
>     v3: New patch
> 
>  drivers/usb/musb/jz4740.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
> index f948eca654f3..1af9e4053312 100644
> --- a/drivers/usb/musb/jz4740.c
> +++ b/drivers/usb/musb/jz4740.c
> @@ -75,6 +75,7 @@ static const struct musb_hdrc_config jz4740_musb_config = {
>  static int jz4740_musb_init(struct musb *musb)
>  {
>  	struct device *dev = musb->controller->parent;
> +	u8 power;
>  	int err;
>  
>  	if (dev->of_node)
> @@ -97,6 +98,14 @@ static int jz4740_musb_init(struct musb *musb)
>  	musb->isr = jz4740_musb_interrupt;
>  	musb->dma_share_usb_irq = true;
>  
> +	/*
> +	 * If the SoC booted from USB the pullup might still be set.
> +	 * Disable it until a gadget is bound.
> +	 */
> +	power = musb_readb(musb->mregs, MUSB_POWER);
> +	power &= ~MUSB_POWER_SOFTCONN;
> +	musb_writeb(musb->mregs, MUSB_POWER, power);
> +
>  	return 0;
>  }

It is preferred the glue drivers don't touch the controller core
registers if possible. Please try the following patch instead.

-Bin.

------------- >8 ------------
t a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 683b719c5026..2f9105e8ea38 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2317,6 +2317,9 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
        musb_disable_interrupts(musb);
        musb_writeb(musb->mregs, MUSB_DEVCTL, 0);

+       /* MUSB_POWER_SOFTCONN might be already set, jz4740 does this. */
+       musb_writeb(musb->mregs, MUSB_POWER, 0);
+
        /* Init IRQ workqueue before request_irq */
        INIT_DELAYED_WORK(&musb->irq_work, musb_irq_work);
        INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset);

Re: [PATCH v3 1/7] usb: musb: dma: Correct parameter passed to IRQ handler

[Bin Liu]

On Tue, Dec 10, 2019 at 06:11:04PM +0100, Paul Cercueil wrote:
> The IRQ handler was passed a pointer to a struct dma_controller, but the
> argument was then casted to a pointer to a struct musb_dma_controller.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>
> ---

I added the Fixes tag, and queued for next v5.5 -rc. Thanks.

-Bin.