All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
@ 2016-06-01  6:44 Yoshihiro Shimoda
  2016-06-01  6:44 ` [PATCH v2 1/2] usb: host: xhci: plat: add ->quirks value for platform-specific Yoshihiro Shimoda
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Yoshihiro Shimoda @ 2016-06-01  6:44 UTC (permalink / raw)
  To: mathias.nyman, gregkh; +Cc: linux-usb, linux-renesas-soc, Yoshihiro Shimoda

I'm afraid but I found a regression of xhci-rcar in v4.7-rc1.
This regression is caused by the following commit:

commit b1c127ae990bccf0187d741c1695a61e54de1943
Author: Felipe Balbi <felipe.balbi@linux.intel.com>
Date:   Fri Apr 22 13:17:16 2016 +0300

    usb: host: xhci: plat: make use of new methods in xhci_plat_priv
    
    Now that the code has been refactored enough,
    switching over to using ->plat_start() and
    ->init_quirk() becomes a very simple patch.
    
    After this patch, there are no further uses for
    xhci_plat_type_is() which will be removed in a
    follow-up patch.
    
    Acked-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
    Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
    Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

< Overview >
The regression is the quirks flag "XHCI_NO_64BIT_SUPPORT" will be overwritten
by xhci_gen_setup(). Then, the driver will not work correctly.

< Detail >
Since the previous code will do the following, the quirks flag can be set:

xhci_plat_setup()
 -> xhci_gen_setup(hcd, xhci_plat_quirks);
  -> xhci->quirks = quirks;
   -> get_quirks() [This is xhci_plat_quirks]
    -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT

However, after we applied the patch above, the quirks will disappear:

xhci_plat_setup()
 -> xhci_priv_init_quirk();
  -> xhci_rcar_init_quirk();
   -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
  -> xhci_gen_setup(hcd, xhci_plat_quirks);
   -> xhci->quirks = quirks; <----------------- here
    -> get_quirks() [This is xhci_plat_quirks]

So, I submitted incremental patches to resolve this issue like the followings:

xhci_plat_setup()
 -> xhci_priv_init_quirk();
  -> xhci_rcar_init_quirk();
  -> xhci_gen_setup(hcd, xhci_plat_quirks);
   -> xhci->quirks = quirks;
    -> get_quirks() [This is xhci_plat_quirks]
     -> xhci->quirks |= priv->quirks (XHCI_NO_64BIT_SUPPORT)

Changes from v1:
 - Revise the commit log in patch 2 because checkpatch.pl said the patch
   has ERRROR.

Yoshihiro Shimoda (2):
  usb: host: xhci: plat: add ->quirks value for platform-specific
  usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT

 drivers/usb/host/xhci-plat.c | 12 +++++++++++-
 drivers/usb/host/xhci-plat.h |  2 ++
 drivers/usb/host/xhci-rcar.c | 21 ---------------------
 3 files changed, 13 insertions(+), 22 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] usb: host: xhci: plat: add ->quirks value for platform-specific
  2016-06-01  6:44 [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
@ 2016-06-01  6:44 ` Yoshihiro Shimoda
  2016-06-01  6:44 ` [PATCH v2 2/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
  2016-06-01  7:01 ` [PATCH v2 0/2] " Felipe Balbi
  2 siblings, 0 replies; 8+ messages in thread
From: Yoshihiro Shimoda @ 2016-06-01  6:44 UTC (permalink / raw)
  To: mathias.nyman, gregkh; +Cc: linux-usb, linux-renesas-soc, Yoshihiro Shimoda

