Re: [Stlinux-devel] FW: [PATCH (linux-stm) 1/2] usb: dwc3: fix kernel compilation in gadget mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Felipe,

I'm one of the maintainer of the STLinux kernel @ST.
Let me clarify the inconvenience occurred about this patches.

We are back-porting some patches from upstream for DWC3 driver into our 3.4.58 based kernel as our chipsets use this host controller.

The way we do it, is exactly what you referred to, by using 'git cherry-pick -xs' to keep authorship, upstream reference and so on, and when cherry-pick is not straightforward, we are used to specify what are the modification added by us wrt the upstream patch.

You might have a look at some recent backport stuff we did for our 3.4 kernel:

http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=cef7a3a85413142e73315463eae2fedea451490e

http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=71538603d521a6e1b5d5b4ad47c77e6f9dfc8882

(feel free to browse our git.stlinux.com)

You will see authorship not changed, properly upstream commit reference and so on.

Regarding the merged patches sent by the colleague Giuseppe, I asked him to provide me an unique patch just to check locally if the issue were actually fixed. It should have been a private, temporary hack for testing.

It was not meant to be applied in the kernel, neither he had the intention to change authorship.

Due to bad git configuration (and user mistakes), when he sent the patch it went to our devel-list plus you as in the signed-off list, but purely by mistake (--suppress-xxx neither passed)

As kernel maintainer, I can guarantee such a work will not included at all in our kernel. It has been just due to user mistakes.

We seriously consider copyright laws.

Our apologies for such inconvenience.

With best regards,
Carmelo

-----Original Message-----
From: Felipe Balbi [mailto:balbi@xxxxxx]
Sent: mercoledì 15 gennaio 2014 19:38
To: Giuseppe CONDORELLI
Cc: stlinux-devel@xxxxxxxxxxxxxxxxxxxxxx; Felipe Balbi; Linux USB Mailing List
Subject: Re: [PATCH (linux-stm) 1/2] usb: dwc3: fix kernel compilation in
gadget mode

On Wed, Jan 15, 2014 at 04:36:25PM +0100, Giuseppe Condorelli wrote:
The following set of patches, from the mainline, were applied to the
tree (after reworking) to solve build time issues about implicit
declaration of irq function and on arguments number mismatch in given
functions
(example: __dwc3_gadget_ep_enable).

[giuseppe.condorelli@xxxxxx: reworked patch to fit current gadget
code)
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@xxxxxx>

wait, what ?



I don't remember seeing this patch ever been sent upstream. This patch
seems to be a merge of several other patches, none of which you Authored,
so how come you:

a) put yourself as author of the code; and
b) add my Signed-off-by without asking me permission for it ?

In fact, you have *never* authored any patch on the dwc3 driver (I just
checked). If you want to get your tree in sync with the code which is mainline,
you should be using git cherry-pick -x to make sure that you maintain
authorship and make it clear which commit you cherry-picked so things can be
tracked down.

Copyright law is a serious business, I wonder what other patches have been
taken from mainline, changed author and applied to your vendor kernel with
my SoB.

I will be very clear:

	. Do *NOT* apply *any* patch you take from mainline with
		yourself as author.

	. Make sure to use git cherry-pick -xs and solve merge conflicts
		where applicable.

	. Do *NOT* use somebody else's {Signed-off,Acked,Tested,etc}-by
		tags without their permission.

Patch below so other people see what ST has been doing.

---
  drivers/usb/dwc3/core.h   |    1 +
  drivers/usb/dwc3/gadget.c |  101
++++++++++++++++++++++++++-------------------
  2 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
