diff mbox

usb: renesas_usbhs: Revise the binding document about the dma-names

Message ID 1428489744-4872-1-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Accepted
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Yoshihiro Shimoda April 8, 2015, 10:42 a.m. UTC
Since the DT should describe the hardware (not the driver limitation),
This patch revises the binding document about the dma-names to change
simple numbering as "ch%d" instead of "tx<n>" and "rx<n>".

Also this patch fixes the actual code of renesas_usbhs driver to handle
the new dma-names.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 This patch is based on Felipe's usb.bit / testing/next branch.
 (commit id = bbc78c07a51f6fd29c227b1220a9016e585358ba)

 Geert is pointed out about this issue:
 https://www.mail-archive.com/devicetree@vger.kernel.org/msg68401.html

 .../devicetree/bindings/usb/renesas_usbhs.txt      |  6 ++----
 drivers/usb/renesas_usbhs/fifo.c                   | 24 ++++++++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)

Comments

Mark Rutland April 8, 2015, 1:14 p.m. UTC | #1
On Wed, Apr 08, 2015 at 11:42:24AM +0100, Yoshihiro Shimoda wrote:
> Since the DT should describe the hardware (not the driver limitation),
> This patch revises the binding document about the dma-names to change
> simple numbering as "ch%d" instead of "tx<n>" and "rx<n>".

The naming given in this patch looks more sensible to me.

> Also this patch fixes the actual code of renesas_usbhs driver to handle
> the new dma-names.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  This patch is based on Felipe's usb.bit / testing/next branch.
>  (commit id = bbc78c07a51f6fd29c227b1220a9016e585358ba)

I take it the existing driver and binding haven't hit mainline, and
therefore there are no users yet?

Mark.