This patch adds quirks in struct xhci_plat_priv to set xhci->quirks
for platform-specific.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/host/xhci-plat.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 9af0cb4..c652f33 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -17,6 +17,7 @@ struct xhci_plat_priv {
 	const char *firmware_name;
 	void (*plat_start)(struct usb_hcd *);
 	int (*init_quirk)(struct usb_hcd *);
+	unsigned int quirks;
 };
 
 #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
  2016-06-01  6:44 [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
  2016-06-01  6:44 ` [PATCH v2 1/2] usb: host: xhci: plat: add ->quirks value for platform-specific Yoshihiro Shimoda
@ 2016-06-01  6:44 ` Yoshihiro Shimoda
  2016-06-01  7:01 ` [PATCH v2 0/2] " Felipe Balbi
  2 siblings, 0 replies; 8+ messages in thread
From: Yoshihiro Shimoda @ 2016-06-01  6:44 UTC (permalink / raw)
  To: mathias.nyman, gregkh; +Cc: linux-usb, linux-renesas-soc, Yoshihiro Shimoda

Since the commit b1c127ae990b ("usb: host: xhci: plat: make use of new
methods in xhci_plat_priv") changed the setting timing of xhci->quirks to
xhci_rcar_init_quirk(), the quirks was overwritten by xhci_gen_setup().

So, this patch fixes the issue using a "quirks" of struct xhci_plat_priv.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/host/xhci-plat.c | 12 +++++++++++-
 drivers/usb/host/xhci-plat.h |  1 +
 drivers/usb/host/xhci-rcar.c | 21 ---------------------
 3 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 676ea45..7f03608 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -57,12 +57,14 @@ static int xhci_priv_init_quirk(struct usb_hcd *hcd)
 
 static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
+	struct xhci_plat_priv *priv = xhci_to_priv(xhci);
+
 	/*
 	 * As of now platform drivers don't provide MSI support so we ensure
 	 * here that the generic code does not try to make a pci_dev from our
 	 * dev struct in order to setup MSI
 	 */
-	xhci->quirks |= XHCI_PLAT;
+	xhci->quirks |= XHCI_PLAT | priv->quirks;
 }
 
 /* called during probe() after chip reset completes */
@@ -89,16 +91,24 @@ static const struct xhci_plat_priv xhci_plat_marvell_armada = {
 	.init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
+/*
+ * On R-Car Gen2 and Gen3, the AC64 bit (bit 0) of HCCPARAMS1 is set to 1.
+ * However, these SoCs don't support 64-bit address memory pointers. So, this
+ * driver clears the AC64 bit of xhci->hcc_params to call
+ * dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in xhci_gen_setup().
+ */
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
 	.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
 	.init_quirk = xhci_rcar_init_quirk,
 	.plat_start = xhci_rcar_start,
+	.quirks = XHCI_NO_64BIT_SUPPORT,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
 	.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
 	.init_quirk = xhci_rcar_init_quirk,
 	.plat_start = xhci_rcar_start,
+	.quirks = XHCI_NO_64BIT_SUPPORT,
 };
 
 static const struct of_device_id usb_xhci_of_match[] = {
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index c652f33..15144c9 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -20,5 +20,6 @@ struct xhci_plat_priv {
 	unsigned int quirks;
 };
 
+#define xhci_to_priv(x) ((struct xhci_plat_priv *)(x)->priv)
 #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
 #endif	/* _XHCI_PLAT_H */
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
index 0e4535e..2617cd7 100644
--- a/drivers/usb/host/xhci-rcar.c
+++ b/drivers/usb/host/xhci-rcar.c
@@ -87,14 +87,6 @@ static int xhci_rcar_is_gen2(struct device *dev)
 		of_device_is_compatible(node, "renensas,rcar-gen2-xhci");
 }
 
-static int xhci_rcar_is_gen3(struct device *dev)
-{
-	struct device_node *node = dev->of_node;
-
-	return of_device_is_compatible(node, "renesas,xhci-r8a7795") ||
-		of_device_is_compatible(node, "renesas,rcar-gen3-xhci");
-}
-
 void xhci_rcar_start(struct usb_hcd *hcd)
 {
 	u32 temp;
@@ -175,22 +167,9 @@ static int xhci_rcar_download_firmware(struct usb_hcd *hcd)
 /* This function needs to initialize a "phy" of usb before */
 int xhci_rcar_init_quirk(struct usb_hcd *hcd)
 {
-	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
-
 	/* If hcd->regs is NULL, we don't just call the following function */
 	if (!hcd->regs)
 		return 0;
 
-	/*
-	 * On R-Car Gen2 and Gen3, the AC64 bit (bit 0) of HCCPARAMS1 is set
-	 * to 1. However, these SoCs don't support 64-bit address memory
-	 * pointers. So, this driver clears the AC64 bit of xhci->hcc_params
-	 * to call dma_set_coherent_mask(dev, DMA_BIT_MASK(32)) in
-	 * xhci_gen_setup().
-	 */
-	if (xhci_rcar_is_gen2(hcd->self.controller) ||
-			xhci_rcar_is_gen3(hcd->self.controller))
-		xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
-
 	return xhci_rcar_download_firmware(hcd);
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
  2016-06-01  6:44 [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
  2016-06-01  6:44 ` [PATCH v2 1/2] usb: host: xhci: plat: add ->quirks value for platform-specific Yoshihiro Shimoda
  2016-06-01  6:44 ` [PATCH v2 2/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
@ 2016-06-01  7:01 ` Felipe Balbi
  2016-06-01  7:47   ` Yoshihiro Shimoda
  2 siblings, 1 reply; 8+ messages in thread
From: Felipe Balbi @ 2016-06-01  7:01 UTC (permalink / raw)
  To: Yoshihiro Shimoda, mathias.nyman, gregkh
  Cc: linux-usb, linux-renesas-soc, Yoshihiro Shimoda

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

Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:

> I'm afraid but I found a regression of xhci-rcar in v4.7-rc1.
> This regression is caused by the following commit:
>
> commit b1c127ae990bccf0187d741c1695a61e54de1943
> Author: Felipe Balbi <felipe.balbi@linux.intel.com>
> Date:   Fri Apr 22 13:17:16 2016 +0300
>
>     usb: host: xhci: plat: make use of new methods in xhci_plat_priv
>     
>     Now that the code has been refactored enough,
>     switching over to using ->plat_start() and
>     ->init_quirk() becomes a very simple patch.
>     
>     After this patch, there are no further uses for
>     xhci_plat_type_is() which will be removed in a
>     follow-up patch.
>     
>     Acked-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>     Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>     Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> < Overview >
> The regression is the quirks flag "XHCI_NO_64BIT_SUPPORT" will be overwritten
> by xhci_gen_setup(). Then, the driver will not work correctly.
>
> < Detail >
> Since the previous code will do the following, the quirks flag can be set:
>
> xhci_plat_setup()
>  -> xhci_gen_setup(hcd, xhci_plat_quirks);
>   -> xhci->quirks = quirks;
>    -> get_quirks() [This is xhci_plat_quirks]
>     -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
>
> However, after we applied the patch above, the quirks will disappear:
>
> xhci_plat_setup()
>  -> xhci_priv_init_quirk();
>   -> xhci_rcar_init_quirk();
>    -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
>   -> xhci_gen_setup(hcd, xhci_plat_quirks);
>    -> xhci->quirks = quirks; <----------------- here
>     -> get_quirks() [This is xhci_plat_quirks]
>
> So, I submitted incremental patches to resolve this issue like the followings:
>
> xhci_plat_setup()
>  -> xhci_priv_init_quirk();
>   -> xhci_rcar_init_quirk();
>   -> xhci_gen_setup(hcd, xhci_plat_quirks);
>    -> xhci->quirks = quirks;
>     -> get_quirks() [This is xhci_plat_quirks]
>      -> xhci->quirks |= priv->quirks (XHCI_NO_64BIT_SUPPORT)

isn't the following enough?

@@ -4886,7 +4886,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
 	xhci_print_registers(xhci);
 
-	xhci->quirks = quirks;
+	xhci->quirks |= quirks;
 
 	get_quirks(dev, xhci);
 
-- 
balbi

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
  2016-06-01  7:01 ` [PATCH v2 0/2] " Felipe Balbi
@ 2016-06-01  7:47   ` Yoshihiro Shimoda
  2016-06-01  7:49     ` Felipe Balbi
  0 siblings, 1 reply; 8+ messages in thread
From: Yoshihiro Shimoda @ 2016-06-01  7:47 UTC (permalink / raw)
  To: Felipe Balbi, mathias.nyman, gregkh; +Cc: linux-usb, linux-renesas-soc

Hi Felipe,

> From: Felipe Balbi
> Sent: Wednesday, June 01, 2016 4:01 PM
> 
> Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
> 
> > I'm afraid but I found a regression of xhci-rcar in v4.7-rc1.
> > This regression is caused by the following commit:
> >
> > commit b1c127ae990bccf0187d741c1695a61e54de1943
> > Author: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Date:   Fri Apr 22 13:17:16 2016 +0300
> >
> >     usb: host: xhci: plat: make use of new methods in xhci_plat_priv
> >
> >     Now that the code has been refactored enough,
> >     switching over to using ->plat_start() and
> >     ->init_quirk() becomes a very simple patch.
> >
> >     After this patch, there are no further uses for
> >     xhci_plat_type_is() which will be removed in a
> >     follow-up patch.
> >
> >     Acked-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >     Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> >     Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> >     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > < Overview >
> > The regression is the quirks flag "XHCI_NO_64BIT_SUPPORT" will be overwritten
> > by xhci_gen_setup(). Then, the driver will not work correctly.
> >
> > < Detail >
> > Since the previous code will do the following, the quirks flag can be set:
> >
> > xhci_plat_setup()
> >  -> xhci_gen_setup(hcd, xhci_plat_quirks);
> >   -> xhci->quirks = quirks;
> >    -> get_quirks() [This is xhci_plat_quirks]
> >     -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
> >
> > However, after we applied the patch above, the quirks will disappear:
> >
> > xhci_plat_setup()
> >  -> xhci_priv_init_quirk();
> >   -> xhci_rcar_init_quirk();
> >    -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
> >   -> xhci_gen_setup(hcd, xhci_plat_quirks);
> >    -> xhci->quirks = quirks; <----------------- here
> >     -> get_quirks() [This is xhci_plat_quirks]
> >
> > So, I submitted incremental patches to resolve this issue like the followings:
> >
> > xhci_plat_setup()
> >  -> xhci_priv_init_quirk();
> >   -> xhci_rcar_init_quirk();
> >   -> xhci_gen_setup(hcd, xhci_plat_quirks);
> >    -> xhci->quirks = quirks;
> >     -> get_quirks() [This is xhci_plat_quirks]
> >      -> xhci->quirks |= priv->quirks (XHCI_NO_64BIT_SUPPORT)
> 
> isn't the following enough?
> 
> @@ -4886,7 +4886,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
>  	xhci_print_registers(xhci);
> 
> -	xhci->quirks = quirks;
> +	xhci->quirks |= quirks;
> 
>  	get_quirks(dev, xhci);

Thank you for the comment!
You're correct. This also can resolve the issue.
Do you prefer such a simple patch?
At least, I prefer such a simple patch :)
Why I wrote this patch set is I thought I should implement similar flow before regression.