e91ddf8..610875b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -705,6 +705,7 @@ struct dwc3 {
  	unsigned		delayed_status:1;
  	unsigned		needs_fifo_resize:1;
  	unsigned		resize_fifos:1;
+	unsigned		pullups_connected:1;

  	enum dwc3_ep0_next	ep0_next_event;
  	enum dwc3_ep0_state	ep0state;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c0cc8f6..2c5f24b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -433,7 +433,8 @@ static int dwc3_gadget_start_config(struct dwc3
*dwc, struct dwc3_ep *dep)

  static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep
*dep,
  		const struct usb_endpoint_descriptor *desc,
-		const struct usb_ss_ep_comp_descriptor *comp_desc)
+		const struct usb_ss_ep_comp_descriptor *comp_desc,
+		bool ignore)
  {
  	struct dwc3_gadget_ep_cmd_params params;

@@ -443,6 +444,9 @@ static int dwc3_gadget_set_ep_config(struct dwc3
*dwc, struct dwc3_ep *dep,
  		|
DWC3_DEPCFG_MAX_PACKET_SIZE(usb_endpoint_maxp(desc))
  		| DWC3_DEPCFG_BURST_SIZE(dep->endpoint.maxburst);

+	if (ignore)
+		params.param0 |= DWC3_DEPCFG_IGN_SEQ_NUM;
+
  	params.param1 = DWC3_DEPCFG_XFER_COMPLETE_EN
  		| DWC3_DEPCFG_XFER_NOT_READY_EN;

@@ -500,7 +504,8 @@ static int dwc3_gadget_set_xfer_resource(struct
dwc3 *dwc, struct dwc3_ep *dep)
   */
  static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
  		const struct usb_endpoint_descriptor *desc,
-		const struct usb_ss_ep_comp_descriptor *comp_desc)
+		const struct usb_ss_ep_comp_descriptor *comp_desc,
+		bool ignore)
  {
  	struct dwc3		*dwc = dep->dwc;
  	u32			reg;
@@ -512,7 +517,7 @@ static int __dwc3_gadget_ep_enable(struct
dwc3_ep *dep,
  			return ret;
  	}

-	ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc);
+	ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc,
ignore);
  	if (ret)
  		return ret;

@@ -658,7 +663,7 @@ static int dwc3_gadget_ep_enable(struct usb_ep
*ep,
  	dev_vdbg(dwc->dev, "Enabling %s\n", dep->name);

  	spin_lock_irqsave(&dwc->lock, flags);
-	ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc);
+	ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc, false);
  	spin_unlock_irqrestore(&dwc->lock, flags);

  	return ret;
@@ -1364,8 +1369,10 @@ static void dwc3_gadget_run_stop(struct dwc3
*dwc, int is_on)
  		if (dwc->revision >= DWC3_REVISION_194A)
  			reg &= ~DWC3_DCTL_KEEP_CONNECT;
  		reg |= DWC3_DCTL_RUN_STOP;
+		dwc->pullups_connected = true;
  	} else {
  		reg &= ~DWC3_DCTL_RUN_STOP;
+		dwc->pullups_connected = false;
  	}

  	dwc3_writel(dwc->regs, DWC3_DCTL, reg); @@ -1405,6 +1412,32 @@
static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
  	return 0;
  }

+static void dwc3_gadget_enable_irq(struct dwc3 *dwc) {
+	u32			reg;
+
+	/* Enable all but Start and End of Frame IRQs */
+	reg = (DWC3_DEVTEN_VNDRDEVTSTRCVEDEN |
+		DWC3_DEVTEN_EVNTOVERFLOWEN |
+		DWC3_DEVTEN_CMDCMPLTEN |
+		DWC3_DEVTEN_ERRTICERREN |
+		DWC3_DEVTEN_WKUPEVTEN |
+		DWC3_DEVTEN_ULSTCNGEN |
+		DWC3_DEVTEN_CONNECTDONEEN |
+		DWC3_DEVTEN_USBRSTEN |
+		DWC3_DEVTEN_DISCONNEVTEN);
+
+	dwc3_writel(dwc->regs, DWC3_DEVTEN, reg);
+}
+
+static void dwc3_gadget_disable_irq(struct dwc3 *dwc) {
+	/* mask all interrupts */
+	dwc3_writel(dwc->regs, DWC3_DEVTEN, 0x00); }
+
+static irqreturn_t dwc3_interrupt(int irq, void *_dwc);
+
  static int dwc3_gadget_start(struct usb_gadget *g,
  		struct usb_gadget_driver *driver)
  {
@@ -1412,6 +1445,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
  	struct dwc3_ep		*dep;
  	unsigned long		flags;
  	int			ret = 0;
+	int			irq;
  	u32			reg;

  	spin_lock_irqsave(&dwc->lock, flags); @@ -1455,14 +1489,14 @@
static
int dwc3_gadget_start(struct usb_gadget *g,
  	dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);

  	dep = dwc->eps[0];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL);
+	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL,
+false);
  	if (ret) {
  		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
  		goto err0;
  	}

  	dep = dwc->eps[1];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL);
