Re: [PATCH 6/6] usb: dwc3: host: convey the PHYs to xhci

2014-09-16 Thread Kishon Vijay Abraham I 
Hi,

On Monday 15 September 2014 05:36 PM, Heikki Krogerus wrote:
 On Fri, Sep 12, 2014 at 07:41:56PM +0530, Kishon Vijay Abraham I wrote:
 I don't think create lookup should be in host init. If it's dt boot, the
 binding should be in dt data or for other boot modes the bindig should be 
 done
 in the board file. This just seems hacky to me.

 So are you now suggesting that instead of using platform independent
 solution of sharing the PHYs here, you would have us add platform
 specific quirks? That would be totally wrong!

 No. The binding between the controller and the PHY is done in hardware design
 and it would be wrong to create such a binding in drivers/* IMO.
 
 And kernel of course always knows the hardware design when it's being
 booted, wrong!

Exactly my point. By creating the binding in drivers/*, you are directly
telling the driver of the hardware design whereas it should have come from dt
or platform data or .. (ACPI?).
 
 Firstly, don't assume this kind of controllers are always part of some
 SoC or chip set. They could easily be on a PCI card for example.

Agreed. Not convinced the current phy-core could handle it well.
 Secondly, don't assume we could tell all the details about the board
 based on some identifiers. Fox example, at least with our SoCs we
 won't be able to differentiate the boards. And it's not like every
 board using the same SoC uses the same USB2 PHY for example. That kind
 of things are up to the manufacturers.

right. In dt, generally we have different dt files for different boards,
similarly we have different board files for different boards where those
bindings can be created. Again not sure about ACPI.
 
 By default we have to rely on hardware descriptions like DT and ACPI
 tables or being able to runtime detect (ULPI). If those things don't
 work, we live without PHY in this case. We add board specific quirks
 _only_ in case of exceptions, where we simply have no other way. If
 it's possible to avoid them, especially with something as simple as
 this, we avoid them!
 
 And please don't even consider use of board files especially if there
 is an option. They are the one thing that we are meant to avoid if
 possible! No?

 For dt yes, I'm not sure about other modes.

 So in the case of dt boot, I'd prefer giving the binding in dt file than
 anywhere else.
 
 I don't know how dt works, but at least in case of ACPI we still need
 to deliver the PHY to xHCI here, even when dwc3 is provided bindings
 to a PHY(s).

Agreed. But can't we have the bindings for xHCI in ACPI itself?

Thanks
Kishon
--
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 6/6] usb: dwc3: host: convey the PHYs to xhci

2014-09-16 Thread Heikki Krogerus 
On Tue, Sep 16, 2014 at 12:07:00PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Monday 15 September 2014 05:36 PM, Heikki Krogerus wrote:
  On Fri, Sep 12, 2014 at 07:41:56PM +0530, Kishon Vijay Abraham I wrote:
  I don't think create lookup should be in host init. If it's dt boot, the
  binding should be in dt data or for other boot modes the bindig should 
  be done
  in the board file. This just seems hacky to me.
 
  So are you now suggesting that instead of using platform independent
  solution of sharing the PHYs here, you would have us add platform
  specific quirks? That would be totally wrong!
 
  No. The binding between the controller and the PHY is done in hardware 
  design
  and it would be wrong to create such a binding in drivers/* IMO.
  
  And kernel of course always knows the hardware design when it's being
  booted, wrong!
 
 Exactly my point. By creating the binding in drivers/*, you are directly
 telling the driver of the hardware design whereas it should have come from dt
 or platform data or .. (ACPI?).

Man, you are not getting this... We want to avoid the damn platform
data!!! If you are not using devicetree it doesn't mean the same as
something like board files or some other platform specific quirks are
suddenly acceptable.

I'm giving you a way of avoiding those things, basically a way
ignoring the platform completely in this case, but you are saying no
no no, we need to have board files. C'mon!

And I'm pretty sure you misspelled this sentence ..you are directly
telling the driver of the hardware design... I believe you meant to
say Wau! With this we newer have to care about the hardware design
with this driver!.

Note. That and some other things that we can always simply ignore here
with this approach include:
- How were we enumerated (DT, ACPI, PCI...)
- Do some other bindings exist (DT)
- Do we even have PHYs to convey
- Does xHCI have any use for the PHYs
- What is the platform we are running on in general

  Firstly, don't assume this kind of controllers are always part of some
  SoC or chip set. They could easily be on a PCI card for example.
 
 Agreed. Not convinced the current phy-core could handle it well.

If there is ever a PHY that needs OS control in this case, it will
almost certainly be possible to runtime detected somehow like ULPI
PHYs can be.

  Secondly, don't assume we could tell all the details about the board
  based on some identifiers. Fox example, at least with our SoCs we
  won't be able to differentiate the boards. And it's not like every
  board using the same SoC uses the same USB2 PHY for example. That kind
  of things are up to the manufacturers.
 
 right. In dt, generally we have different dt files for different boards,
 similarly we have different board files for different boards where those
 bindings can be created. Again not sure about ACPI.

I'll repeat what I said below - If we don't have description of the
hardware from something like DT or ACPI, and that includes the
bindings, or we can't runtime detect the underlying hardware, by
default it means we live without PHYs. Quirks you add only in case of
exceptions, but I actually think there will never be need for them...

If we are not using ACPI, we are using devicetree and otherwise we can
runtime detect the PHYs one way or the other. And again, if no PHY is
found it simply means the PHYs are autonomous.

So as you can see, it's possible we will newer need any kind of
platform data in order to provide dwc3 the correct PHYs, no matter
what kind of platform we are running on, so it makes no sense of
having them just for dwc3 host. Do you not agree?

  I don't know how dt works, but at least in case of ACPI we still need
  to deliver the PHY to xHCI here, even when dwc3 is provided bindings
  to a PHY(s).
 
 Agreed. But can't we have the bindings for xHCI in ACPI itself?

No. ACPI will never provide separate object for the xHCI that is part
of DWC3. There is nothing the PHYs could be bound to.


-- 
heikki
--
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 6/6] usb: dwc3: host: convey the PHYs to xhci

2014-09-15 Thread Heikki Krogerus 
On Fri, Sep 12, 2014 at 07:41:56PM +0530, Kishon Vijay Abraham I wrote:
  I don't think create lookup should be in host init. If it's dt boot, the
  binding should be in dt data or for other boot modes the bindig should be 
  done
  in the board file. This just seems hacky to me.
  
  So are you now suggesting that instead of using platform independent
  solution of sharing the PHYs here, you would have us add platform
  specific quirks? That would be totally wrong!
 
 No. The binding between the controller and the PHY is done in hardware design
 and it would be wrong to create such a binding in drivers/* IMO.

And kernel of course always knows the hardware design when it's being
booted, wrong!

Firstly, don't assume this kind of controllers are always part of some
SoC or chip set. They could easily be on a PCI card for example.
Secondly, don't assume we could tell all the details about the board
based on some identifiers. Fox example, at least with our SoCs we
won't be able to differentiate the boards. And it's not like every
board using the same SoC uses the same USB2 PHY for example. That kind
of things are up to the manufacturers.

By default we have to rely on hardware descriptions like DT and ACPI
tables or being able to runtime detect (ULPI). If those things don't
work, we live without PHY in this case. We add board specific quirks
_only_ in case of exceptions, where we simply have no other way. If
it's possible to avoid them, especially with something as simple as
this, we avoid them!

  And please don't even consider use of board files especially if there
  is an option. They are the one thing that we are meant to avoid if
  possible! No?
 
 For dt yes, I'm not sure about other modes.
 
 So in the case of dt boot, I'd prefer giving the binding in dt file than
 anywhere else.

I don't know how dt works, but at least in case of ACPI we still need
to deliver the PHY to xHCI here, even when dwc3 is provided bindings
to a PHY(s).


-- 
heikki
--
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 6/6] usb: dwc3: host: convey the PHYs to xhci

2014-09-12 Thread Kishon Vijay Abraham I 
Hi,

On Friday 12 September 2014 07:19 PM, Heikki Krogerus wrote:
 On Thu, Sep 11, 2014 at 08:31:19PM +0530, Kishon Vijay Abraham I wrote:
 @@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
 goto err1;
 }
  
 +   phy_create_lookup(dwc-usb2_generic_phy, usb2-phy,
 + dev_name(xhci-dev));
 +   phy_create_lookup(dwc-usb3_generic_phy, usb3-phy,
 + dev_name(xhci-dev));
 +

 I don't think create lookup should be in host init. If it's dt boot, the
 binding should be in dt data or for other boot modes the bindig should be 
 done
 in the board file. This just seems hacky to me.
 
 So are you now suggesting that instead of using platform independent
 solution of sharing the PHYs here, you would have us add platform
 specific quirks? That would be totally wrong!

No. The binding between the controller and the PHY is done in hardware design
and it would be wrong to create such a binding in drivers/* IMO.
 
 And please don't even consider use of board files especially if there
 is an option. They are the one thing that we are meant to avoid if
 possible! No?

For dt yes, I'm not sure about other modes.

So in the case of dt boot, I'd prefer giving the binding in dt file than
anywhere else.

Thanks
Kishon
--
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 6/6] usb: dwc3: host: convey the PHYs to xhci

2014-09-11 Thread Kishon Vijay Abraham I 
Hi,

On Thursday 21 August 2014 05:03 PM, Heikki Krogerus wrote:
 On some platforms a PHY may need to be handled also in the
 host controller driver. Exynos5420 SoC requires some PHY
 tuning based on the USB speed. This patch delivers dwc3's
 PHYs to the xhci platform device when it's created.
 
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 Tested-by: Vivek Gautam gautam.vi...@samsung.com
 Acked-by: Felipe Balbi ba...@ti.com
 ---
  drivers/usb/dwc3/host.c | 22 --
  1 file changed, 16 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
 index dcb8ca0..12bfd3c 100644
 --- a/drivers/usb/dwc3/host.c
 +++ b/drivers/usb/dwc3/host.c
 @@ -29,8 +29,7 @@ int dwc3_host_init(struct dwc3 *dwc)
   xhci = platform_device_alloc(xhci-hcd, PLATFORM_DEVID_AUTO);
   if (!xhci) {
   dev_err(dwc-dev, couldn't allocate xHCI device\n);
 - ret = -ENOMEM;
 - goto err0;
 + return -ENOMEM;
   }
  
   dma_set_coherent_mask(xhci-dev, dwc-dev-coherent_dma_mask);
 @@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
   goto err1;
   }
  
 + phy_create_lookup(dwc-usb2_generic_phy, usb2-phy,
 +   dev_name(xhci-dev));
 + phy_create_lookup(dwc-usb3_generic_phy, usb3-phy,
 +   dev_name(xhci-dev));
 +

I don't think create lookup should be in host init. If it's dt boot, the
binding should be in dt data or for other boot modes the bindig should be done
in the board file. This just seems hacky to me.

Thanks
Kishon
--
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 6/6] usb: dwc3: host: convey the PHYs to xhci

2014-08-21 Thread Heikki Krogerus 
On some platforms a PHY may need to be handled also in the
host controller driver. Exynos5420 SoC requires some PHY
tuning based on the USB speed. This patch delivers dwc3's
PHYs to the xhci platform device when it's created.

Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Tested-by: Vivek Gautam gautam.vi...@samsung.com
Acked-by: Felipe Balbi ba...@ti.com
---
 drivers/usb/dwc3/host.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index dcb8ca0..12bfd3c 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -29,8 +29,7 @@ int dwc3_host_init(struct dwc3 *dwc)
xhci = platform_device_alloc(xhci-hcd, PLATFORM_DEVID_AUTO);
if (!xhci) {
dev_err(dwc-dev, couldn't allocate xHCI device\n);
-   ret = -ENOMEM;
-   goto err0;
+   return -ENOMEM;
}
 
dma_set_coherent_mask(xhci-dev, dwc-dev-coherent_dma_mask);
@@ -60,22 +59,33 @@ int dwc3_host_init(struct dwc3 *dwc)
goto err1;
}
 
+   phy_create_lookup(dwc-usb2_generic_phy, usb2-phy,
+ dev_name(xhci-dev));
+   phy_create_lookup(dwc-usb3_generic_phy, usb3-phy,
+ dev_name(xhci-dev));
+
ret = platform_device_add(xhci);
if (ret) {
dev_err(dwc-dev, failed to register xHCI device\n);
-   goto err1;
+   goto err2;
}
 
return 0;
-
+err2:
+   phy_remove_lookup(dwc-usb2_generic_phy, usb2-phy,
+ dev_name(xhci-dev));
+   phy_remove_lookup(dwc-usb3_generic_phy, usb3-phy,
+ dev_name(xhci-dev));
 err1:
platform_device_put(xhci);
-
-err0:
return ret;
 }
 
 void dwc3_host_exit(struct dwc3 *dwc)
 {
+   phy_remove_lookup(dwc-usb2_generic_phy, usb2-phy,
+ dev_name(dwc-xhci-dev));
+   phy_remove_lookup(dwc-usb3_generic_phy, usb3-phy,
+ dev_name(dwc-xhci-dev));
platform_device_unregister(dwc-xhci);
 }
-- 
2.1.0

--
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