Best regards,
Yoshihiro Shimoda

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
  2016-06-01  7:47   ` Yoshihiro Shimoda
@ 2016-06-01  7:49     ` Felipe Balbi
  2016-06-01 11:41       ` Mathias Nyman
  0 siblings, 1 reply; 8+ messages in thread
From: Felipe Balbi @ 2016-06-01  7:49 UTC (permalink / raw)
  To: Yoshihiro Shimoda, mathias.nyman, gregkh; +Cc: linux-usb, linux-renesas-soc

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


Hi,

Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
>> From: Felipe Balbi
>> Sent: Wednesday, June 01, 2016 4:01 PM
>> 
>> Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> writes:
>> 
>> > I'm afraid but I found a regression of xhci-rcar in v4.7-rc1.
>> > This regression is caused by the following commit:
>> >
>> > commit b1c127ae990bccf0187d741c1695a61e54de1943
>> > Author: Felipe Balbi <felipe.balbi@linux.intel.com>
>> > Date:   Fri Apr 22 13:17:16 2016 +0300
>> >
>> >     usb: host: xhci: plat: make use of new methods in xhci_plat_priv
>> >
>> >     Now that the code has been refactored enough,
>> >     switching over to using ->plat_start() and
>> >     ->init_quirk() becomes a very simple patch.
>> >
>> >     After this patch, there are no further uses for
>> >     xhci_plat_type_is() which will be removed in a
>> >     follow-up patch.
>> >
>> >     Acked-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> >     Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
>> >     Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> >     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> >
>> > < Overview >
>> > The regression is the quirks flag "XHCI_NO_64BIT_SUPPORT" will be overwritten
>> > by xhci_gen_setup(). Then, the driver will not work correctly.
>> >
>> > < Detail >
>> > Since the previous code will do the following, the quirks flag can be set:
>> >
>> > xhci_plat_setup()
>> >  -> xhci_gen_setup(hcd, xhci_plat_quirks);
>> >   -> xhci->quirks = quirks;
>> >    -> get_quirks() [This is xhci_plat_quirks]
>> >     -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
>> >
>> > However, after we applied the patch above, the quirks will disappear:
>> >
>> > xhci_plat_setup()
>> >  -> xhci_priv_init_quirk();
>> >   -> xhci_rcar_init_quirk();
>> >    -> xhci->quirks |= XHCI_NO_64BIT_SUPPORT
>> >   -> xhci_gen_setup(hcd, xhci_plat_quirks);
>> >    -> xhci->quirks = quirks; <----------------- here
>> >     -> get_quirks() [This is xhci_plat_quirks]
>> >
>> > So, I submitted incremental patches to resolve this issue like the followings:
>> >
>> > xhci_plat_setup()
>> >  -> xhci_priv_init_quirk();
>> >   -> xhci_rcar_init_quirk();
>> >   -> xhci_gen_setup(hcd, xhci_plat_quirks);
>> >    -> xhci->quirks = quirks;
>> >     -> get_quirks() [This is xhci_plat_quirks]
>> >      -> xhci->quirks |= priv->quirks (XHCI_NO_64BIT_SUPPORT)
>> 
>> isn't the following enough?
>> 
>> @@ -4886,7 +4886,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>>  		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
>>  	xhci_print_registers(xhci);
>> 
>> -	xhci->quirks = quirks;
>> +	xhci->quirks |= quirks;
>> 
>>  	get_quirks(dev, xhci);
>
> Thank you for the comment!
> You're correct. This also can resolve the issue.
> Do you prefer such a simple patch?
> At least, I prefer such a simple patch :)

I'll defer that to Mathias :-)

> Why I wrote this patch set is I thought I should implement similar
> flow before regression.

understood :-)

-- 
balbi

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
  2016-06-01  7:49     ` Felipe Balbi
