| Message ID | 1428489744-4872-1-git-send-email-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | Accepted, archived |
| Commit | 5a294e5469891d0701183049c4a9678887fa7091 |
| Headers | show |
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 devicetree" 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/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,
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(-)