> 
>  Geert is pointed out about this issue:
>  https://www.mail-archive.com/devicetree@vger.kernel.org/msg68401.html
> 
>  .../devicetree/bindings/usb/renesas_usbhs.txt      |  6 ++----
>  drivers/usb/renesas_usbhs/fifo.c                   | 24 ++++++++++++++--------
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> index dc2a18f..ddbe304 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> @@ -15,10 +15,8 @@ Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
>    - dmas: Must contain a list of references to DMA specifiers.
> -  - dma-names : Must contain a list of DMA names:
> -   - tx0 ... tx<n>
> -   - rx0 ... rx<n>
> -    - This <n> means DnFIFO in USBHS module.
> +  - dma-names : named "ch%d", where %d is the channel number ranging from zero
> +                to the number of channels (DnFIFOs) minus one.
>  
>  Example:
>  	usbhs: usb@e6590000 {
> diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
> index 8597cf9..bc23b4a 100644
> --- a/drivers/usb/renesas_usbhs/fifo.c
> +++ b/drivers/usb/renesas_usbhs/fifo.c
> @@ -1227,15 +1227,21 @@ static void usbhsf_dma_init_dt(struct device *dev, struct usbhs_fifo *fifo,
>  {
>  	char name[16];
>  
> -	snprintf(name, sizeof(name), "tx%d", channel);
> -	fifo->tx_chan = dma_request_slave_channel_reason(dev, name);
> -	if (IS_ERR(fifo->tx_chan))
> -		fifo->tx_chan = NULL;
> -
> -	snprintf(name, sizeof(name), "rx%d", channel);
> -	fifo->rx_chan = dma_request_slave_channel_reason(dev, name);
> -	if (IS_ERR(fifo->rx_chan))
> -		fifo->rx_chan = NULL;
> +	/*
> +	 * To avoid complex handing for DnFIFOs, the driver uses each
> +	 * DnFIFO as TX or RX direction (not bi-direction).
> +	 * So, the driver uses odd channels for TX, even channels for RX.
> +	 */
> +	snprintf(name, sizeof(name), "ch%d", channel);
> +	if (channel & 1) {
> +		fifo->tx_chan = dma_request_slave_channel_reason(dev, name);
> +		if (IS_ERR(fifo->tx_chan))
> +			fifo->tx_chan = NULL;
> +	} else {
> +		fifo->rx_chan = dma_request_slave_channel_reason(dev, name);
> +		if (IS_ERR(fifo->rx_chan))
> +			fifo->rx_chan = NULL;
> +	}
>  }
>  
>  static void usbhsf_dma_init(struct usbhs_priv *priv, struct usbhs_fifo *fifo,
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yoshihiro Shimoda April 9, 2015, 12:17 a.m. UTC | #2
Hi Mark,

> 
> On Wed, Apr 08, 2015 at 11:42:24AM +0100, Yoshihiro Shimoda wrote:
> > Since the DT should describe the hardware (not the driver limitation),
> > This patch revises the binding document about the dma-names to change
> > simple numbering as "ch%d" instead of "tx<n>" and "rx<n>".
> 
> The naming given in this patch looks more sensible to me.

Thank you for your comment!

> > Also this patch fixes the actual code of renesas_usbhs driver to handle
> > the new dma-names.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  This patch is based on Felipe's usb.bit / testing/next branch.
> >  (commit id = bbc78c07a51f6fd29c227b1220a9016e585358ba)
> 
> I take it the existing driver and binding haven't hit mainline, and
> therefore there are no users yet?

That's correct. At the moment, nobody uses the dma-names in the node of
renesas_usbhs yet.

Best regards,
Yoshihiro Shimoda

> Mark.
> 
> >
> >  Geert is pointed out about this issue:
> >  https://www.mail-archive.com/devicetree@vger.kernel.org/msg68401.html
> >
> >  .../devicetree/bindings/usb/renesas_usbhs.txt      |  6 ++----
> >  drivers/usb/renesas_usbhs/fifo.c                   | 24 ++++++++++++++--------
> >  2 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> > index dc2a18f..ddbe304 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> > @@ -15,10 +15,8 @@ Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> >    - dmas: Must contain a list of references to DMA specifiers.
> > -  - dma-names : Must contain a list of DMA names:
> > -   - tx0 ... tx<n>
> > -   - rx0 ... rx<n>
> > -    - This <n> means DnFIFO in USBHS module.
> > +  - dma-names : named "ch%d", where %d is the channel number ranging from zero
> > +                to the number of channels (DnFIFOs) minus one.
> >
> >  Example:
> >  	usbhs: usb@e6590000 {
> > diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
> > index 8597cf9..bc23b4a 100644
> > --- a/drivers/usb/renesas_usbhs/fifo.c
> > +++ b/drivers/usb/renesas_usbhs/fifo.c
> > @@ -1227,15 +1227,21 @@ static void usbhsf_dma_init_dt(struct device *dev, struct usbhs_fifo *fifo,
> >  {
> >  	char name[16];
> >
> > -	snprintf(name, sizeof(name), "tx%d", channel);
> > -	fifo->tx_chan = dma_request_slave_channel_reason(dev, name);
> > -	if (IS_ERR(fifo->tx_chan))
> > -		fifo->tx_chan = NULL;
> > -
> > -	snprintf(name, sizeof(name), "rx%d", channel);
> > -	fifo->rx_chan = dma_request_slave_channel_reason(dev, name);
> > -	if (IS_ERR(fifo->rx_chan))
> > -		fifo->rx_chan = NULL;
> > +	/*
> > +	 * To avoid complex handing for DnFIFOs, the driver uses each
> > +	 * DnFIFO as TX or RX direction (not bi-direction).
> > +	 * So, the driver uses odd channels for TX, even channels for RX.
> > +	 */
> > +	snprintf(name, sizeof(name), "ch%d", channel);
> > +	if (channel & 1) {
> > +		fifo->tx_chan = dma_request_slave_channel_reason(dev, name);
> > +		if (IS_ERR(fifo->tx_chan))
> > +			fifo->tx_chan = NULL;
> > +	} else {
> > +		fifo->rx_chan = dma_request_slave_channel_reason(dev, name);
> > +		if (IS_ERR(fifo->rx_chan))
> > +			fifo->rx_chan = NULL;
> > +	}
> >  }
> >
> >  static void usbhsf_dma_init(struct usbhs_priv *priv, struct usbhs_fifo *fifo,
> > --
> > 1.9.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven April 9, 2015, 7:22 a.m. UTC | #3
On Wed, Apr 8, 2015 at 12:42 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Since the DT should describe the hardware (not the driver limitation),
> This patch revises the binding document about the dma-names to change
> simple numbering as "ch%d" instead of "tx<n>" and "rx<n>".
>
> Also this patch fixes the actual code of renesas_usbhs driver to handle
> the new dma-names.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland April 9, 2015, 9:06 a.m. UTC | #4
On Thu, Apr 09, 2015 at 01:17:44AM +0100, Yoshihiro Shimoda wrote:
> Hi Mark,
> 
> > 
> > On Wed, Apr 08, 2015 at 11:42:24AM +0100, Yoshihiro Shimoda wrote:
> > > Since the DT should describe the hardware (not the driver limitation),
> > > This patch revises the binding document about the dma-names to change
> > > simple numbering as "ch%d" instead of "tx<n>" and "rx<n>".
> > 
> > The naming given in this patch looks more sensible to me.
> 
> Thank you for your comment!
> 
> > > Also this patch fixes the actual code of renesas_usbhs driver to handle
> > > the new dma-names.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > >  This patch is based on Felipe's usb.bit / testing/next branch.
> > >  (commit id = bbc78c07a51f6fd29c227b1220a9016e585358ba)
> > 
> > I take it the existing driver and binding haven't hit mainline, and
> > therefore there are no users yet?
> 
> That's correct. At the moment, nobody uses the dma-names in the node of
> renesas_usbhs yet.

Given that:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yoshihiro Shimoda April 28, 2015, 2:43 a.m. UTC | #5
Hi Felipe,


> Sent: Thursday, April 09, 2015 6:06 PM
> 
> On Thu, Apr 09, 2015 at 01:17:44AM +0100, Yoshihiro Shimoda wrote:
> > Hi Mark,
> >
> > >
> > > On Wed, Apr 08, 2015 at 11:42:24AM +0100, Yoshihiro Shimoda wrote:
> > > > Since the DT should describe the hardware (not the driver limitation),
> > > > This patch revises the binding document about the dma-names to change
> > > > simple numbering as "ch%d" instead of "tx<n>" and "rx<n>".
> > >
> > > The naming given in this patch looks more sensible to me.
> >
> > Thank you for your comment!
> >
> > > > Also this patch fixes the actual code of renesas_usbhs driver to handle
> > > > the new dma-names.
> > > >
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > ---
> > > >  This patch is based on Felipe's usb.bit / testing/next branch.
> > > >  (commit id = bbc78c07a51f6fd29c227b1220a9016e585358ba)
> > >
> > > I take it the existing driver and binding haven't hit mainline, and
> > > therefore there are no users yet?
> >
> > That's correct. At the moment, nobody uses the dma-names in the node of
> > renesas_usbhs yet.
> 
> Given that:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

Would you apply this patch to your repository?
(If possible, it is merged in your fixes branch because the commit ab330cf388 is already merged
 on v4.1-rc1 and this patch is related to the commit.)

Or, should I resubmit this patch with Mark and Geert's Acked-by?
(I confirmed that this patch could be applied on the branches of both fixes and testing/next.)

Best regards,
Yoshihiro Shimoda

P.S.
I am surprised, the Morimoto-san's patch "tidyup usbhs_for_each_dfifo macro" is threaded to
this patch. However, this patch is not related to the Morimoto-san's patch.
http://thread.gmane.org/gmane.linux.ports.sh.devel/45229/focus=124680

> Thanks,
> Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index dc2a18f..ddbe304 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -15,10 +15,8 @@  Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
   - dmas: Must contain a list of references to DMA specifiers.
-  - dma-names : Must contain a list of DMA names:
-   - tx0 ... tx<n>
-   - rx0 ... rx<n>
-    - This <n> means DnFIFO in USBHS module.
+  - dma-names : named "ch%d", where %d is the channel number ranging from zero
+                to the number of channels (DnFIFOs) minus one.
 
 Example:
 	usbhs: usb@e6590000 {
diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 8597cf9..bc23b4a 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -1227,15 +1227,21 @@  static void usbhsf_dma_init_dt(struct device *dev, struct usbhs_fifo *fifo,
 {
 	char name[16];
 
-	snprintf(name, sizeof(name), "tx%d", channel);
-	fifo->tx_chan = dma_request_slave_channel_reason(dev, name);
-	if (IS_ERR(fifo->tx_chan))
-		fifo->tx_chan = NULL;
-
-	snprintf(name, sizeof(name), "rx%d", channel);
-	fifo->rx_chan = dma_request_slave_channel_reason(dev, name);
-	if (IS_ERR(fifo->rx_chan))
-		fifo->rx_chan = NULL;
+	/*
+	 * To avoid complex handing for DnFIFOs, the driver uses each
+	 * DnFIFO as TX or RX direction (not bi-direction).
+	 * So, the driver uses odd channels for TX, even channels for RX.
+	 */
+	snprintf(name, sizeof(name), "ch%d", channel);
+	if (channel & 1) {
+		fifo->tx_chan = dma_request_slave_channel_reason(dev, name);
+		if (IS_ERR(fifo->tx_chan))
+			fifo->tx_chan = NULL;
+	} else {
+		fifo->rx_chan = dma_request_slave_channel_reason(dev, name);
+		if (IS_ERR(fifo->rx_chan))
+			fifo->rx_chan = NULL;
+	}
 }
 
 static void usbhsf_dma_init(struct usbhs_priv *priv, struct usbhs_fifo *fifo,