@ 2016-06-01 11:41       ` Mathias Nyman
  2016-06-02  4:53         ` Yoshihiro Shimoda
  0 siblings, 1 reply; 8+ messages in thread
From: Mathias Nyman @ 2016-06-01 11:41 UTC (permalink / raw)
  To: Felipe Balbi, Yoshihiro Shimoda, mathias.nyman, gregkh
  Cc: linux-usb, linux-renesas-soc

>>> isn't the following enough?
>>>
>>> @@ -4886,7 +4886,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>>>   		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
>>>   	xhci_print_registers(xhci);
>>>
>>> -	xhci->quirks = quirks;
>>> +	xhci->quirks |= quirks;
>>>
>>>   	get_quirks(dev, xhci);
>>
>> Thank you for the comment!
>> You're correct. This also can resolve the issue.
>> Do you prefer such a simple patch?
>> At least, I prefer such a simple patch :)
>
> I'll defer that to Mathias :-)
>

I think that xhci->quirks |= quirks will do as a rc fix.

looks like setting xhci->quirks need some generic cleanup for usb-next.
Now in 4.7-rc1 we set xhci->quirks before xhci_gen_setup in xhci_priv_init_quirk(),
and during xhci_gen_setup() when copying module parameters quirk, and when calling
the get_quirk() callback.

