Re: [PATCH 06/20] usb: host: ehci-sead3: Support probing using device tree

2016-08-09 Thread Jonas Gorski
Hi,

On 9 August 2016 at 14:35, Paul Burton  wrote:
> Introduce support for probing the SEAD3 EHCI driver using device tree.
>
> Signed-off-by: Paul Burton 
> ---
>
>  drivers/usb/host/ehci-sead3.c | 29 +++--
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-sead3.c b/drivers/usb/host/ehci-sead3.c
> index 3d86cc2..05db1ae 100644
> --- a/drivers/usb/host/ehci-sead3.c
> +++ b/drivers/usb/host/ehci-sead3.c
> @@ -20,6 +20,7 @@
>   */
>
>  #include 
> +#include 
>  #include 
>
>  static int ehci_sead3_setup(struct usb_hcd *hcd)
> @@ -96,15 +97,25 @@ static int ehci_hcd_sead3_drv_probe(struct 
> platform_device *pdev)
>  {
> struct usb_hcd *hcd;
> struct resource *res;
> -   int ret;
> +   int ret, irq;
>
> if (usb_disabled())
> return -ENODEV;
>
> -   if (pdev->resource[1].flags != IORESOURCE_IRQ) {
> -   pr_debug("resource[1] is not IORESOURCE_IRQ");
> -   return -ENOMEM;
> +   if (pdev->dev.of_node) {
> +   irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +   if (!irq) {
> +   dev_err(>dev, "failed to map IRQ\n");
> +   return -ENODEV;
> +   }
> +   } else {
> +   if (pdev->resource[1].flags != IORESOURCE_IRQ) {
> +   pr_debug("resource[1] is not IORESOURCE_IRQ");
> +   return -ENOMEM;
> +   }
> +   irq = pdev->resource[1].start;
> }
> +

Why not just use platform_get_irq(pdev, 0) instead of this whole
block? Then you wouldn't need to care anymore whether resource 1 is
the IRQ resource (and avoid a potential overrun when the device has
only one resource), or if it is probed through of.

> hcd = usb_create_hcd(_sead3_hc_driver, >dev, "SEAD-3");
> if (!hcd)
> return -ENOMEM;
> @@ -121,8 +132,7 @@ static int ehci_hcd_sead3_drv_probe(struct 
> platform_device *pdev)
> /* Root hub has integrated TT. */
> hcd->has_tt = 1;
>
> -   ret = usb_add_hcd(hcd, pdev->resource[1].start,
> - IRQF_SHARED);
> +   ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> if (ret == 0) {
> platform_set_drvdata(pdev, hcd);
> device_wakeup_enable(hcd->self.controller);
> @@ -172,12 +182,19 @@ static const struct dev_pm_ops sead3_ehci_pmops = {
>  #define SEAD3_EHCI_PMOPS NULL
>  #endif
>
> +static const struct of_device_id sead3_ehci_of_match[] = {
> +   { .compatible = "mti,sead3-ehci" },

I don't see this compatible documented anywhere, please add binding
documentation for it first (and CC devicetree@vger ).

> +   {},
> +};
> +MODULE_DEVICE_TABLE(of, sead3_ehci_of_match);
> +
>  static struct platform_driver ehci_hcd_sead3_driver = {
> .probe  = ehci_hcd_sead3_drv_probe,
> .remove = ehci_hcd_sead3_drv_remove,
> .shutdown   = usb_hcd_platform_shutdown,
> .driver = {
> .name   = "sead3-ehci",
> +   .of_match_table = sead3_ehci_of_match,
> .pm = SEAD3_EHCI_PMOPS,
> }
>  };

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


[PATCH] usb: ehci-orion: fix probe for !GENERIC_PHY

2015-08-23 Thread Jonas Gorski
Commit d445913ce0ab7f (usb: ehci-orion: add optional PHY support)
added support for optional phys, but devm_phy_optional_get returns
-ENOSYS if GENERIC_PHY is not enabled.

This causes probe failures, even when there are no phys specified:

[1.443365] orion-ehci f1058000.usb: init f1058000.usb fail, -38
[1.449403] orion-ehci: probe of f1058000.usb failed with error -38

Similar to dwc3, treat -ENOSYS as no phy.

Fixes: d445913ce0ab7f (usb: ehci-orion: add optional PHY support)

Signed-off-by: Jonas Gorski j...@openwrt.org
---
 drivers/usb/host/ehci-orion.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index bfcbb9a..ee8d5fa 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -224,7 +224,8 @@ static int ehci_orion_drv_probe(struct platform_device 
*pdev)
priv-phy = devm_phy_optional_get(pdev-dev, usb);
if (IS_ERR(priv-phy)) {
err = PTR_ERR(priv-phy);
-   goto err_phy_get;
+   if (err != -ENOSYS)
+   goto err_phy_get;
} else {
err = phy_init(priv-phy);
if (err)
-- 
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors

2014-01-22 Thread Jonas Gorski
Hi,

On Wed, 22 Jan 2014 20:28:26 +0100
Hans de Goede hdego...@redhat.com wrote:
 Hi,
 
 On 01/21/2014 08:39 PM, Florian Fainelli wrote:
  2014/1/21 Hans de Goede hdego...@redhat.com:
  This uses the already documented devicetree booleans for this.
 
  (I would greatly appreciate if you could CC people who gave you
  feedback on this before)
 
 Will do.
 
  A more informative commit message would be welcome, along with a
  reference to which Device Tree binding documentation you are referring
  to.
 
 I've added a reference to the bindings doc in the commit msg for my next 
 version.
 
  Signed-off-by: Hans de Goede hdego...@redhat.com
  ---
drivers/usb/host/Kconfig |  3 +++
drivers/usb/host/ehci-platform.c | 33 +++--
2 files changed, 34 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
  index 237d7b1..4af41f3 100644
  --- a/drivers/usb/host/Kconfig
  +++ b/drivers/usb/host/Kconfig
  @@ -256,6 +256,9 @@ config USB_EHCI_ATH79
config USB_EHCI_HCD_PLATFORM
   tristate Generic EHCI driver for a platform device
   depends on !PPC_OF
  +   # Support BE on architectures which have readl_be
  +   select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || 
  SPARC || PPC32 || PPC64)
  +   select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || 
  SPARC || PPC32 || PPC64)
 
  I do not think this is that simple nor correct for at least Microblaze
  and MIPS since they can run in either BE or LE mode, and those
  specific platforms should already do the proper select at the
  board/SoC level. This *might* be correct for SPARC, PPC32 and PPC64,
  although I believe some specific PPC64 boards can run in little-endian
  mode like the P-series, SPARC might too.
 
  It seems to me that you should not touch this and keep the existing
  selects in place, if it turns out that the selects are missing the
  error messages you added below are catching those misuses.
 
 As discussed with Alan, I will drop these lines from my next version.
 
   default n
   ---help---
 Adds an EHCI host driver for a generic platform device, which
  diff --git a/drivers/usb/host/ehci-platform.c 
  b/drivers/usb/host/ehci-platform.c
  index d8aebc0..5888abb 100644
  --- a/drivers/usb/host/ehci-platform.c
  +++ b/drivers/usb/host/ehci-platform.c
  @@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
 
   hcd-has_tt = pdata-has_tt;
   ehci-has_synopsys_hc_bug = pdata-has_synopsys_hc_bug;
  -   ehci-big_endian_desc = pdata-big_endian_desc;
  -   ehci-big_endian_mmio = pdata-big_endian_mmio;
  +   if (pdata-big_endian_desc)
  +   ehci-big_endian_desc = 1;
  +   if (pdata-big_endian_mmio)
  +   ehci-big_endian_mmio = 1;
 
   if (pdata-pre_setup) {
   retval = pdata-pre_setup(hcd);
  @@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device 
  *dev)
   struct resource *res_mem;
   struct usb_ehci_pdata *pdata = dev_get_platdata(dev-dev);
   struct ehci_platform_priv *priv;
  +   struct ehci_hcd *ehci;
   int err, irq, clk = 0;
 
   if (usb_disabled())
  @@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device 
  *dev)
   platform_set_drvdata(dev, hcd);
   dev-dev.platform_data = pdata;
   priv = hcd_to_ehci_priv(hcd);
  +   ehci = hcd_to_ehci(hcd);
 
   if (pdata == ehci_platform_defaults  dev-dev.of_node) {
  +   if (of_property_read_bool(dev-dev.of_node, 
  big-endian-regs))
  +   ehci-big_endian_mmio = 1;
  +
  +   if (of_property_read_bool(dev-dev.of_node, 
  big-endian-desc))
  +   ehci-big_endian_desc = 1;
  +
  +   if (of_property_read_bool(dev-dev.of_node, big-endian))
  +   ehci-big_endian_mmio = ehci-big_endian_desc = 1;
 
  Ok, so I am confused now, should you update
  pdata-ehci_big_endian_{desc,mmio} here or is it valid to directly
  modify ehci-big_endian_{desc,mmio}, is not there any risk  to undo
  what is done in ehci_platform_reset(), or is ehci_platform_reset()
  only called for non-DT cases?
 
 Both the pdata checks in ehci_platform_reset() and the dt checks here only
 ever set these flags, neither code path clears them. And in the dt case pdata
 will be NULL and vice versa.

If it's safe to set ehci-big_endian_{desc,mmio} from the _probe()
routine, then maybe the pdata sets in _reset() should be moved into here
instead of adding extra cludges/checks into _reset().

 
 
  +
  +#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
  +   if (ehci-big_endian_mmio) {
  +   dev_err(dev-dev,
  +   Error big-endian-regs not compiled in\n);
 
  I do not think using the Device Tree property name would be very
  informative 

Re: [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors

2014-01-22 Thread Jonas Gorski
On Wed, 22 Jan 2014 16:17:42 -0500 (EST)
Alan Stern st...@rowland.harvard.edu wrote:

 On Wed, 22 Jan 2014, Jonas Gorski wrote:
 
  If it's safe to set ehci-big_endian_{desc,mmio} from the _probe()
  routine, then maybe the pdata sets in _reset() should be moved into here
  instead of adding extra cludges/checks into _reset().
 
 Why?  What difference would it make?

Effectivewise none, but to me it seems to be cleaner to set them once in
probe() instead of everytime reset() is called.

I admit I don't know the code flow good enough if reset() is called
more than once in the lifetime of a hcd device.

And as I said, it would allow doing the checks the patch adds for both
DT and !DT, not just DT only.


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


Re: [PATCH 04/25] MIPS: Netlogic: use ehci-platform driver

2012-10-04 Thread Jonas Gorski
On 3 October 2012 17:02, Florian Fainelli flor...@openwrt.org wrote:
 Signed-off-by: Florian Fainelli flor...@openwrt.org
 ---
  arch/mips/netlogic/xlr/platform.c |6 ++
  1 file changed, 6 insertions(+)

 diff --git a/arch/mips/netlogic/xlr/platform.c 
 b/arch/mips/netlogic/xlr/platform.c
 index 71b44d8..1731dfd 100644
 --- a/arch/mips/netlogic/xlr/platform.c
 +++ b/arch/mips/netlogic/xlr/platform.c
 @@ -15,6 +15,7 @@
  #include linux/serial_8250.h
  #include linux/serial_reg.h
  #include linux/i2c.h
 +#include linux/usb/ehci_pdriver.h

  #include asm/netlogic/haldefs.h
  #include asm/netlogic/xlr/iomap.h
 @@ -123,6 +124,10 @@ static u64 xls_usb_dmamask = ~(u32)0;
 },  \
 }

 +static struct usb_ehci_pdata xls_usb_ehci_pdata = {
 +   .caps_offset= 0,
 +};
 +
  static struct platform_device xls_usb_ehci_device =
  USB_PLATFORM_DEV(ehci-xls, 0, PIC_USB_IRQ);

Don't you also need to change this to ehci-platform?

  static struct platform_device xls_usb_ohci_device_0 =
 @@ -172,6 +177,7 @@ int xls_platform_usb_init(void)
 memres = CPHYSADDR((unsigned long)usb_mmio);
 xls_usb_ehci_device.resource[0].start = memres;
 xls_usb_ehci_device.resource[0].end = memres + 0x400 - 1;
 +   xls_usb_ehci_device.dev.platform_data = xls_usb_ehci_pdata;

 memres += 0x400;
 xls_usb_ohci_device_0.resource[0].start = memres;
 --
 1.7.9.5

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


Re: [PATCH 20/25] MIPS: Netlogic: convert to use OHCI platform driver

2012-10-04 Thread Jonas Gorski
On 3 October 2012 17:03, Florian Fainelli flor...@openwrt.org wrote:
 Signed-off-by: Florian Fainelli flor...@openwrt.org
 ---
  arch/mips/netlogic/xlr/platform.c |5 +
  1 file changed, 5 insertions(+)

 diff --git a/arch/mips/netlogic/xlr/platform.c 
 b/arch/mips/netlogic/xlr/platform.c
 index 320b7ef..755ddcc 100644
 --- a/arch/mips/netlogic/xlr/platform.c
 +++ b/arch/mips/netlogic/xlr/platform.c
 @@ -16,6 +16,7 @@
  #include linux/serial_reg.h
  #include linux/i2c.h
  #include linux/usb/ehci_pdriver.h
 +#include linux/usb/ohci_pdriver.h

  #include asm/netlogic/haldefs.h
  #include asm/netlogic/xlr/iomap.h
 @@ -129,6 +130,8 @@ static struct usb_ehci_pdata xls_usb_ehci_pdata = {
 .need_io_watchdog = 1,
  };

 +static struct usb_ohci_pdata xls_usb_ohci_pdata;
 +
  static struct platform_device xls_usb_ehci_device =
  USB_PLATFORM_DEV(ehci-xls, 0, PIC_USB_IRQ);
  static struct platform_device xls_usb_ohci_device_0 =

And change after the device names of the ohci devices to ohci-platform?

 @@ -183,10 +186,12 @@ int xls_platform_usb_init(void)
 memres += 0x400;
 xls_usb_ohci_device_0.resource[0].start = memres;
 xls_usb_ohci_device_0.resource[0].end = memres + 0x400 - 1;
 +   xls_usb_ohci_device_0.dev.platform_data = xls_usb_ohci_pdata;

 memres += 0x400;
 xls_usb_ohci_device_1.resource[0].start = memres;
 xls_usb_ohci_device_1.resource[0].end = memres + 0x400 - 1;
 +   xls_usb_ohci_device_1.dev.platform_data = xls_usb_ohci_pdata;

 return platform_add_devices(xls_platform_devices,
 ARRAY_SIZE(xls_platform_devices));
 --
 1.7.9.5

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