All of lore.kernel.org
 help / color / mirror / Atom feed
* [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
@ 2018-06-07 10:38 Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2018-06-07 10:38 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman; +Cc: Hans de Goede, linux-usb

On some Bay Trail (BYT) systems the firmware does not enable the
ULPI Refclk.

This commit adds a helper which checks and if necessary enabled the Refclk
and calls this helper for BYT machines.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/dwc3/dwc3-pci.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 34b84d3bc7cf..65bc110388f3 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -42,6 +42,9 @@
 #define PCI_INTEL_BXT_STATE_D0		0
 #define PCI_INTEL_BXT_STATE_D3		3
 
+#define GP_RWREG1			0xa0
+#define GP_RWREG1_ULPI_REFCLK_DISABLE	(1 << 17)
+
 /**
  * struct dwc3_pci - Driver private structure
  * @dwc3: child dwc3 platform_device
@@ -78,6 +81,34 @@ static struct gpiod_lookup_table platform_bytcr_gpios = {
 	},
 };
 
+static void dwc3_pci_enable_ulpi_refclock(struct pci_dev *pci)
+{
+	void __iomem	*reg;
+	struct resource	res;
+	struct device	*dev = &pci->dev;
+	u32		value;
+
+	res.start	= pci_resource_start(pci, 1);
+	res.end		= pci_resource_end(pci, 1);
+	res.name	= "dwc_usb3_bar1";
+	res.flags	= IORESOURCE_MEM;
+
+	reg = devm_ioremap_resource(dev, &res);
+	if (IS_ERR(reg)) {
+		dev_err(dev, "cannot check GP_RWREG1 to assert ulpi refclock\n");
+		return;
+	}
+
+	value = readl(reg + GP_RWREG1);
+	if (!(value & GP_RWREG1_ULPI_REFCLK_DISABLE))
+		return; /* ULPI refclk already enabled */
+
+	dev_warn(dev, "ULPI refclock is disabled from the BIOS, enabling it\n");
+	value &= ~GP_RWREG1_ULPI_REFCLK_DISABLE;
+	writel(value, reg + GP_RWREG1);
+	msleep(100);
+}
+
 static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 {
 	struct platform_device		*dwc3 = dwc->dwc3;
@@ -134,6 +165,9 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 			struct gpio_desc *gpio;
 			const char *vendor;
 
+			/* On BYT the FW does not always enable the refclock */
+			dwc3_pci_enable_ulpi_refclock(pdev);
+
 			ret = devm_acpi_dev_add_driver_gpios(&pdev->dev,
 					acpi_dwc3_byt_gpios);
 			if (ret)

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

* [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
@ 2018-06-07 17:23 Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-06-07 17:23 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Felipe Balbi, Greg Kroah-Hartman, USB

On Thu, Jun 7, 2018 at 4:42 PM, Hans de Goede <hdegoede@redhat.com> wrote:

>>> +       msleep(100);
>>
>>
>> This has to be explained.
>
>
> Erm, this comes 1:1 from Intels Android X86 patches I've no
> idea why it is there, I believe it is better to leave this
> uncommented then making something up.

The above would be a good candidate

/* This comes from the Intel Android x86 tree w/o any explanation */

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

* [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
@ 2018-06-07 17:01 Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2018-06-07 17:01 UTC (permalink / raw)
  To: Sergei Shtylyov, Felipe Balbi, Greg Kroah-Hartman; +Cc: linux-usb

Hi,

On 07-06-18 18:43, Sergei Shtylyov wrote:
> Hello!
> 
> On 06/07/2018 01:38 PM, Hans de Goede wrote:
> 
>> On some Bay Trail (BYT) systems the firmware does not enable the
>> ULPI Refclk.
>>
>> This commit adds a helper which checks and if necessary enabled the Refclk
>> and calls this helper for BYT machines.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/usb/dwc3/dwc3-pci.c | 34 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 34 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
>> index 34b84d3bc7cf..65bc110388f3 100644
>> --- a/drivers/usb/dwc3/dwc3-pci.c
>> +++ b/drivers/usb/dwc3/dwc3-pci.c
> [...]
>> @@ -78,6 +81,34 @@ static struct gpiod_lookup_table platform_bytcr_gpios = {
>>   	},
>>   };
>>   
>> +static void dwc3_pci_enable_ulpi_refclock(struct pci_dev *pci)
>> +{
>> +	void __iomem	*reg;
>> +	struct resource	res;
>> +	struct device	*dev = &pci->dev;
>> +	u32		value;
>> +
>> +	res.start	= pci_resource_start(pci, 1);
>> +	res.end		= pci_resource_end(pci, 1);
>> +	res.name	= "dwc_usb3_bar1";
>> +	res.flags	= IORESOURCE_MEM;
>> +
>> +	reg = devm_ioremap_resource(dev, &res);
>> +	if (IS_ERR(reg)) {
>> +		dev_err(dev, "cannot check GP_RWREG1 to assert ulpi refclock\n");
> 
>     Eh?
>     BTW, devm_ioremap_resource() prints the error cause already...
> 
>> +		return;
>> +	}
>> +
>> +	value = readl(reg + GP_RWREG1);
>> +	if (!(value & GP_RWREG1_ULPI_REFCLK_DISABLE))
> 
>     I guess that dev_err() belongs here...

Nope the error is that GP_RWREG1 cannot be read because the remap
fails. I agree that this is a bit silly and I will fix for v2,
but returning here without printing anything is correct.

Regards,

Hans



> 
>> +		return; /* ULPI refclk already enabled */
>> +
>> +	dev_warn(dev, "ULPI refclock is disabled from the BIOS, enabling it\n");
>> +	value &= ~GP_RWREG1_ULPI_REFCLK_DISABLE;
>> +	writel(value, reg + GP_RWREG1);
>> +	msleep(100);
>> +}
>> +
>>   static int dwc3_pci_quirks(struct dwc3_pci *dwc)
>>   {
>>   	struct platform_device		*dwc3 = dwc->dwc3;
> [...]
> 
> MBR, Sergei
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
@ 2018-06-07 16:43 Sergei Shtylyov
  0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2018-06-07 16:43 UTC (permalink / raw)
  To: Hans de Goede, Felipe Balbi, Greg Kroah-Hartman; +Cc: linux-usb

Hello!

On 06/07/2018 01:38 PM, Hans de Goede wrote:

> On some Bay Trail (BYT) systems the firmware does not enable the
> ULPI Refclk.
> 
> This commit adds a helper which checks and if necessary enabled the Refclk
> and calls this helper for BYT machines.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/usb/dwc3/dwc3-pci.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 34b84d3bc7cf..65bc110388f3 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
[...]
> @@ -78,6 +81,34 @@ static struct gpiod_lookup_table platform_bytcr_gpios = {
>  	},
>  };
>  
> +static void dwc3_pci_enable_ulpi_refclock(struct pci_dev *pci)
> +{
> +	void __iomem	*reg;
> +	struct resource	res;
> +	struct device	*dev = &pci->dev;
> +	u32		value;
> +
> +	res.start	= pci_resource_start(pci, 1);
> +	res.end		= pci_resource_end(pci, 1);
> +	res.name	= "dwc_usb3_bar1";
> +	res.flags	= IORESOURCE_MEM;
> +
> +	reg = devm_ioremap_resource(dev, &res);
> +	if (IS_ERR(reg)) {
> +		dev_err(dev, "cannot check GP_RWREG1 to assert ulpi refclock\n");

   Eh?
   BTW, devm_ioremap_resource() prints the error cause already...

> +		return;
> +	}
> +
> +	value = readl(reg + GP_RWREG1);
> +	if (!(value & GP_RWREG1_ULPI_REFCLK_DISABLE))

   I guess that dev_err() belongs here...

> +		return; /* ULPI refclk already enabled */
> +
> +	dev_warn(dev, "ULPI refclock is disabled from the BIOS, enabling it\n");
> +	value &= ~GP_RWREG1_ULPI_REFCLK_DISABLE;
> +	writel(value, reg + GP_RWREG1);
> +	msleep(100);
> +}
> +
>  static int dwc3_pci_quirks(struct dwc3_pci *dwc)
>  {
>  	struct platform_device		*dwc3 = dwc->dwc3;
[...]

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

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

* [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
@ 2018-06-07 13:42 Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2018-06-07 13:42 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Felipe Balbi, Greg Kroah-Hartman, USB

Hi,

On 07-06-18 15:30, Andy Shevchenko wrote:
> On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> On some Bay Trail (BYT) systems the firmware does not enable the
>> ULPI Refclk.
>>
>> This commit adds a helper which checks and if necessary enabled the Refclk
>> and calls this helper for BYT machines.
> 
> 
>> +static void dwc3_pci_enable_ulpi_refclock(struct pci_dev *pci)
>> +{
>> +       void __iomem    *reg;
>> +       struct resource res;
>> +       struct device   *dev = &pci->dev;
>> +       u32             value;
>> +
> 
>> +       res.start       = pci_resource_start(pci, 1);
>> +       res.end         = pci_resource_end(pci, 1);
>> +       res.name        = "dwc_usb3_bar1";
>> +       res.flags       = IORESOURCE_MEM;
>> +
>> +       reg = devm_ioremap_resource(dev, &res);
>> +       if (IS_ERR(reg)) {
>> +               dev_err(dev, "cannot check GP_RWREG1 to assert ulpi refclock\n");
>> +               return;
>> +       }
> 
> I'm not sure I understand what's wrong with simple
> pci_iomap() & Co (perhaps pcim_iomap() / pcim_iomap_regions() and others)

Good point, I took this from the crufty Android X86 patched which
Intel maintains here:
https://github.com/01org/ProductionKernelQuilts

And did not realize I could simplify this, will fix for v2.

>> +
>> +       value = readl(reg + GP_RWREG1);
>> +       if (!(value & GP_RWREG1_ULPI_REFCLK_DISABLE))
>> +               return; /* ULPI refclk already enabled */
>> +
>> +       dev_warn(dev, "ULPI refclock is disabled from the BIOS, enabling it\n");
>> +       value &= ~GP_RWREG1_ULPI_REFCLK_DISABLE;
>> +       writel(value, reg + GP_RWREG1);
> 
>> +       msleep(100);
> 
> This has to be explained.

Erm, this comes 1:1 from Intels Android X86 patches I've no
idea why it is there, I believe it is better to leave this
uncommented then making something up.

Regards,

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

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

* [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot
@ 2018-06-07 13:30 Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-06-07 13:30 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Felipe Balbi, Greg Kroah-Hartman, USB

On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On some Bay Trail (BYT) systems the firmware does not enable the
> ULPI Refclk.
>
> This commit adds a helper which checks and if necessary enabled the Refclk
> and calls this helper for BYT machines.


> +static void dwc3_pci_enable_ulpi_refclock(struct pci_dev *pci)
> +{
> +       void __iomem    *reg;
> +       struct resource res;
> +       struct device   *dev = &pci->dev;
> +       u32             value;
> +

> +       res.start       = pci_resource_start(pci, 1);
> +       res.end         = pci_resource_end(pci, 1);
> +       res.name        = "dwc_usb3_bar1";
> +       res.flags       = IORESOURCE_MEM;
> +
> +       reg = devm_ioremap_resource(dev, &res);
> +       if (IS_ERR(reg)) {
> +               dev_err(dev, "cannot check GP_RWREG1 to assert ulpi refclock\n");
> +               return;
> +       }

I'm not sure I understand what's wrong with simple
pci_iomap() & Co (perhaps pcim_iomap() / pcim_iomap_regions() and others)

> +
> +       value = readl(reg + GP_RWREG1);
> +       if (!(value & GP_RWREG1_ULPI_REFCLK_DISABLE))
> +               return; /* ULPI refclk already enabled */
> +
> +       dev_warn(dev, "ULPI refclock is disabled from the BIOS, enabling it\n");
> +       value &= ~GP_RWREG1_ULPI_REFCLK_DISABLE;
> +       writel(value, reg + GP_RWREG1);

> +       msleep(100);

This has to be explained.

> +}

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

end of thread, other threads:[~2018-06-07 17:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 10:38 [2/3] usb: dwc3: pci: Enable ULPI Refclk on platforms where the firmware doesnot Hans de Goede
2018-06-07 13:30 Andy Shevchenko
2018-06-07 13:42 Hans de Goede
2018-06-07 16:43 Sergei Shtylyov
2018-06-07 17:01 Hans de Goede
2018-06-07 17:23 Andy Shevchenko

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.