+	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL,
+false);
  	if (ret) {
  		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
  		goto err1;
@@ -1472,6 +1506,17 @@ static int dwc3_gadget_start(struct usb_gadget
*g,
  	dwc->ep0state = EP0_SETUP_PHASE;
  	dwc3_ep0_out_start(dwc);

+	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+	ret = request_irq(irq, dwc3_interrupt, IRQF_SHARED,
+				"dwc3", dwc);
+	if (ret) {
+		dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
+				irq, ret);
+		goto err1;
+	}
+
+	dwc3_gadget_enable_irq(dwc);
+
  	spin_unlock_irqrestore(&dwc->lock, flags);

  	return 0;
@@ -1491,9 +1536,14 @@ static int dwc3_gadget_stop(struct usb_gadget
*g,  {
  	struct dwc3		*dwc = gadget_to_dwc(g);
  	unsigned long		flags;
+	int			irq;

  	spin_lock_irqsave(&dwc->lock, flags);

+	dwc3_gadget_disable_irq(dwc);
+	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
+	free_irq(irq, dwc);
+
  	__dwc3_gadget_ep_disable(dwc->eps[0]);
  	__dwc3_gadget_ep_disable(dwc->eps[1]);

@@ -2131,14 +2181,14 @@ static void
dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
  	}

  	dep = dwc->eps[0];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL);
+	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL,
+true);
  	if (ret) {
  		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
  		return;
  	}

  	dep = dwc->eps[1];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL);
+	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc,
NULL,
+true);
  	if (ret) {
  		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
  		return;
@@ -2346,7 +2396,6 @@ int __devinit dwc3_gadget_init(struct dwc3 *dwc)
{
  	u32					reg;
  	int					ret;
-	int					irq;

  	dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc-
ctrl_req),
  			&dwc->ctrl_req_addr, GFP_KERNEL);
@@ -2404,16 +2453,6 @@ int __devinit dwc3_gadget_init(struct dwc3
*dwc)
  	if (ret)
  		goto err4;

-	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
-
-	ret = request_irq(irq, dwc3_interrupt, IRQF_SHARED,
-			"dwc3", dwc);
-	if (ret) {
-		dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
-				irq, ret);
-		goto err5;
-	}
-
  	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
  	reg |= DWC3_DCFG_LPM_CAP;
  	dwc3_writel(dwc->regs, DWC3_DCFG, reg); @@ -2422,18 +2461,6 @@
int
__devinit dwc3_gadget_init(struct dwc3 *dwc)
  	reg |= DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA;
  	dwc3_writel(dwc->regs, DWC3_DCTL, reg);

-	/* Enable all but Start and End of Frame IRQs */
-	reg = (DWC3_DEVTEN_VNDRDEVTSTRCVEDEN |
-			DWC3_DEVTEN_EVNTOVERFLOWEN |
-			DWC3_DEVTEN_CMDCMPLTEN |
-			DWC3_DEVTEN_ERRTICERREN |
-			DWC3_DEVTEN_WKUPEVTEN |
-			DWC3_DEVTEN_ULSTCNGEN |
-			DWC3_DEVTEN_CONNECTDONEEN |
-			DWC3_DEVTEN_USBRSTEN |
-			DWC3_DEVTEN_DISCONNEVTEN);
-	dwc3_writel(dwc->regs, DWC3_DEVTEN, reg);
-
  	/* Enable USB2 LPM and automatic phy suspend only on recent
versions */
  	if (dwc->revision >= DWC3_REVISION_194A) {
  		reg = dwc3_readl(dwc->regs, DWC3_DCFG); @@ -2456,7
+2483,7 @@ int
__devinit dwc3_gadget_init(struct dwc3 *dwc)
  	if (ret) {
  		dev_err(dwc->dev, "failed to register gadget device\n");
  		put_device(&dwc->gadget.dev);
-		goto err6;
+		goto err5;
  	}

  	ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget); @@ -2470,10
+2497,6 @@ int __devinit dwc3_gadget_init(struct dwc3 *dwc)
  err7:
  	device_unregister(&dwc->gadget.dev);

-err6:
-	dwc3_writel(dwc->regs, DWC3_DEVTEN, 0x00);
-	free_irq(irq, dwc);
-
  err5:
  	dwc3_gadget_free_endpoints(dwc);

@@ -2500,13 +2523,7 @@ err0:

  void dwc3_gadget_exit(struct dwc3 *dwc)  {
-	int			irq;
-
  	usb_del_gadget_udc(&dwc->gadget);
-	irq = platform_get_irq(to_platform_device(dwc->dev), 0);
-
-	dwc3_writel(dwc->regs, DWC3_DEVTEN, 0x00);
-	free_irq(irq, dwc);

  	dwc3_gadget_free_endpoints(dwc);

--
1.7.7.6


--
balbi


_______________________________________________
Stlinux-devel mailing list
Stlinux-devel@xxxxxxxxxxxxxxxxxxxxxx
https://lists.codex.cro.st.com/mailman/listinfo/stlinux-devel

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux