[PATCH/RFC 0/4] usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c

[PATCH/RFC 0/4] usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c

[Yoshihiro Shimoda]

This patch set is based on the latest Felipe's usb.git / testing/next branch.
(commit id = 15e4292a2d21e9997fdb2b8c014cc461b3f268f0)

This patch set also needs an additional patch (like the end of this email)
to fix the renesas_usbhs driver for R-Car Gen3.
(I will submit this patch after v4.8-rc1 was released.)

I'm not sure this is a correct way or not. So, I added "RFC" in this patch set.
About the detail of this patch set, please refer to the commit log of
first patch ("usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c").

Yoshihiro Shimoda (4):
  usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c
  usb: gadget: u_ether: add a flag to avoid skb_reserve() calling
  usb: gadget: f_ncm: add support for no_skb_reserve
  usb: renesas_usbhs: set quirk_avoids_skb_reserve if USB-DMAC is used

 drivers/usb/gadget/function/f_ncm.c    |  2 ++
 drivers/usb/gadget/function/u_ether.c  |  5 ++++-
 drivers/usb/gadget/function/u_ether.h  |  1 +
 drivers/usb/renesas_usbhs/mod_gadget.c |  2 ++
 include/linux/usb/gadget.h             | 13 +++++++++++++
 5 files changed, 22 insertions(+), 1 deletion(-)

---
< about an additonal patch >
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Date: Thu, 7 Jul 2016 12:47:20 +0900
Subject: [PATCH] usb: renesas_usbhs: Fix receiving data corrupt on R-Car Gen3
 with dmac

Since R-Car Gen3 SoC has the USB-DMAC, this driver should set
dparam->has_usb_dmac to 1. Otherwise, behavior of this driver and
the usb-dmac driver will be mismatch, then sometimes receiving data will
be corrupt.

Fixes: de18757e272d ("usb: renesas_usbhs: add R-Car Gen3 power control")
Cc: <stable@vger.kernel.org> # v4.5+
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/renesas_usbhs/common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 8fbbc2d..ac67bab 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -514,7 +514,8 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
 	if (gpio > 0)
 		dparam->enable_gpio = gpio;
 
-	if (dparam->type == USBHS_TYPE_RCAR_GEN2)
+	if (dparam->type == USBHS_TYPE_RCAR_GEN2 ||
+	    dparam->type == USBHS_TYPE_RCAR_GEN3)
 		dparam->has_usb_dmac = 1;
 
 	return info;
-- 
1.9.1

[PATCH/RFC 1/4] usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c

[Yoshihiro Shimoda]

Some platforms (e.g. R-Car SoCs) has memory alignment restriction.
If memory alignment is not match, the usb peripheral driver desides
not to DMA controller. Then, the performance is not good.

In the case of u_ether.c, since it calls skb_reserve() in rx_submit(),
it is possible to cause memory alignment mispatch in some platforms
(like R-Car SoCs).

So, this patch adds a new quirk "quirk_avoids_skb_reserve" to avoid
skb_reserve() calling in u_ether.c to improve performance.