There is nothing special happening between xhci_priv_init_quirk and when calling
get_quirks() in xhci_gen_setup() that need quirks to be set in two stages.

xhci pci drivers only use the get_quirk callback, platform drivers use both.

Looks like the get_quirk() callback is a legacy thing from when the memory for
the xhci struct was allocated in xhci_gen_setup, and xhci->quirks could only be
set after that.

Now xhci struct is part of the usb_hcd, so we could probably get rid of the get_quirk
callback alltogether by setting the pci quirks in xhci_pci_setup

-Mathias  

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT
  2016-06-01 11:41       ` Mathias Nyman
@ 2016-06-02  4:53         ` Yoshihiro Shimoda
  0 siblings, 0 replies; 8+ messages in thread
From: Yoshihiro Shimoda @ 2016-06-02  4:53 UTC (permalink / raw)
  To: Mathias Nyman, Felipe Balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-renesas-soc

Hi,

> From: Mathias Nyman
> Sent: Wednesday, June 01, 2016 8:42 PM
> 
> >>> isn't the following enough?
> >>>
> >>> @@ -4886,7 +4886,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
> >>>   		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
> >>>   	xhci_print_registers(xhci);
> >>>
> >>> -	xhci->quirks = quirks;
> >>> +	xhci->quirks |= quirks;
> >>>
> >>>   	get_quirks(dev, xhci);
> >>
> >> Thank you for the comment!
> >> You're correct. This also can resolve the issue.
> >> Do you prefer such a simple patch?
> >> At least, I prefer such a simple patch :)
> >
> > I'll defer that to Mathias :-)
> >
> 
> I think that xhci->quirks |= quirks will do as a rc fix.

Thank you very much for the comment and submitting such a patch!

> looks like setting xhci->quirks need some generic cleanup for usb-next.
> Now in 4.7-rc1 we set xhci->quirks before xhci_gen_setup in xhci_priv_init_quirk(),
> and during xhci_gen_setup() when copying module parameters quirk, and when calling
> the get_quirk() callback.
> 
> There is nothing special happening between xhci_priv_init_quirk and when calling
> get_quirks() in xhci_gen_setup() that need quirks to be set in two stages.
> 
> xhci pci drivers only use the get_quirk callback, platform drivers use both.
> 
> Looks like the get_quirk() callback is a legacy thing from when the memory for
> the xhci struct was allocated in xhci_gen_setup, and xhci->quirks could only be
> set after that.
> 
> Now xhci struct is part of the usb_hcd, so we could probably get rid of the get_quirk
> callback alltogether by setting the pci quirks in xhci_pci_setup

I understood it.

Best regards,
Yoshihiro Shimoda

> -Mathias

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-06-02  4:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01  6:44 [PATCH v2 0/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
2016-06-01  6:44 ` [PATCH v2 1/2] usb: host: xhci: plat: add ->quirks value for platform-specific Yoshihiro Shimoda
2016-06-01  6:44 ` [PATCH v2 2/2] usb: host: xhci: rcar: fix the quirks setting of XHCI_NO_64BIT_SUPPORT Yoshihiro Shimoda
2016-06-01  7:01 ` [PATCH v2 0/2] " Felipe Balbi
2016-06-01  7:47   ` Yoshihiro Shimoda
2016-06-01  7:49     ` Felipe Balbi
2016-06-01 11:41       ` Mathias Nyman
2016-06-02  4:53         ` Yoshihiro Shimoda

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.