Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On 01/21/2014 12:11 PM, Kishon Vijay Abraham I wrote:
> Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs),
> do not return from probe if the USB PHY library returns -ENODEV as that
> indicates the platform does not have PHY.
>
> Signed-off-by: Kishon Vijay Abraham I
Reviewed-by: Roger Quadros
cheers,
-roger
> ---
> drivers/usb/dwc3/core.c | 34 ++
> 1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index a49217a..e009d4e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -411,32 +411,26 @@ static int dwc3_probe(struct platform_device *pdev)
>
> if (IS_ERR(dwc->usb2_phy)) {
> ret = PTR_ERR(dwc->usb2_phy);
> -
> - /*
> - * if -ENXIO is returned, it means PHY layer wasn't
> - * enabled, so it makes no sense to return -EPROBE_DEFER
> - * in that case, since no PHY driver will ever probe.
> - */
> - if (ret == -ENXIO)
> + if (ret == -ENXIO || ret == -ENODEV) {
> + dwc->usb2_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> return ret;
> -
> - dev_err(dev, "no usb2 phy configured\n");
> - return -EPROBE_DEFER;
> + } else {
> + dev_err(dev, "no usb2 phy configured\n");
> + return ret;
> + }
> }
>
> if (IS_ERR(dwc->usb3_phy)) {
> ret = PTR_ERR(dwc->usb3_phy);
> -
> - /*
> - * if -ENXIO is returned, it means PHY layer wasn't
> - * enabled, so it makes no sense to return -EPROBE_DEFER
> - * in that case, since no PHY driver will ever probe.
> - */
> - if (ret == -ENXIO)
> + if (ret == -ENXIO || ret == -ENODEV) {
> + dwc->usb3_phy = NULL;
> + } else if (ret == -EPROBE_DEFER) {
> return ret;
> -
> - dev_err(dev, "no usb3 phy configured\n");
> - return -EPROBE_DEFER;
> + } else {
> + dev_err(dev, "no usb3 phy configured\n");
> + return ret;
> + }
> }
>
> dwc->xhci_resources[0].start = res->start;
>
--
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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Tue, Jan 21, 2014 at 03:41:38PM +0530, Kishon Vijay Abraham I wrote: > Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), > do not return from probe if the USB PHY library returns -ENODEV as that this isn't correct, they all have PHYs, some of them might not be controllable. > indicates the platform does not have PHY. not really, that indicates the current platform tried to grab a PHY and the PHY doesn't exist. If there's anybody with a non-controllable PHY and someone gives me a really good reason for not using the generic no-op PHY, then we should add a flag and we could: if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) dwc3_grab_phys(dwc); But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Your commit log really needs quite some extra work. What are you trying to achieve ? Is this related to AM437x which doesn't have a USB3 PHY due to the way the HW was wired up ? If so, please mention it. Explain how AM437x HW was wired up, why it lacks a USB3 PHY and why we should support it the way you want. That sort of detail needs to be clear in the commit log, specially considering the peculiar nature of AM43xx which uses a USB3 IP in USB2-only mode, that needs to be documented in the commit log :-) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Tuesday 21 January 2014 08:17 PM, Felipe Balbi wrote: > On Tue, Jan 21, 2014 at 03:41:38PM +0530, Kishon Vijay Abraham I wrote: >> Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), >> do not return from probe if the USB PHY library returns -ENODEV as that > > this isn't correct, they all have PHYs, some of them might not be > controllable. right, but we use USB PHY library only for controllable PHYs (apart from using nop). > >> indicates the platform does not have PHY. > > not really, that indicates the current platform tried to grab a PHY and > the PHY doesn't exist. If there's anybody with a non-controllable PHY > and someone gives me a really good reason for not using the generic > no-op PHY, then we should add a flag and we could: > > if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) > dwc3_grab_phys(dwc); > > But I really want to see the argument against using no-op. As far as I > could see, everybody needs a PHY driver one way or another, some > platforms just haven't sent any PHY driver upstream and have their own > hacked up solution to avoid using the PHY layer. I was trying to address Heikki concerns in my previous version [1] where I used quirks to identify if the platform does not have PHY. [1] -> https://lkml.org/lkml/2013/12/5/32 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Felipe, On Tue, Jan 21, 2014 at 08:47:25AM -0600, Felipe Balbi wrote: > On Tue, Jan 21, 2014 at 03:41:38PM +0530, Kishon Vijay Abraham I wrote: > > Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), > > do not return from probe if the USB PHY library returns -ENODEV as that > > this isn't correct, they all have PHYs, some of them might not be > controllable. > > > indicates the platform does not have PHY. > > not really, that indicates the current platform tried to grab a PHY and > the PHY doesn't exist. If there's anybody with a non-controllable PHY > and someone gives me a really good reason for not using the generic > no-op PHY, then we should add a flag and we could: > > if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) > dwc3_grab_phys(dwc); Why would you need to know if the PHY drivers are needed or not explicitly in your controller driver? > But I really want to see the argument against using no-op. As far as I > could see, everybody needs a PHY driver one way or another, some > platforms just haven't sent any PHY driver upstream and have their own > hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. Are we talking about the old USB PHY library or the new PHY framework with the no-op PHY driver? Well, in any case, I don't understand what is the purpose of the no-op PHY driver. What are you drying to achieve with that? Thanks, -- 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Mon, Jan 27, 2014 at 05:08:55PM +0200, Heikki Krogerus wrote: > > > Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), > > > do not return from probe if the USB PHY library returns -ENODEV as that > > > > this isn't correct, they all have PHYs, some of them might not be > > controllable. > > > > > indicates the platform does not have PHY. > > > > not really, that indicates the current platform tried to grab a PHY and > > the PHY doesn't exist. If there's anybody with a non-controllable PHY > > and someone gives me a really good reason for not using the generic > > no-op PHY, then we should add a flag and we could: > > > > if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) > > dwc3_grab_phys(dwc); > > Why would you need to know if the PHY drivers are needed or not > explicitly in your controller driver? because, one way or another, they all do need it. Except for quirky ones like AM437x where a USB3 IP was hardwired into USB2-only mode, so it really lacks a USB3 PHY. > > But I really want to see the argument against using no-op. As far as I > > could see, everybody needs a PHY driver one way or another, some > > platforms just haven't sent any PHY driver upstream and have their own > > hacked up solution to avoid using the PHY layer. > > Not true in our case. Platforms using Intel's SoCs and chip sets may > or may not have controllable USB PHY. Quite often they don't. The > Baytrails have usually ULPI PHY for USB2, but that does not mean they > provide any vendor specific functions or any need for a driver in any > case. that's different from what I heard. > Are we talking about the old USB PHY library or the new PHY framework > with the no-op PHY driver? > > Well, in any case, I don't understand what is the purpose of the no-op > PHY driver. What are you drying to achieve with that? I'm trying to avoid supporting 500 different combinations for a single driver. We already support old USB PHY layer and generic PHY layer, now they both need to be made optional. The old USB PHY layer also provides a no-op, now called phy-generic, which is actually pretty useful. On top of all that, I'm sure you'll subscribe to the idea of preventing dwc3 from becoming another MUSB which supports several different configurations and none work correctly. I much prefer to take baby steps which are well thought-out and very well exercised, so I'll be very pedantic about proof of testing. An invasive change such as $subject needs to be very well-tested in different architectures with different Kconfig choices before I'd feel comfortable applying it. Also, the assumptions made in $subject are just plain wrong, which gets me really iffy about applying it or not. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > > Why would you need to know if the PHY drivers are needed or not > > explicitly in your controller driver? > > because, one way or another, they all do need it. Except for quirky ones > like AM437x where a USB3 IP was hardwired into USB2-only mode, so it > really lacks a USB3 PHY. The Baytrail board I deal with has completely autonomous PHYs. But my question is, why would you need to care about the PHYs in your controller driver? Why can't you leave the responsibility of them to the upper layers and trust what they tell you? If there is no USB3 PHY for dwc3 then, the driver gets something like -ENODEV and just continues. There is no need to know about the details. For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. > > > But I really want to see the argument against using no-op. As far as I > > > could see, everybody needs a PHY driver one way or another, some > > > platforms just haven't sent any PHY driver upstream and have their own > > > hacked up solution to avoid using the PHY layer. > > > > Not true in our case. Platforms using Intel's SoCs and chip sets may > > or may not have controllable USB PHY. Quite often they don't. The > > Baytrails have usually ULPI PHY for USB2, but that does not mean they > > provide any vendor specific functions or any need for a driver in any > > case. > > that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. The problem is that we just don't always know all the details about the platform. If the PHY is ULPI PHY, we can do runtime detection, but we can't even rely on always having ULPI. > > Are we talking about the old USB PHY library or the new PHY framework > > with the no-op PHY driver? > > > > Well, in any case, I don't understand what is the purpose of the no-op > > PHY driver. What are you drying to achieve with that? > > I'm trying to avoid supporting 500 different combinations for a single > driver. We already support old USB PHY layer and generic PHY layer, now > they both need to be made optional. This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. But I still don't understand what is the benefit in the no-op? You really need to explain this. What you have now in dwc3 is expectations regarding the PHY, which put a lot of pressure on upper layers to satisfy the driver. The no-op is just an extra thing that you have to provide when you fail to determine if the system requires a PHY driver or not, or if you know that it doesn't, plus an additional check for the case where the PHY lib/framework is not enabled. I don't see the value in it. > The old USB PHY layer also provides > a no-op, now called phy-generic, which is actually pretty useful. > > On top of all that, I'm sure you'll subscribe to the idea of preventing > dwc3 from becoming another MUSB which supports several different > configurations and none work correctly. I much prefer to take baby steps > which are well thought-out and very well exercised, so I'll be very > pedantic about proof of testing. I think our goals are the same. I just want to also minimize the need for any platform specific extra work from the upper layers regarding the PHYs. Thanks, -- 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: > On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > > > Why would you need to know if the PHY drivers are needed or not > > > explicitly in your controller driver? > > > > because, one way or another, they all do need it. Except for quirky ones > > like AM437x where a USB3 IP was hardwired into USB2-only mode, so it > > really lacks a USB3 PHY. > > The Baytrail board I deal with has completely autonomous PHYs. But my > question is, why would you need to care about the PHYs in your > controller driver? Why can't you leave the responsibility of them to > the upper layers and trust what they tell you? > > If there is no USB3 PHY for dwc3 then, the driver gets something like > -ENODEV and just continues. There is no need to know about the > details. it's a little more complicated than that. We could get -EPROBE_DEFER meaning we should try probing at a later time because the PHY isn't available yet. But sure, if we can very easily differentiate between those two conditions in a way that error report from PHY layer (whichever it is), then we can certainly do that. > For the controller drivers the PHYs are just a resource like any > other. The controller drivers can't have any responsibility of > them. They should not care if PHY drivers are available for them or > not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. > > > > But I really want to see the argument against using no-op. As far as I > > > > could see, everybody needs a PHY driver one way or another, some > > > > platforms just haven't sent any PHY driver upstream and have their own > > > > hacked up solution to avoid using the PHY layer. > > > > > > Not true in our case. Platforms using Intel's SoCs and chip sets may > > > or may not have controllable USB PHY. Quite often they don't. The > > > Baytrails have usually ULPI PHY for USB2, but that does not mean they > > > provide any vendor specific functions or any need for a driver in any > > > case. > > > > that's different from what I heard. > > I don't know where you got that impression, but it's not true. The > Baytrail SoCs for example don't have internal USB PHYs, which means > the manufacturers using it can select what they want. So we have > boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. > The problem is that we just don't always know all the details about > the platform. If the PHY is ULPI PHY, we can do runtime detection, but > we can't even rely on always having ULPI. right, we've had that before on n900 ;-) > > > Are we talking about the old USB PHY library or the new PHY framework > > > with the no-op PHY driver? > > > > > > Well, in any case, I don't understand what is the purpose of the no-op > > > PHY driver. What are you drying to achieve with that? > > > > I'm trying to avoid supporting 500 different combinations for a single > > driver. We already support old USB PHY layer and generic PHY layer, now > > they both need to be made optional. > > This is really good to get. We have some projects where we are dealing > with more embedded environments, like IVI, where the kernel should be > stripped of everything useless. Since the PHYs are autonomous, we > should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) > But I still don't understand what is the benefit in the no-op? You > really need to explain this. What you have now in dwc3 is expectations > regarding the PHY, which put a lot of pressure on upper layers to > satisfy the driver. The no-op is just an extra thing that you have to > provide when you fail to determine if the system requires a PHY driver > or not, or if you know that it doesn't, plus an additional check for > the case where the PHY lib/framework is not enabled. I don't see the > value in it. it "solves" the difficulty above. If everybody has to pro
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: > On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: > > On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > > For the controller drivers the PHYs are just a resource like any > > other. The controller drivers can't have any responsibility of > > them. They should not care if PHY drivers are available for them or > > not, or even if the PHY framework is available or not. > > huh? If memory isn't available you don't continue probing, right ? If > your IORESOURCE_MEM is missing, you also don't continue probing, if your > IRQ line is missing, you bail too. Those are also nothing but resources > to the driver, what you're asking here is to treat PHY as a _different_ > resource; which might be fine, but we need to make sure we don't > continue probing when a PHY is missing in a platform that certainly > needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. > > > > > But I really want to see the argument against using no-op. As far as I > > > > > could see, everybody needs a PHY driver one way or another, some > > > > > platforms just haven't sent any PHY driver upstream and have their own > > > > > hacked up solution to avoid using the PHY layer. > > > > > > > > Not true in our case. Platforms using Intel's SoCs and chip sets may > > > > or may not have controllable USB PHY. Quite often they don't. The > > > > Baytrails have usually ULPI PHY for USB2, but that does not mean they > > > > provide any vendor specific functions or any need for a driver in any > > > > case. > > > > > > that's different from what I heard. > > > > I don't know where you got that impression, but it's not true. The > > Baytrail SoCs for example don't have internal USB PHYs, which means > > the manufacturers using it can select what they want. So we have > > boards where PHY driver(s) is needed and boards where it isn't. > > alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? > You have an external PIPE3 interface ? That's quite an achievement, > kudos to your HW designers. Getting timing closure on PIPE3 is a > difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. > > This is really good to get. We have some projects where we are dealing > > with more embedded environments, like IVI, where the kernel should be > > stripped of everything useless. Since the PHYs are autonomous, we > > should be able to disable the PHY libraries/frameworks. > > hmmm, in that case it's a lot easier to treat. We can use > ERR_PTR(-ENXIO) as an indication that the framework is disabled, or > something like that. > > The difficult is really reliably supporting e.g. OMAP5 (which won't work > without a PHY) and your BayTrail with autonomous PHYs. What can we use > as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? > I mean, I need to know that a particular platform depends on a PHY > driver before I decide to return -EPROBE_DEFER or just assume the PHY > isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. > > I think our goals are the same. I just want to also minimize the need > > for any platform specific extra work from the upper layers regarding > > the PHYs. > > I'll agree to that, but let's not apply patches until we're 100% sure > there will be no regressions. Looking at $subject, I don't think it > satisfies that condition ;-) Agreed. Let's think this through carefully. Cheers, -- 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: > Hi, > > On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: >> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: >>> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: >>> For the controller drivers the PHYs are just a resource like any >>> other. The controller drivers can't have any responsibility of >>> them. They should not care if PHY drivers are available for them or >>> not, or even if the PHY framework is available or not. >> >> huh? If memory isn't available you don't continue probing, right ? If >> your IORESOURCE_MEM is missing, you also don't continue probing, if your >> IRQ line is missing, you bail too. Those are also nothing but resources >> to the driver, what you're asking here is to treat PHY as a _different_ >> resource; which might be fine, but we need to make sure we don't >> continue probing when a PHY is missing in a platform that certainly >> needs a PHY. > > Yes, true. In my head I was comparing the PHY only to resources like > gpios, clocks, dma channels, etc. that are often optional to the > drivers. > >> But I really want to see the argument against using no-op. As far as I >> could see, everybody needs a PHY driver one way or another, some >> platforms just haven't sent any PHY driver upstream and have their own >> hacked up solution to avoid using the PHY layer. > > Not true in our case. Platforms using Intel's SoCs and chip sets may > or may not have controllable USB PHY. Quite often they don't. The > Baytrails have usually ULPI PHY for USB2, but that does not mean they > provide any vendor specific functions or any need for a driver in any > case. that's different from what I heard. >>> >>> I don't know where you got that impression, but it's not true. The >>> Baytrail SoCs for example don't have internal USB PHYs, which means >>> the manufacturers using it can select what they want. So we have >>> boards where PHY driver(s) is needed and boards where it isn't. >> >> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? >> You have an external PIPE3 interface ? That's quite an achievement, >> kudos to your HW designers. Getting timing closure on PIPE3 is a >> difficult task. > > No, only the USB2 PHY is external. I'm giving you wrong information, > I'm sorry about that. Need to concentrate on what I'm writing. > > > >>> This is really good to get. We have some projects where we are dealing >>> with more embedded environments, like IVI, where the kernel should be >>> stripped of everything useless. Since the PHYs are autonomous, we >>> should be able to disable the PHY libraries/frameworks. >> >> hmmm, in that case it's a lot easier to treat. We can use >> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or >> something like that. >> >> The difficult is really reliably supporting e.g. OMAP5 (which won't work >> without a PHY) and your BayTrail with autonomous PHYs. What can we use >> as an indication ? > > OMAP has it's own glue driver, so shouldn't it depend on the PHY > layer? right, but the PHY is connected to the dwc3 core and not to the glue. > >> I mean, I need to know that a particular platform depends on a PHY >> driver before I decide to return -EPROBE_DEFER or just assume the PHY >> isn't needed ;-) > > I don't think dwc3 (core) should care about that. The PHY layer needs > to tell us that. If the PHY driver that the platform depends is not > available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up > returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. 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 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: > On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: >> Hi, >> >> On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: >>> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. >>> >>> huh? If memory isn't available you don't continue probing, right ? If >>> your IORESOURCE_MEM is missing, you also don't continue probing, if your >>> IRQ line is missing, you bail too. Those are also nothing but resources >>> to the driver, what you're asking here is to treat PHY as a _different_ >>> resource; which might be fine, but we need to make sure we don't >>> continue probing when a PHY is missing in a platform that certainly >>> needs a PHY. >> >> Yes, true. In my head I was comparing the PHY only to resources like >> gpios, clocks, dma channels, etc. that are often optional to the >> drivers. >> >>> But I really want to see the argument against using no-op. As far as I >>> could see, everybody needs a PHY driver one way or another, some >>> platforms just haven't sent any PHY driver upstream and have their own >>> hacked up solution to avoid using the PHY layer. >> >> Not true in our case. Platforms using Intel's SoCs and chip sets may >> or may not have controllable USB PHY. Quite often they don't. The >> Baytrails have usually ULPI PHY for USB2, but that does not mean they >> provide any vendor specific functions or any need for a driver in any >> case. > > that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. >>> >>> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? >>> You have an external PIPE3 interface ? That's quite an achievement, >>> kudos to your HW designers. Getting timing closure on PIPE3 is a >>> difficult task. >> >> No, only the USB2 PHY is external. I'm giving you wrong information, >> I'm sorry about that. Need to concentrate on what I'm writing. >> >> >> This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. >>> >>> hmmm, in that case it's a lot easier to treat. We can use >>> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or >>> something like that. >>> >>> The difficult is really reliably supporting e.g. OMAP5 (which won't work >>> without a PHY) and your BayTrail with autonomous PHYs. What can we use >>> as an indication ? >> >> OMAP has it's own glue driver, so shouldn't it depend on the PHY >> layer? > > right, but the PHY is connected to the dwc3 core and not to the glue. >> >>> I mean, I need to know that a particular platform depends on a PHY >>> driver before I decide to return -EPROBE_DEFER or just assume the PHY >>> isn't needed ;-) >> >> I don't think dwc3 (core) should care about that. The PHY layer needs >> to tell us that. If the PHY driver that the platform depends is not >> available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up >> returning -EPROBE_DEFER. > > I don't think the PHY layer can 'reliably' tell if PHY driver is available or > not. Consider when the phy_provider_register fails, there is no way to know if > PHY driver is available or not. There are a few cases where PHY layer returns > -EPROBE_DEFER but none of them can tell for sure that PHY driver is either > available and failed or not available at all. It would be best for us to leave > that to the platforms if we want to be sure if the platform needs a PHY or > not. > Just to summarize this thread on what we need 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or not. It should be as generic as possible. 2) dwc3 core should continue probe even if PHY layer is not enabled, as not all platforms need it. 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) 4) dwc3 core should error out on any error condition if PHY device is available and caused some error, e.g. init error. 5) dwc3 core should return EPROBE_DEFER if PHY device is available but device driver is not yet loaded. 6) platform glue should do the necessary sanity checks for availability of all resources like PHY device, PHY layer, etc, before populating the dwc3 device. e.g. in
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: > Hi, > > On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: >> On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: >>> Hi, >>> >>> On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: > On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > For the controller drivers the PHYs are just a resource like any > other. The controller drivers can't have any responsibility of > them. They should not care if PHY drivers are available for them or > not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. >>> >>> Yes, true. In my head I was comparing the PHY only to resources like >>> gpios, clocks, dma channels, etc. that are often optional to the >>> drivers. >>> But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. >>> >>> Not true in our case. Platforms using Intel's SoCs and chip sets may >>> or may not have controllable USB PHY. Quite often they don't. The >>> Baytrails have usually ULPI PHY for USB2, but that does not mean they >>> provide any vendor specific functions or any need for a driver in any >>> case. >> >> that's different from what I heard. > > I don't know where you got that impression, but it's not true. The > Baytrail SoCs for example don't have internal USB PHYs, which means > the manufacturers using it can select what they want. So we have > boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. >>> >>> No, only the USB2 PHY is external. I'm giving you wrong information, >>> I'm sorry about that. Need to concentrate on what I'm writing. >>> >>> >>> > This is really good to get. We have some projects where we are dealing > with more embedded environments, like IVI, where the kernel should be > stripped of everything useless. Since the PHYs are autonomous, we > should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? >>> >>> OMAP has it's own glue driver, so shouldn't it depend on the PHY >>> layer? >> >> right, but the PHY is connected to the dwc3 core and not to the glue. >>> I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) >>> >>> I don't think dwc3 (core) should care about that. The PHY layer needs >>> to tell us that. If the PHY driver that the platform depends is not >>> available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up >>> returning -EPROBE_DEFER. >> >> I don't think the PHY layer can 'reliably' tell if PHY driver is available or >> not. Consider when the phy_provider_register fails, there is no way to know >> if >> PHY driver is available or not. There are a few cases where PHY layer returns >> -EPROBE_DEFER but none of them can tell for sure that PHY driver is either >> available and failed or not available at all. It would be best for us to >> leave >> that to the platforms if we want to be sure if the platform needs a PHY or >> not. >> > > Just to summarize this thread on what we need Thanks for summarizing. > > 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed or > not. > It should be as generic as possible. > > 2) dwc3 core should continue probe even if PHY layer is not enabled, as not > all platforms need it. > > 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) > > 4) dwc3 core should error out on any error condition if PHY device is > available and caused some error, > e.g. init error. > > 5) dwc3 core should return EPROBE_DEFER if PH
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: >> Hi, >> >> On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: >>> On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: > On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: >> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: >> For the controller drivers the PHYs are just a resource like any >> other. The controller drivers can't have any responsibility of >> them. They should not care if PHY drivers are available for them or >> not, or even if the PHY framework is available or not. > > huh? If memory isn't available you don't continue probing, right ? If > your IORESOURCE_MEM is missing, you also don't continue probing, if your > IRQ line is missing, you bail too. Those are also nothing but resources > to the driver, what you're asking here is to treat PHY as a _different_ > resource; which might be fine, but we need to make sure we don't > continue probing when a PHY is missing in a platform that certainly > needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. > But I really want to see the argument against using no-op. As far as I > could see, everybody needs a PHY driver one way or another, some > platforms just haven't sent any PHY driver upstream and have their own > hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. >>> >>> that's different from what I heard. >> >> I don't know where you got that impression, but it's not true. The >> Baytrail SoCs for example don't have internal USB PHYs, which means >> the manufacturers using it can select what they want. So we have >> boards where PHY driver(s) is needed and boards where it isn't. > > alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? > You have an external PIPE3 interface ? That's quite an achievement, > kudos to your HW designers. Getting timing closure on PIPE3 is a > difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. >> This is really good to get. We have some projects where we are dealing >> with more embedded environments, like IVI, where the kernel should be >> stripped of everything useless. Since the PHYs are autonomous, we >> should be able to disable the PHY libraries/frameworks. > > hmmm, in that case it's a lot easier to treat. We can use > ERR_PTR(-ENXIO) as an indication that the framework is disabled, or > something like that. > > The difficult is really reliably supporting e.g. OMAP5 (which won't work > without a PHY) and your BayTrail with autonomous PHYs. What can we use > as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? >>> >>> right, but the PHY is connected to the dwc3 core and not to the glue. > I mean, I need to know that a particular platform depends on a PHY > driver before I decide to return -EPROBE_DEFER or just assume the PHY > isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. >>> >>> I don't think the PHY layer can 'reliably' tell if PHY driver is available >>> or >>> not. Consider when the phy_provider_register fails, there is no way to know >>> if >>> PHY driver is available or not. There are a few cases where PHY layer >>> returns >>> -EPROBE_DEFER but none of them can tell for sure that PHY driver is either >>> available and failed or not available at all. It would be best for us to >>> leave >>> that to the platforms if we want to be sure if the platform needs a PHY or >>> not. >>> >> >> Just to summarize this thread on what we need > > Thanks for summarizing. >> >> 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed >> or not. >> It should be as generic as possible. >> >> 2) dwc3 core should continue probe even if PHY layer is not enabled, as not >> all platforms need it. >> >> 3) dwc3 core should continue probe if PHY device is not available. (-ENODEV?) >> >> 4)
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Roger, On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: > On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: >> Hi Roger, >> >> On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: >>> Hi, >>> >>> On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: > Hi, > > On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: >> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: >>> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: >>> For the controller drivers the PHYs are just a resource like any >>> other. The controller drivers can't have any responsibility of >>> them. They should not care if PHY drivers are available for them or >>> not, or even if the PHY framework is available or not. >> >> huh? If memory isn't available you don't continue probing, right ? If >> your IORESOURCE_MEM is missing, you also don't continue probing, if your >> IRQ line is missing, you bail too. Those are also nothing but resources >> to the driver, what you're asking here is to treat PHY as a _different_ >> resource; which might be fine, but we need to make sure we don't >> continue probing when a PHY is missing in a platform that certainly >> needs a PHY. > > Yes, true. In my head I was comparing the PHY only to resources like > gpios, clocks, dma channels, etc. that are often optional to the > drivers. > >> But I really want to see the argument against using no-op. As far as >> I >> could see, everybody needs a PHY driver one way or another, some >> platforms just haven't sent any PHY driver upstream and have their >> own >> hacked up solution to avoid using the PHY layer. > > Not true in our case. Platforms using Intel's SoCs and chip sets may > or may not have controllable USB PHY. Quite often they don't. The > Baytrails have usually ULPI PHY for USB2, but that does not mean they > provide any vendor specific functions or any need for a driver in any > case. that's different from what I heard. >>> >>> I don't know where you got that impression, but it's not true. The >>> Baytrail SoCs for example don't have internal USB PHYs, which means >>> the manufacturers using it can select what they want. So we have >>> boards where PHY driver(s) is needed and boards where it isn't. >> >> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? >> You have an external PIPE3 interface ? That's quite an achievement, >> kudos to your HW designers. Getting timing closure on PIPE3 is a >> difficult task. > > No, only the USB2 PHY is external. I'm giving you wrong information, > I'm sorry about that. Need to concentrate on what I'm writing. > > > >>> This is really good to get. We have some projects where we are dealing >>> with more embedded environments, like IVI, where the kernel should be >>> stripped of everything useless. Since the PHYs are autonomous, we >>> should be able to disable the PHY libraries/frameworks. >> >> hmmm, in that case it's a lot easier to treat. We can use >> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or >> something like that. >> >> The difficult is really reliably supporting e.g. OMAP5 (which won't work >> without a PHY) and your BayTrail with autonomous PHYs. What can we use >> as an indication ? > > OMAP has it's own glue driver, so shouldn't it depend on the PHY > layer? right, but the PHY is connected to the dwc3 core and not to the glue. > >> I mean, I need to know that a particular platform depends on a PHY >> driver before I decide to return -EPROBE_DEFER or just assume the PHY >> isn't needed ;-) > > I don't think dwc3 (core) should care about that. The PHY layer needs > to tell us that. If the PHY driver that the platform depends is not > available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up > returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. >>> >>> Just to summarize this thread on what we need >> >> Thanks for summarizing. >>> >>> 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed >>> or not. >>> It should be as generic as possible. >>> >
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: > On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: >> Hi Roger, >> >> On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: >>> Hi, >>> >>> On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: > Hi, > > On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: >> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: >>> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: >>> For the controller drivers the PHYs are just a resource like any >>> other. The controller drivers can't have any responsibility of >>> them. They should not care if PHY drivers are available for them or >>> not, or even if the PHY framework is available or not. >> >> huh? If memory isn't available you don't continue probing, right ? If >> your IORESOURCE_MEM is missing, you also don't continue probing, if your >> IRQ line is missing, you bail too. Those are also nothing but resources >> to the driver, what you're asking here is to treat PHY as a _different_ >> resource; which might be fine, but we need to make sure we don't >> continue probing when a PHY is missing in a platform that certainly >> needs a PHY. > > Yes, true. In my head I was comparing the PHY only to resources like > gpios, clocks, dma channels, etc. that are often optional to the > drivers. > >> But I really want to see the argument against using no-op. As far as >> I >> could see, everybody needs a PHY driver one way or another, some >> platforms just haven't sent any PHY driver upstream and have their >> own >> hacked up solution to avoid using the PHY layer. > > Not true in our case. Platforms using Intel's SoCs and chip sets may > or may not have controllable USB PHY. Quite often they don't. The > Baytrails have usually ULPI PHY for USB2, but that does not mean they > provide any vendor specific functions or any need for a driver in any > case. that's different from what I heard. >>> >>> I don't know where you got that impression, but it's not true. The >>> Baytrail SoCs for example don't have internal USB PHYs, which means >>> the manufacturers using it can select what they want. So we have >>> boards where PHY driver(s) is needed and boards where it isn't. >> >> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? >> You have an external PIPE3 interface ? That's quite an achievement, >> kudos to your HW designers. Getting timing closure on PIPE3 is a >> difficult task. > > No, only the USB2 PHY is external. I'm giving you wrong information, > I'm sorry about that. Need to concentrate on what I'm writing. > > > >>> This is really good to get. We have some projects where we are dealing >>> with more embedded environments, like IVI, where the kernel should be >>> stripped of everything useless. Since the PHYs are autonomous, we >>> should be able to disable the PHY libraries/frameworks. >> >> hmmm, in that case it's a lot easier to treat. We can use >> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or >> something like that. >> >> The difficult is really reliably supporting e.g. OMAP5 (which won't work >> without a PHY) and your BayTrail with autonomous PHYs. What can we use >> as an indication ? > > OMAP has it's own glue driver, so shouldn't it depend on the PHY > layer? right, but the PHY is connected to the dwc3 core and not to the glue. > >> I mean, I need to know that a particular platform depends on a PHY >> driver before I decide to return -EPROBE_DEFER or just assume the PHY >> isn't needed ;-) > > I don't think dwc3 (core) should care about that. The PHY layer needs > to tell us that. If the PHY driver that the platform depends is not > available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up > returning -EPROBE_DEFER. I don't think the PHY layer can 'reliably' tell if PHY driver is available or not. Consider when the phy_provider_register fails, there is no way to know if PHY driver is available or not. There are a few cases where PHY layer returns -EPROBE_DEFER but none of them can tell for sure that PHY driver is either available and failed or not available at all. It would be best for us to leave that to the platforms if we want to be sure if the platform needs a PHY or not. >>> >>> Just to summarize this thread on what we need >> >> Thanks for summarizing. >>> >>> 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY needed >>> or not. >>> It should be as generic as possible. I think this co
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On 02/24/2014 11:51 AM, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: >> On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: >>> Hi Roger, >>> >>> On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: Hi, On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: > On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: >> Hi, >> >> On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: >>> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. >>> >>> huh? If memory isn't available you don't continue probing, right ? If >>> your IORESOURCE_MEM is missing, you also don't continue probing, if your >>> IRQ line is missing, you bail too. Those are also nothing but resources >>> to the driver, what you're asking here is to treat PHY as a _different_ >>> resource; which might be fine, but we need to make sure we don't >>> continue probing when a PHY is missing in a platform that certainly >>> needs a PHY. >> >> Yes, true. In my head I was comparing the PHY only to resources like >> gpios, clocks, dma channels, etc. that are often optional to the >> drivers. >> >>> But I really want to see the argument against using no-op. As far >>> as I >>> could see, everybody needs a PHY driver one way or another, some >>> platforms just haven't sent any PHY driver upstream and have their >>> own >>> hacked up solution to avoid using the PHY layer. >> >> Not true in our case. Platforms using Intel's SoCs and chip sets may >> or may not have controllable USB PHY. Quite often they don't. The >> Baytrails have usually ULPI PHY for USB2, but that does not mean they >> provide any vendor specific functions or any need for a driver in any >> case. > > that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. >>> >>> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? >>> You have an external PIPE3 interface ? That's quite an achievement, >>> kudos to your HW designers. Getting timing closure on PIPE3 is a >>> difficult task. >> >> No, only the USB2 PHY is external. I'm giving you wrong information, >> I'm sorry about that. Need to concentrate on what I'm writing. >> >> >> This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. >>> >>> hmmm, in that case it's a lot easier to treat. We can use >>> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or >>> something like that. >>> >>> The difficult is really reliably supporting e.g. OMAP5 (which won't work >>> without a PHY) and your BayTrail with autonomous PHYs. What can we use >>> as an indication ? >> >> OMAP has it's own glue driver, so shouldn't it depend on the PHY >> layer? > > right, but the PHY is connected to the dwc3 core and not to the glue. >> >>> I mean, I need to know that a particular platform depends on a PHY >>> driver before I decide to return -EPROBE_DEFER or just assume the PHY >>> isn't needed ;-) >> >> I don't think dwc3 (core) should care about that. The PHY layer needs >> to tell us that. If the PHY driver that the platform depends is not >> available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up >> returning -EPROBE_DEFER. > > I don't think the PHY layer can 'reliably' tell if PHY driver is > available or > not. Consider when the phy_provider_register fails, there is no way to > know if > PHY driver is available or not. There are a few cases where PHY layer > returns > -EPROBE_DEFER but none of them can tell for sure that PHY driver is either > available and failed or not available at all. It would be best for us to > leave > that to the platforms if we want to be sure if the platform needs a PHY > or not. > Just to summarize this thread on what we need >>> >>> Tha
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Monday 24 February 2014 04:35 PM, Roger Quadros wrote: > On 02/24/2014 11:51 AM, Kishon Vijay Abraham I wrote: >> Hi Roger, >> >> On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: >>> On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: Hi Roger, On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: > Hi, > > On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: >> On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: >>> Hi, >>> >>> On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: > On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > For the controller drivers the PHYs are just a resource like any > other. The controller drivers can't have any responsibility of > them. They should not care if PHY drivers are available for them or > not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. >>> >>> Yes, true. In my head I was comparing the PHY only to resources like >>> gpios, clocks, dma channels, etc. that are often optional to the >>> drivers. >>> But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. >>> >>> Not true in our case. Platforms using Intel's SoCs and chip sets may >>> or may not have controllable USB PHY. Quite often they don't. The >>> Baytrails have usually ULPI PHY for USB2, but that does not mean >>> they >>> provide any vendor specific functions or any need for a driver in >>> any >>> case. >> >> that's different from what I heard. > > I don't know where you got that impression, but it's not true. The > Baytrail SoCs for example don't have internal USB PHYs, which means > the manufacturers using it can select what they want. So we have > boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. >>> >>> No, only the USB2 PHY is external. I'm giving you wrong information, >>> I'm sorry about that. Need to concentrate on what I'm writing. >>> >>> >>> > This is really good to get. We have some projects where we are dealing > with more embedded environments, like IVI, where the kernel should be > stripped of everything useless. Since the PHYs are autonomous, we > should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? >>> >>> OMAP has it's own glue driver, so shouldn't it depend on the PHY >>> layer? >> >> right, but the PHY is connected to the dwc3 core and not to the glue. >>> I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) >>> >>> I don't think dwc3 (core) should care about that. The PHY layer needs >>> to tell us that. If the PHY driver that the platform depends is not >>> available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up >>> returning -EPROBE_DEFER. >> >> I don't think the PHY layer can 'reliably' tell if PHY driver is >> available or >> not. Consider when the phy_provider_register fails, there is no way to >> know if >> PHY driver is available or not. There are a few cases where PHY layer >> returns >> -EPROBE_DEFER but none of them can tell for sure that PHY driver is >> either >> available and failed or not availabl
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
On Mon, Feb 24, 2014 at 03:21:05PM +0530, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: > > On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: > >> Hi Roger, > >> > >> On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: > >>> Hi, > >>> > >>> On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: > On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: > > Hi, > > > > On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: > >> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: > >>> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: > >>> For the controller drivers the PHYs are just a resource like any > >>> other. The controller drivers can't have any responsibility of > >>> them. They should not care if PHY drivers are available for them or > >>> not, or even if the PHY framework is available or not. > >> > >> huh? If memory isn't available you don't continue probing, right ? If > >> your IORESOURCE_MEM is missing, you also don't continue probing, if > >> your > >> IRQ line is missing, you bail too. Those are also nothing but resources > >> to the driver, what you're asking here is to treat PHY as a _different_ > >> resource; which might be fine, but we need to make sure we don't > >> continue probing when a PHY is missing in a platform that certainly > >> needs a PHY. > > > > Yes, true. In my head I was comparing the PHY only to resources like > > gpios, clocks, dma channels, etc. that are often optional to the > > drivers. > > > >> But I really want to see the argument against using no-op. As far > >> as I > >> could see, everybody needs a PHY driver one way or another, some > >> platforms just haven't sent any PHY driver upstream and have their > >> own > >> hacked up solution to avoid using the PHY layer. > > > > Not true in our case. Platforms using Intel's SoCs and chip sets may > > or may not have controllable USB PHY. Quite often they don't. The > > Baytrails have usually ULPI PHY for USB2, but that does not mean > > they > > provide any vendor specific functions or any need for a driver in > > any > > case. > > that's different from what I heard. > >>> > >>> I don't know where you got that impression, but it's not true. The > >>> Baytrail SoCs for example don't have internal USB PHYs, which means > >>> the manufacturers using it can select what they want. So we have > >>> boards where PHY driver(s) is needed and boards where it isn't. > >> > >> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? > >> You have an external PIPE3 interface ? That's quite an achievement, > >> kudos to your HW designers. Getting timing closure on PIPE3 is a > >> difficult task. > > > > No, only the USB2 PHY is external. I'm giving you wrong information, > > I'm sorry about that. Need to concentrate on what I'm writing. > > > > > > > >>> This is really good to get. We have some projects where we are dealing > >>> with more embedded environments, like IVI, where the kernel should be > >>> stripped of everything useless. Since the PHYs are autonomous, we > >>> should be able to disable the PHY libraries/frameworks. > >> > >> hmmm, in that case it's a lot easier to treat. We can use > >> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or > >> something like that. > >> > >> The difficult is really reliably supporting e.g. OMAP5 (which won't > >> work > >> without a PHY) and your BayTrail with autonomous PHYs. What can we use > >> as an indication ? > > > > OMAP has it's own glue driver, so shouldn't it depend on the PHY > > layer? > > right, but the PHY is connected to the dwc3 core and not to the glue. > > > >> I mean, I need to know that a particular platform depends on a PHY > >> driver before I decide to return -EPROBE_DEFER or just assume the PHY > >> isn't needed ;-) > > > > I don't think dwc3 (core) should care about that. The PHY layer needs > > to tell us that. If the PHY driver that the platform depends is not > > available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up > > returning -EPROBE_DEFER. > > I don't think the PHY layer can 'reliably' tell if PHY driver is > available or > not. Consider when the phy_provider_register fails, there is no way to > know if > PHY driver is available or not. There are a few cases where PHY layer > returns > -EPROBE_DEFER but none of them can tell for sure that PHY driver is > either > available and failed or not available at all. It would be best for us to > leav