A peripheral driver will set this flag and network gadget drivers
(e.g. f_ncm.c) will reference the flag via gadget_avoids_skb_reserve().

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 include/linux/usb/gadget.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdf..c7ff7fe 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -346,6 +346,8 @@ struct usb_gadget_ops {
  *	or B-Peripheral wants to take host role.
  * @quirk_ep_out_aligned_size: epout requires buffer size to be aligned to
  *	MaxPacketSize.
+ * @quirk_avoids_skb_reserve: udc/platform wants to avoid skb_reserve() in
+ *	u_ether.c to improve performance.
  * @is_selfpowered: if the gadget is self-powered.
  * @deactivated: True if gadget is deactivated - in deactivated state it cannot
  *	be connected.
@@ -398,6 +400,7 @@ struct usb_gadget {
 	unsigned			quirk_altset_not_supp:1;
 	unsigned			quirk_stall_not_supp:1;
 	unsigned			quirk_zlp_not_supp:1;
+	unsigned			quirk_avoids_skb_reserve:1;
 	unsigned			is_selfpowered:1;
 	unsigned			deactivated:1;
 	unsigned			connected:1;
@@ -463,6 +466,16 @@ static inline int gadget_is_zlp_supported(struct usb_gadget *g)
 }
 
 /**
+ * gadget_avoids_skb_reserve - return true iff the hardware would like to avoid
+ *	skb_reserve to improve performance.
+ * @g: controller to check for quirk
+ */
+static inline int gadget_avoids_skb_reserve(struct usb_gadget *g)
+{
+	return g->quirk_avoids_skb_reserve;
+}
+
+/**
  * gadget_is_dualspeed - return true iff the hardware handles high speed
  * @g: controller that might support both high and full speeds
  */
-- 
1.9.1

[PATCH/RFC 2/4] usb: gadget: u_ether: add a flag to avoid skb_reserve() calling

[Yoshihiro Shimoda]

This patch adds a flag "no_skb_reserve" in struct eth_dev.
So, if a peripheral driver sets the quirk_avoids_skb_reserve flag,
upper network gadget drivers (e.g. f_ncm.c) can avoid skb_reserve()
calling using the flag as well.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/gadget/function/u_ether.c | 5 ++++-
 drivers/usb/gadget/function/u_ether.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index a3f7e7c..04e30c6 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -82,6 +82,7 @@ struct eth_dev {
 #define	WORK_RX_MEMORY		0
 
 	bool			zlp;
+	bool			no_skb_reserve;
 	u8			host_mac[ETH_ALEN];
 	u8			dev_mac[ETH_ALEN];
 };
@@ -233,7 +234,8 @@ rx_submit(struct eth_dev *dev, struct usb_request *req, gfp_t gfp_flags)
 	 * but on at least one, checksumming fails otherwise.  Note:
 	 * RNDIS headers involve variable numbers of LE32 values.
 	 */
-	skb_reserve(skb, NET_IP_ALIGN);
+	if (likely(!dev->no_skb_reserve))
+		skb_reserve(skb, NET_IP_ALIGN);
 
 	req->buf = skb->data;
 	req->length = size;
@@ -1062,6 +1064,7 @@ struct net_device *gether_connect(struct gether *link)
 
 	if (result == 0) {
 		dev->zlp = link->is_zlp_ok;
+		dev->no_skb_reserve = link->no_skb_reserve;
 		DBG(dev, "qlen %d\n", qlen(dev->gadget, dev->qmult));
 
 		dev->header_len = link->header_len;
diff --git a/drivers/usb/gadget/function/u_ether.h b/drivers/usb/gadget/function/u_ether.h
index c77145b..81d94a7 100644
--- a/drivers/usb/gadget/function/u_ether.h
+++ b/drivers/usb/gadget/function/u_ether.h
@@ -64,6 +64,7 @@ struct gether {
 	struct usb_ep			*out_ep;
 
 	bool				is_zlp_ok;
+	bool				no_skb_reserve;
 
 	u16				cdc_filter;
 
-- 
1.9.1

[PATCH/RFC 3/4] usb: gadget: f_ncm: add support for no_skb_reserve

[Yoshihiro Shimoda]

This patch adds to support no_skb_reserve function to improve
performance for some platforms. About the detail, please refer to
the commit log of "quirk_avoids_skb_reserve" in
include/linux/usb/gadget.h.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/gadget/function/f_ncm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 97f0a9b..8c43298 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -852,6 +852,8 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 			 */
 			ncm->port.is_zlp_ok =
 				gadget_is_zlp_supported(cdev->gadget);
+			ncm->port.no_skb_reserve =
+				gadget_avoids_skb_reserve(cdev->gadget);
 			ncm->port.cdc_filter = DEFAULT_FILTER;
 			DBG(cdev, "activate ncm\n");
 			net = gether_connect(&ncm->port);
-- 
1.9.1

[PATCH/RFC 4/4] usb: renesas_usbhs: set quirk_avoids_skb_reserve if USB-DMAC is used

[Yoshihiro Shimoda]

This patch sets the quirk_avoids_skb_reserve flag to improve performance
of a network gadget driver (e.g. f_ncm.c) if USB-DMAC is used.

For example (on r8a7795 board + f_ncm.c + iperf udp mode / receiving):
  - without this patch:  90.3 Mbits/sec
  - with    this patch: 273   Mbits/sec

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/renesas_usbhs/mod_gadget.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 50f3363..b615c66 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -1103,6 +1103,8 @@ int usbhs_mod_gadget_probe(struct usbhs_priv *priv)
 	gpriv->gadget.name		= "renesas_usbhs_udc";
 	gpriv->gadget.ops		= &usbhsg_gadget_ops;
 	gpriv->gadget.max_speed		= USB_SPEED_HIGH;
+	gpriv->gadget.quirk_avoids_skb_reserve = usbhs_get_dparam(priv,
+								has_usb_dmac);
 
 	INIT_LIST_HEAD(&gpriv->gadget.ep_list);
 
-- 
1.9.1

Re: [PATCH/RFC 0/4] usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]


Yoshihiro-san,

Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> This patch set is based on the latest Felipe's usb.git / testing/next branch.
> (commit id = 15e4292a2d21e9997fdb2b8c014cc461b3f268f0)
>
> This patch set also needs an additional patch (like the end of this email)
> to fix the renesas_usbhs driver for R-Car Gen3.
> (I will submit this patch after v4.8-rc1 was released.)
>
> I'm not sure this is a correct way or not. So, I added "RFC" in this patch set.
> About the detail of this patch set, please refer to the commit log of
> first patch ("usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c").
>
> Yoshihiro Shimoda (4):
>   usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c
>   usb: gadget: u_ether: add a flag to avoid skb_reserve() calling
>   usb: gadget: f_ncm: add support for no_skb_reserve
>   usb: renesas_usbhs: set quirk_avoids_skb_reserve if USB-DMAC is used
>
>  drivers/usb/gadget/function/f_ncm.c    |  2 ++
>  drivers/usb/gadget/function/u_ether.c  |  5 ++++-
>  drivers/usb/gadget/function/u_ether.h  |  1 +
>  drivers/usb/renesas_usbhs/mod_gadget.c |  2 ++
>  include/linux/usb/gadget.h             | 13 +++++++++++++
>  5 files changed, 22 insertions(+), 1 deletion(-)

It has been over a month since you sent this series and nobody said
anything. Can you resend without RFC so I can apply?

Thank you

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

RE: [PATCH/RFC 0/4] usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c

[Yoshihiro Shimoda]

Hi Felipe-san,

> From: Felipe Balbi
> Sent: Thursday, August 18, 2016 4:35 PM
> 
> Yoshihiro-san,
> 
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> > This patch set is based on the latest Felipe's usb.git / testing/next branch.
> > (commit id = 15e4292a2d21e9997fdb2b8c014cc461b3f268f0)
> >
> > This patch set also needs an additional patch (like the end of this email)
> > to fix the renesas_usbhs driver for R-Car Gen3.
> > (I will submit this patch after v4.8-rc1 was released.)
> >
> > I'm not sure this is a correct way or not. So, I added "RFC" in this patch set.
> > About the detail of this patch set, please refer to the commit log of
> > first patch ("usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c").
> >
> > Yoshihiro Shimoda (4):
> >   usb: gadget: add a new quirk to avoid skb_reserve in u_ether.c
> >   usb: gadget: u_ether: add a flag to avoid skb_reserve() calling
> >   usb: gadget: f_ncm: add support for no_skb_reserve
> >   usb: renesas_usbhs: set quirk_avoids_skb_reserve if USB-DMAC is used
> >
> >  drivers/usb/gadget/function/f_ncm.c    |  2 ++
> >  drivers/usb/gadget/function/u_ether.c  |  5 ++++-
> >  drivers/usb/gadget/function/u_ether.h  |  1 +
> >  drivers/usb/renesas_usbhs/mod_gadget.c |  2 ++
> >  include/linux/usb/gadget.h             | 13 +++++++++++++
> >  5 files changed, 22 insertions(+), 1 deletion(-)
> 
> It has been over a month since you sent this series and nobody said
> anything. Can you resend without RFC so I can apply?

Sure! I will resend this patch set without RFC today (or tomorrow).

Best regards,
Yoshihiro Shimoda

> Thank you
> 
> --
> balbi