Re: [PATCH] usb: musb: fix context save over suspend.
NeilBrown writes: > On Mon, 21 Jan 2013 13:38:59 +0200 Igor Grinberg > wrote: > >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA1 >> >> Hi Neil, >> >> On 01/21/13 11:28, NeilBrown wrote: >> > >> > >> > The standard suspend sequence involves runtime_resuming >> > devices before suspending the system. >> > So just saving context in runtime_suspend and restoring it >> > in runtime resume isn't enough. We must also save in "suspend" >> > and restore in "resume". >> > >> > Without this patch, and OMAP3 system with off_mode enabled will find >> > the musb port non-functional after suspend/resume. With the patch it >> > works perfectly. >> >> Hmmm... Some time ago, this has been removed in >> 5d193ce8 (usb: musb: PM: fix context save/restore in suspend/resume path) >> >> Am I missing something? Or things changed and now this patch is correct? > > Hi Igor, > thanks for alerting me to that patch does anyone else get the feeling > that power management to too complex to be understood by a mere human? Yes. ;) > That commit (5d193ce8) suggests that the musb-hdrc device is an > 'omap_device', or maybe has a PM domain set to something else. > However it isn't/doesn't. dev->pm_domain is NULL. So no PM domain layer > will ever call the musb_core musb_runtime_suspend/resume. > > The parent device - musb-omap2430 - is an omap device, does have pm_domain > set, and does have its omap2430_runtime_suspend/resume called for system > suspend and so the context for that device is saved and restored. > However that doesn't help the context for musb-hdrc. > > Whether musb ever was an omap_device is beyond my archaeological skills to > determine. > > Kevin: Was musb-hdrc ever a device with a pm_domain? or was it only ever > the various possible parents that had domains? > Are you able to defend your earlier patch in today's kernel? It > certainly causes my device not to work properly. Sorry for the delay here, I'm back to a place where I can test this on real hardware. My patch was fixing a real hang when musb was built-in (or loaded), in host-mode (mini-A cable attached) but no devices attached. I just tried to reproduce this, and with your patch, the system hangs during suspend. That being said, your description makes sense why this context save/restore is needed. Perhaps your patch needs to add a check whether the device is runtime suspended (I gather this is what Ruslan's patch is doing.) Kevin -- 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] usb: musb: fix context save over suspend.
NeilBrown writes: > On Tue, 12 Feb 2013 13:03:36 -0800 Kevin Hilman wrote: > >> NeilBrown writes: >> [...] >> My patch was fixing a real hang when musb was built-in (or loaded), in >> host-mode (mini-A cable attached) but no devices attached. I just tried >> to reproduce this, and with your patch, the system hangs during suspend. >> > > Odd. I plug in a mini-A cable, note that the 'mode' file holds > 'a_idle' (sometimes just plugging in the cable isn't enough to trigger that, > but sometimes it is) and suspend/resume work perfectly. Though after > resume it is back to b_idle. > > unplug/replug and it is back to a_idle. suspend/resume and back to b_idle. > >> That being said, your description makes sense why this context >> save/restore is needed. Perhaps your patch needs to add a check whether >> the device is runtime suspended (I gather this is what Ruslan's patch is >> doing.) > > I'm not sure it is possible for the device to be runtime suspended at this > point. Certainly my device never is, even if it was just before the suspend > sequence started. Something is waking it up... > (instruments the code). > > Ahh - usb_suspend() calls choose_wakeup() which might call > pm_runtime_resume() if the could be a need to reprogram the wakeup setting. > As that is a 'might', the device might not be runtime-awake when 'suspend' > runs. > > Can you see if this, on top of my previous patch, does any better on your > hardware? Yes, this patch adding the check on top of your previous one makes things work just fine on my hardware (3530/Overo). Kevin > Thanks, > NeilBrown > > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > index 956db0e..00deb94 100644 > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -2278,7 +2278,8 @@ static int musb_suspend(struct device *dev) > } > > spin_unlock_irqrestore(&musb->lock, flags); > - musb_save_context(musb); > + if (!pm_runtime_status_suspended(dev)) > + musb_save_context(musb); > return 0; > } > > @@ -2289,7 +2290,8 @@ static int musb_resume_noirq(struct device *dev) >* module got reset through the PSC (vs just being disabled). >*/ > struct musb *musb = dev_to_musb(dev); > - musb_restore_context(musb); > + if (!pm_runtime_status_suspended(dev)) > + musb_restore_context(musb); > return 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
Re: [RFC PATCH] PM / Runtime: Allow to inactivate devices during system suspend
Alan Stern writes: > On Tue, 19 Nov 2013, Ulf Hansson wrote: > >> At the moment, system PM is already affecting behaviour of runtime PM >> since it is preventing runtime suspend during system suspend. > > Sure. And that behavior is documented. In any case, it's a bug for > drivers to depend on runtime suspend for carrying out a system suspend. As Rafael mentioned, there is bus/pm_domain code that comes into play here, so I'm not sure it's always a bug. IMO, it's not a bug for the driver to depend on runtime PM if the bus/pm_domain is handling the details. On OMAP, we handle all the SoC on-chip devices with a pm_domain since the low-level PM operations that need to happen are bus-level things not device-level things. Therefore, drivers for these devices can rely entirely on runtime PM, even for system suspend. The late/early callbacks in the pm_domain can see if the device is runtime suspended already or not and behave accordingly. So, this all *can* work by handling it at the bus/pm_domain level, but as Ulf has mentioned (and I agree) it seems like a clunky workaround because the PM core is preventing it from happening as one might expect. Kevin -- 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: [RFC PATCH] PM / Runtime: Allow to inactivate devices during system suspend
Alan Stern writes: > On Tue, 19 Nov 2013, Kevin Hilman wrote: > >> Alan Stern writes: >> >> > On Tue, 19 Nov 2013, Ulf Hansson wrote: >> > >> >> At the moment, system PM is already affecting behaviour of runtime PM >> >> since it is preventing runtime suspend during system suspend. >> > >> > Sure. And that behavior is documented. In any case, it's a bug for >> > drivers to depend on runtime suspend for carrying out a system suspend. >> >> As Rafael mentioned, there is bus/pm_domain code that comes into play >> here, so I'm not sure it's always a bug. >> >> IMO, it's not a bug for the driver to depend on runtime PM if the >> bus/pm_domain is handling the details. >> >> On OMAP, we handle all the SoC on-chip devices with a pm_domain since >> the low-level PM operations that need to happen are bus-level things not >> device-level things. Therefore, drivers for these devices can rely >> entirely on runtime PM, even for system suspend. The late/early >> callbacks in the pm_domain can see if the device is runtime suspended >> already or not and behave accordingly. >> >> So, this all *can* work by handling it at the bus/pm_domain level, but >> as Ulf has mentioned (and I agree) it seems like a clunky workaround >> because the PM core is preventing it from happening as one might expect. > > The problem is that userspace can disable runtime PM at any time by > writing "on" to /sys/.../power/control. Once that's done, you can't > depend on runtime PM to put the device into a low-power state during > system suspend. We handle that too. The way we handle it, that the pm_domain code "knows" that the device *should* be runtime suspended by the time the pm_domain suspend_noirq callback is called. So it checks pm_runtime_status_suspended() and if the device is not runtime suspended, the pm_domain will "forcibly" runtime suspend the device (which just means, calling the driver's ->runtime_suspend(), doing the bus-level magic, and setting the runtime PM state accordingly.) This is arguably the same approach as the using the late callbacks that you've already described, but it's just done in a hidden way at the bus level. Also, this solution predates the addtion of the late callbacks and I believe may have even been initially proposed by you (or maybe Rafael, I forgot.) Kevin -- 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 V5 5/9] USB: OHCI: make ohci-at91 a separate driver
Manjunath,
Manjunath Goudar writes:
> Separate the TI OHCI Atmel host controller driver from ohci-hcd
> host code so that it can be built as a separate driver module.
> This work is part of enabling multi-platform kernels on ARM.
This broke booting on atmel sama5 boards (and likely others with the
same conversion)...
[...]
> +static int __init ohci_at91_init(void)
> +{
> + if (usb_disabled())
> + return -ENODEV;
> +
> + pr_info("%s: " DRIVER_DESC "\n", hcd_name);
> + ohci_init_driver(&ohci_at91_hc_driver, NULL);
ohci_init_driver() doesn't have any sanity checks for NULL overrides, so
it blindly dereferences and faults.
Some of the other conversions have this same problem (at least omap3).
Did anyone test this series on hardware?
I'm not too familiar with OHCI, but something like the patch below is
probably needed along with this series.
Kevin
>From a3b5cc90e74038a6449fbd25e0d720ea02884f30 Mon Sep 17 00:00:00 2001
From: Kevin Hilman
Date: Fri, 27 Sep 2013 08:07:19 -0700
Subject: [PATCH] USB: OHCI: ohci_init_driver(): sanity check overrides
Check for non-NULL overrides before dereferencing since platforms may
pass in NULL overrides.
Signed-off-by: Kevin Hilman
---
drivers/usb/host/ohci-hcd.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 21d937a..8ada13f 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1161,10 +1161,12 @@ void ohci_init_driver(struct hc_driver *drv,
/* Copy the generic table to drv and then apply the overrides */
*drv = ohci_hc_driver;
- drv->product_desc = over->product_desc;
- drv->hcd_priv_size += over->extra_priv_size;
- if (over->reset)
- drv->reset = over->reset;
+ if (over) {
+ drv->product_desc = over->product_desc;
+ drv->hcd_priv_size += over->extra_priv_size;
+ if (over->reset)
+ drv->reset = over->reset;
+ }
}
EXPORT_SYMBOL_GPL(ohci_init_driver);
--
1.8.3
--
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: [RFC PATCH 2/6] mfd: omap-usb-host: Put pins in IDLE state on suspend
Hi Roger, Roger Quadros writes: > In order to support wake up from suspend use the pinctrl > framework to put the USB host pins in IDLE state during suspend. > > CC: Samuel Ortiz > Signed-off-by: Roger Quadros You should use helpers for this now in the pinctrl core: http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html Kevin -- 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: [RFC PATCH 4/6] USB: ehci-omap: Suspend the controller during bus suspend
Hi Roger, Roger Quadros writes: > Runtime suspend the controller during bus suspend and resume it > during bus resume. This will ensure that the USB Host power domain > enters lower power state and does not prevent the SoC from > endering deeper sleep states. > > Remote wakeup will come up as an interrupt while the controller > is suspended, so tackle it carefully using a workqueue. I don't think you need a special workqueue here. The workqueue of the PM core (pm_wq) will be used if you just use an asynchronous 'get' in the ISR. Then, the driver's own runtime PM callbacks can be used instead of an additional workqueue. Another thing to worry about here when using runtime PM to implement suspend/resume is that runtime PM can be disabled from userspace (e.g. echo disabled > /sys/devices/.../power/control.) If that's the case, all the pm_runtime_suspended() checks will always fail becuase that call also checks if runtime PM is disabled. You'll likely want to use the pm_runtime_status_suspended() check instead, which checks only the status, and doesn't matter if runtime PM is currently disabled. Because of the corner issues here, please test system suspend/resume when runtime PM has been disabled. Kevin -- 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: [RFC PATCH 5/6] ARM: dts: omap3beagle-xm: Add idle state pins for USB host
Roger Quadros writes:
> Add the Idle state pins for USB host and enable WAKEUP on
> DIR, DAT0-3, so that the PHY can wakeup the OMAP SoC from
> sleep on any USB activity (e.g. remote wakeup or connect/disconnect).
>
> CC: Benoît Cousson
> Signed-off-by: Roger Quadros
This one doesn't apply...
> ---
> arch/arm/boot/dts/omap3-beagle-xm.dts | 29 +++--
> 1 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts
> b/arch/arm/boot/dts/omap3-beagle-xm.dts
> index d3808ed..f1d56c2 100644
> --- a/arch/arm/boot/dts/omap3-beagle-xm.dts
> +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
> @@ -89,12 +89,7 @@
> };
>
> &omap3_pmx_core {
> - pinctrl-names = "default";
> - pinctrl-0 = <
> - &hsusbb2_pins
> - >;
> -
> - hsusbb2_pins: pinmux_hsusbb2_pins {
This omap3_pmx_core section doesn't exist upstream in the xM DTS file
(but does in omap3-beagle.dts.)
Is there a dependency patch missing?
Kevin
--
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] USB: OHCI: make ohci-da8xx a separate driver
On 07/02/2013 08:14 AM, Manjunath Goudar wrote: > > > On 2 July 2013 20:20, Sergei Shtylyov > <mailto:sergei.shtyl...@cogentembedded.com>> wrote: > > Hello. > > > On 02-07-2013 15:36, Manjunath Goudar wrote: > > Separate the Davinci OHCI host controller driver from ohci-hcd > host code so that it can be built as a separate driver module. > This work is part of enabling multi-platform kernels on ARM; > it would be nice to have in 3.11. > > > One preexisting error: > "da8xx_syscfg0_base" [drivers/usb/host/ohci-da8xx.__ko] undefined! > > > Fixed eventually using below modification: > added EXPORT_SYMBOL_GPL(da8xx___syscfg0_base) in > arch/arm/mach-davinci/devices-__da8xx.c. > > >And you managed to get this fix into the DaVinci tree? I tried it > long ago and it was refused by then DaVinci maintainer Kevin Hilman. > > > Yes I saw your patch that is what I mentioned in patch description. > We will wait for DaVinci maintainer response,what he will suggest. Note that Sekhar Nori (now Cc'd) is the primary maintainer of davinci, and I'll defer the final decision to him. However, the mach-davinci change is not in this patch, so I'm not sure exactly how it relates here, since that problem exists independently of this patch. That being said, I will NAK the above EXPORT_SYMBOL change in mach-davinci code because passing data between platform code and drivers via global variables is still a bad idea. Some helper accessor functions will need to be created to abstract those low-level accesses. > Signed-off-by: Manjunath Goudar <mailto:manjunath.gou...@linaro.org>> > Cc: Arnd Bergmann mailto:a...@arndb.de>> > Cc: Alan Stern <mailto:st...@rowland.harvard.edu>> > Cc: Sergei Shtylyov <mailto:sshtyl...@ru.mvista.com>> > Cc: Kevin Hilman mailto:k...@hilman.org>> Also, please khil...@linaro.org instead of my personal address. Thanks, Kevin -- 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 4/6] mfd: omap-usb-host: Put pins in IDLE state on suspend
On 07/10/2013 05:23 PM, Roger Quadros wrote:
> In order to support wake up from suspend use the pinctrl
> framework to put the USB host pins in IDLE state during suspend.
>
> CC: Samuel Ortiz
> Signed-off-by: Roger Quadros
[...]
> @@ -608,6 +618,14 @@ static int usbhs_omap_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> + if (!dev->pins || !dev->pins->idle_state) {
> + /* If IDLE pins are not available, we can't remote wakeup,
> + * so prevent idling in that case.
> + */
nit: multi-line comment style
Also, if there are no pins, aren't the pinctrl ops nops anyways? IOW,
not sure the need
for this is clear, and it's not mentioned in the changelog.
Kevin
--
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] OMAP: config : disable the usb host configuration in omap2plus_defconfig
Kevin Hilman writes: > Keshava Munegowda writes: > >> The usb host is disabled in the omap2 build; This is because >> usb host is causing the retention to break in cpu idle. > > ... and causes warnings during boot, and hangs in suspend, can't suspend > using NFSroot, etc. etc. > > Thank you for disabling this by default, But I don't think this fix > is targetted enough (more below.) > > Tony, when v3.5 came out, I asked them to fix this properly, but nothing > was done. So I've resorted to requesting that this feature be disabled > by default since it introduced PM regressions. Of course, I'd like to > see this fixed properly instead, but we've run out of time for v3.5 and > IMO, this PM regression needs to be fixed for v3.5. See my more > targetted patches below. > >> Signed-off-by: Keshava Munegowda >> --- >> arch/arm/configs/omap2plus_defconfig | 11 --- >> 1 file changed, 11 deletions(-) >> >> diff --git a/arch/arm/configs/omap2plus_defconfig >> b/arch/arm/configs/omap2plus_defconfig >> index 9854ff4..7b32da6 100644 >> --- a/arch/arm/configs/omap2plus_defconfig >> +++ b/arch/arm/configs/omap2plus_defconfig >> @@ -170,17 +170,6 @@ CONFIG_SND_USB_AUDIO=m >> CONFIG_SND_SOC=m >> CONFIG_SND_OMAP_SOC=m >> CONFIG_SND_OMAP_SOC_OMAP3_PANDORA=m >> -CONFIG_USB=y >> -CONFIG_USB_DEBUG=y >> -CONFIG_USB_ANNOUNCE_NEW_DEVICES=y >> -CONFIG_USB_DEVICEFS=y >> -CONFIG_USB_SUSPEND=y >> -CONFIG_USB_MON=y >> -CONFIG_USB_EHCI_HCD=y >> -CONFIG_USB_WDM=y >> -CONFIG_USB_STORAGE=y >> -CONFIG_USB_LIBUSUAL=y >> -CONFIG_USB_TEST=y > > Why disable all of these? With that, it's not terribly clear what is > going on, or why. > > Instead, what you need is a targetted fix to just disable the problem > driver by default: > > Either this: > > diff --git a/arch/arm/configs/omap2plus_defconfig > b/arch/arm/configs/omap2plus_defconfig > index 9854ff4..11828e6 100644 > --- a/arch/arm/configs/omap2plus_defconfig > +++ b/arch/arm/configs/omap2plus_defconfig > @@ -176,7 +176,6 @@ CONFIG_USB_ANNOUNCE_NEW_DEVICES=y > CONFIG_USB_DEVICEFS=y > CONFIG_USB_SUSPEND=y > CONFIG_USB_MON=y > -CONFIG_USB_EHCI_HCD=y > CONFIG_USB_WDM=y > CONFIG_USB_STORAGE=y > CONFIG_USB_LIBUSUAL=y > > or this: > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index e129c82..3747826 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -825,7 +825,7 @@ config MFD_WL1273_CORE > config MFD_OMAP_USB_HOST > bool "Support OMAP USBHS core driver" > depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3 > - default y > + default n > help > This is the core driver for the OAMP EHCI and OHCI drivers. > This MFD driver does the required setup functionalities for > > Will have the inteded affect with the benefit of being much clearer > about what is actually being disabled, and why. Since I'm primarily concerned about automated PM testing of omap2plus_defconfig, I think the first one is better since it just affects the default omap2plus_defconfig and allows custom defconfigs to continue to work as usual. I'll send a patch. Kevin -- 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] OMAP: USB : Fix the EHCI enumeration and core retention issue
"Munegowda, Keshava" writes: > On Wed, Jul 11, 2012 at 3:59 PM, Samuel Ortiz wrote: >> Hi Keshava, Kevin, >> >> On Fri, Jul 06, 2012 at 05:29:00PM +0530, Munegowda, Keshava wrote: >>> Samuel >>> I have sent that patch to disable the ehci in >>> omap2plus_defconfig; after merging that >>> please merge this patch too. This will fix the crashes in during boot >>> with NFS in beagleXM >> I'm going to apply and push this patch for 3.5, and the defconfig patch can >> be >> pushed through Tony's tree. >> Kevin, could you please ACK it ? >> >> Cheers, >> Samuel. >> > > Thanks Samuel > > Kevin, > need your ack for this. You never answered earlier questions from myself or Russ Dill about the more targetted patches from Russ: ARM: OMAP: USB: Fixup ehci_hcd_omap_probe error path Fix OMAP EHCI suspend/resume failure (i693) '354ab856' causes Also, your current $SUBJECT patch is large and not well described. e.g. what is "not correct" and why. Why does it fix the problems mentioned? The original changelog mentions the "core retention issue" but this patch does nothing to address that. If you want an Ack from me, especially because I'm not an expert in this IP, you'll have to describe things in a way that I can understand. IMO, at this point of the dev cycle (trying to stabilize v3.5), the full cleanup/fix of this feature will need to be done for v3.6. For v3.5, I think the two patches from Russ Dill should be merged. They are targetted fixes and very well described. Kevin > The commit id 354ab8567ae3107a8cbe7228c3181990ba598aac titled > "Fix OMAP EHCI suspend/resume failure (i693)", > is handling the clocks in ehci driver which is not correct, > it will be through the usb host driver ( /mfd/omap-usb_host.c ) exporting > APIs to ehci driver to handler the port clocks. > > for now, this patch is necessary to remove the > commit id 354ab8567ae3107a8cbe7228c3181990ba598aac titled > "Fix OMAP EHCI suspend/resume failure (i693)", > > regards > keshava > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: Removing vestiges of CONFIG_USB_SUSPEND
Hi Alan, Alan Stern writes: > Felipe and Kevin: > > While removing the remaining usages of USB_SUSPEND (things that I > missed in the original patch), I noticed that > arch/arm/configs/omap2plus_defconfig does not enable PM_RUNTIME -- even > though it currently does enable USB_SUSPEND, which depends on > PM_RUNTIME. Was this an oversight? Looks like an oversight, but covered up by some other default options in mach-omap2/Kconfig. Specifially, config ARCH_OMAP2PLUS_TYPICAL (default y) does a select PM_RUNTIME. > In a little while I will submit a patch which removes USB_SUSPEND from > that file. You or someone else may want to write a separate patch to > enable PM_RUNTIME. I think we'll be covered by our current defaults. Thanks for the heads up. Kevin -- 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] usb: phy: msm: fix compilation errors when !CONFIG_PM_SLEEP
Josh Cartwright writes:
> Both the PM_RUNTIME and PM_SLEEP callbacks call into the common
> msm_otg_{suspend,resume} routines, however these routines are only being
> built when CONFIG_PM_SLEEP. In addition, msm_otg_{suspend,resume} also
> depends on msm_hsusb_config_vddcx(), which is only built when
> CONFIG_PM_SLEEP.
>
> Fix the CONFIG_PM_RUNTIME, !CONFIG_PM_SLEEP case by changing the
> preprocessor conditional, and moving msm_hsusb_config_vddcx().
>
> While we're here, eliminate the CONFIG_PM conditional for setting
> up the dev_pm_ops.
>
> This address the following errors Russell King has hit doing randconfig
> builds:
>
> drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_suspend':
> drivers/usb/phy/phy-msm-usb.c:1691:2: error: implicit declaration of function
> 'msm_otg_suspend'
> drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_runtime_resume':
> drivers/usb/phy/phy-msm-usb.c:1699:2: error: implicit declaration of function
> 'msm_otg_resume'
>
> Cc: Ivan T. Ivanov
> Reported-by: Russell King
> Signed-off-by: Josh Cartwright
[...]
> @@ -440,7 +414,32 @@ static int msm_otg_reset(struct usb_phy *phy)
> #define PHY_SUSPEND_TIMEOUT_USEC (500 * 1000)
> #define PHY_RESUME_TIMEOUT_USEC (100 * 1000)
>
> -#ifdef CONFIG_PM_SLEEP
> +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
nit: you should just use CONFIG_PM here since that is what it's for.
c.f. kernel/power/Kconfig:
config PM
def_bool y
depends on PM_SLEEP || PM_RUNTIME
Kevin
--
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: regression(ti platforms): next-20140210 (ehci?)
Roger Quadros writes:
> +devicetree
>
> On 02/10/2014 05:59 PM, Nishanth Menon wrote:
>> Hi,
>>
>> A quick note to report that I saw regression in today's next tag (logs
>> indicate around EHCI) boot on various TI platforms:
>>
>> Note: crane and sdp2430 are not expected to pass with
>> multi_v7_defconfig (note: omap2plus_defconfig boot seems to be sane
>> but USB is disabled there)
>>
>> next-20140210-multi_v7_defconfig
>> 1: am335x-evm: Boot PASS: http://slexy.org/raw/s2zYHdPb94
>> 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2UChLyzSE
>> 3: am3517-evm: Boot FAIL: http://slexy.org/raw/s20Br9XLO1
>> around ehci
>>
>> 4: am37x-evm: Boot FAIL: http://slexy.org/raw/s20mVz9Wc7
>> around ehci
>>
>> 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s2byveBYtT
>> 6: BeagleBoard-XM: Boot FAIL: http://slexy.org/raw/s21sOgJNwK
>> around ehci
>>
>> 7: BeagleBone-Black: Boot PASS: http://slexy.org/raw/s2ovVNAmO7
>> 8: crane: No Image built - Missing platform support?:
>> 9: dra7: Boot PASS: http://slexy.org/raw/s217qwaXsM
>> 10:ldp: Boot FAIL: http://slexy.org/raw/s203IvjE23
>> around ehci
>>
>> 11: PandaBoard-ES: Boot FAIL: http://slexy.org/raw/s2NvkRx2YJ
>> around ehci
>
> I think the problem is that ehci-platform driver gets loaded instead of
> ehci-omap.
>
> In the DT node we have compatible ids for both. e.g. for omap4.dtsi
>
> usbhsehci: ehci@4a064c00 {
> compatible = "ti,ehci-omap", "usb-ehci";
> reg = <0x4a064c00 0x400>;
> interrupt-parent = <&gic>;
> interrupts = ;
> };
>
> Shouldn't ehci-omap driver be getting a higher priority than usb-ehci?
>
> A quick fix would be to eliminate "usb-ehci" from the DT node of all failing
> platforms.
I can confirm that simply remvoing usb-ehci from omap[34].dtsi nodes
fixed the problem for me on 3530/overo, 3730/beagle-xM and
4460/panda-es. But I don't think that's the right fix. First we have
to figure out why ehci-omap stopped getting loaded first.
Kevin
--
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: regression(ti platforms): next-20140210 (ehci?)
Nishanth Menon writes:
> On 02/10/2014 12:28 PM, Kevin Hilman wrote:
>> Roger Quadros writes:
>>
>>> +devicetree
>>>
>>> On 02/10/2014 05:59 PM, Nishanth Menon wrote:
>>>> Hi,
>>>>
>>>> A quick note to report that I saw regression in today's next tag (logs
>>>> indicate around EHCI) boot on various TI platforms:
>>>>
>>>> Note: crane and sdp2430 are not expected to pass with
>>>> multi_v7_defconfig (note: omap2plus_defconfig boot seems to be sane
>>>> but USB is disabled there)
>>>>
>>>> next-20140210-multi_v7_defconfig
>>>> 1: am335x-evm: Boot PASS: http://slexy.org/raw/s2zYHdPb94
>>>> 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2UChLyzSE
>>>> 3: am3517-evm: Boot FAIL: http://slexy.org/raw/s20Br9XLO1
>>>> around ehci
>>>>
>>>> 4: am37x-evm: Boot FAIL: http://slexy.org/raw/s20mVz9Wc7
>>>> around ehci
>>>>
>>>> 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s2byveBYtT
>>>> 6: BeagleBoard-XM: Boot FAIL: http://slexy.org/raw/s21sOgJNwK
>>>> around ehci
>>>>
>>>> 7: BeagleBone-Black: Boot PASS: http://slexy.org/raw/s2ovVNAmO7
>>>> 8: crane: No Image built - Missing platform support?:
>>>> 9: dra7: Boot PASS: http://slexy.org/raw/s217qwaXsM
>>>> 10:ldp: Boot FAIL: http://slexy.org/raw/s203IvjE23
>>>> around ehci
>>>>
>>>> 11: PandaBoard-ES: Boot FAIL: http://slexy.org/raw/s2NvkRx2YJ
>>>> around ehci
>>>
>>> I think the problem is that ehci-platform driver gets loaded instead of
>>> ehci-omap.
>>>
>>> In the DT node we have compatible ids for both. e.g. for omap4.dtsi
>>>
>>> usbhsehci: ehci@4a064c00 {
>>> compatible = "ti,ehci-omap", "usb-ehci";
>>> reg = <0x4a064c00 0x400>;
>>> interrupt-parent = <&gic>;
>>> interrupts = >> IRQ_TYPE_LEVEL_HIGH>;
>>> };
>>>
>>> Shouldn't ehci-omap driver be getting a higher priority than usb-ehci?
>>>
>>> A quick fix would be to eliminate "usb-ehci" from the DT node of all
>>> failing platforms.
>>
>> I can confirm that simply remvoing usb-ehci from omap[34].dtsi nodes
>> fixed the problem for me on 3530/overo, 3730/beagle-xM and
>> 4460/panda-es. But I don't think that's the right fix. First we have
>> to figure out why ehci-omap stopped getting loaded first.
>
> Wont that depend on driver probe order? of_match_device is fairly
> simple compatible walk through without looking at other drivers which
> might also be compatible, but not yet probed?
>
> The issue started I think with the following patch getting merged:
> ehci-platform: Add support for clks and phy passed through devicetree
> some version of http://www.spinics.net/lists/linux-usb/msg101061.html
> introduced { .compatible = "usb-ehci", },
This is what I was getting at: an understanding of what caused the
failue in the first place.
> Now, in the build we have two drivers which dts claims compatibility
> with, but only 1 driver actually works (drivers/usb/host/ehci-omap.c)
> for the platform. Thinking that way, in fact, the current
> compatibility even matches drivers/usb/host/ehci-ppc-of.c which
> obviously wont work either.
Right, so I agree that it makes sense to remove a compatible string
where there is no compatability, but a couple other things should happen
here.
1) changelog should describe why this compatible string is in the omap
dtsi files in first place.
2) investigation into the patch that introduced this change to double
check it's not introducing other breakage as well.
Kevin
--
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 v2 0/2] ohci-/ehci-platform: Change compatible string to generic-?hci
Hans de Goede writes: > Hi Greg, > > And here is v2 of my ohci-/ehci-platform fixes for the regression of USB > support on various ARM boards I caused in linux-next. > > As expected some people still did not like the ?hci-platform compatible > string I went for in v1, hence this v2. The good news is it seems everyone > seems to be able to live with generic-?hci as compatible now, so it seems this > issue is finally settled. > > Please add these 2 patches to your usb-next tree, as said in the cover-letter > of v1, I'm fine with you squashing these into the first 2 patches of my > original series, but if you want them separate to preserve history that is > fine too. I tested this series on top of next-20140211 on the OMAP platforms where I noticed the regressions, and confirm it fixes the problem. Tested-by: Kevin Hilman Kevin -- 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 00/16] OMAP USB Host cleanup
Roger Quadros writes: > Hi, > > This patchset addresses the following > > - Avoid addressing clocks one by one by name and use a for loop + bunch > of cleanups. > - Get number of channels/ports dynamically either from revision register > or from platform data. Avoids getting clocks that are not present. > - Add OMAP5 and HSIC mode (Not tested) > - Save power on Panda when EHCI driver is not loaded. > Seeing the clock changes/cleanups, I gave this a spin on OMAP3 (3530/Beagle, 3530/Overo, 3730/Beagle-xM, 3730/OveroSTORM) to see if it fixed up the problem where CORE does not hit retention in idle when USB host is enabled, even with no devices attached. Unfortunately, it didn't help. :( Felipe, Keshava, any plans to address this problem which has been around for a couple cycles now and led me to disable USB host in omap2plus_defconfig?[1] Kevin [1] https://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=06b4ba529528fbf9c24ce37b7618f4b0264750e2 -- 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 00/16] OMAP USB Host cleanup
Roger Quadros writes: > Kevin, > > On 11/16/2012 10:08 PM, Kevin Hilman wrote: >> Roger Quadros writes: >> >>> Hi, >>> >>> This patchset addresses the following >>> >>> - Avoid addressing clocks one by one by name and use a for loop + bunch >>> of cleanups. >>> - Get number of channels/ports dynamically either from revision register >>> or from platform data. Avoids getting clocks that are not present. >>> - Add OMAP5 and HSIC mode (Not tested) >>> - Save power on Panda when EHCI driver is not loaded. >>> >> >> Seeing the clock changes/cleanups, I gave this a spin on OMAP3 >> (3530/Beagle, 3530/Overo, 3730/Beagle-xM, 3730/OveroSTORM) to see if it >> fixed up the problem where CORE does not hit retention in idle when USB >> host is enabled, even with no devices attached. >> >> Unfortunately, it didn't help. :( > > oh that's bad. But this series wasn't meant to fix that ;). Oh, sorry. Yeah, I didn't mean this as a nak. Just an opportunity to complain to the maintainers that a long-standing issue needs to be addressed. Kevin -- 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 v3 2/4] usb: musb: core: added helper function for parsing DT
Alexandre Bailon writes:
> From: Petr Kulhavy
>
> This adds the function musb_get_mode() to get the DT property "dr_mode"
>
> Signed-off-by: Petr Kulhavy
> Acked-by: Sergei Shtylyov
> Signed-off-by: Alexandre Bailon
> Tested-by: David Lechner
> ---
> drivers/usb/musb/musb_core.c | 19 +++
> drivers/usb/musb/musb_core.h | 5 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 27dadc0..bba07e7 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -100,6 +100,7 @@
> #include
> #include
> #include
> +#include
>
> #include "musb_core.h"
> #include "musb_trace.h"
> @@ -130,6 +131,24 @@ static inline struct musb *dev_to_musb(struct device
> *dev)
> return dev_get_drvdata(dev);
> }
>
> +enum musb_mode musb_get_mode(struct device *dev)
> +{
> + enum usb_dr_mode mode;
> +
> + mode = usb_get_dr_mode(dev);
> + switch (mode) {
> + case USB_DR_MODE_HOST:
> + return MUSB_HOST;
> + case USB_DR_MODE_PERIPHERAL:
> + return MUSB_PERIPHERAL;
> + case USB_DR_MODE_OTG:
> + case USB_DR_MODE_UNKNOWN:
> + default:
> + return MUSB_OTG;
> + }
> +}
> +EXPORT_SYMBOL_GPL(musb_get_mode);
> +
> /*-*/
>
> #ifndef CONFIG_BLACKFIN
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index 2cb88a49..a406468 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -617,4 +617,9 @@ static inline void
> musb_platform_post_root_reset_end(struct musb *musb)
> musb->ops->post_root_reset_end(musb);
> }
>
> +/* gets the "dr_mode" property from DT and converts it into musb_mode
> + * if the property is not found or not recognized returns MUSB_OTG
> + */
nit: multi-line comment style
> +extern enum musb_mode musb_get_mode(struct device *dev);
> +
> #endif /* __MUSB_CORE_H__ */
Otherwise,
Reviewed-by: Kevin Hilman
Kevin
--
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 v3 3/3] usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in dsps
Bin Liu writes: > On Thu, Jan 19, 2017 at 11:06:59AM +0100, Alexandre Bailon wrote: >> Despite the CPPI 4.1 is a generic DMA, it is tied to USB. >> On the dsps, CPPI 4.1 interrupt's registers are in USBSS (the MUSB glue). >> Currently, to enable / disable and clear interrupts, the CPPI 4.1 driver >> maps and accesses to USBSS's register, which making CPPI 4.1 driver not >> really generic. >> Move the interrupt management to dsps driver. >> >> Signed-off-by: Alexandre Bailon >> --- >> drivers/dma/cppi41.c | 28 >> drivers/usb/musb/musb_dsps.c | 77 >> ++-- >> 2 files changed, 82 insertions(+), 23 deletions(-) > > This patch touches both dma and musb modules, I know it makes review > easier, but how we get it merged? One maintainer ACK it and the other > pick it up? Sorry for the dumb question, I am new as a maintainer... For patches where the change needs to go together, then one maintainer can ack, and the other can merge. Alternately, if the patch can be done in a way that the parts can go independently, that is sometimes cleaner. I don't know this code well enough to know which is which. Kevin -- 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 v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
On Wed, May 25, 2016 at 6:18 AM, Sekhar Nori wrote: > On Monday 23 May 2016 08:44 PM, David Lechner wrote: >> On 05/09/2016 06:46 PM, David Lechner wrote: >>> v5 changes: renamed "usbphy" to "usb_phy" or "usb-phy" as appropriate >>> [...] >> >> What should I be doing to keep this moving along? > > We need the related driver changes to be applied first. I could then use > an immutable branch to push the platform changes against. > > I did take a look at the patches and they look good to me. Except the > one comment from Sergei which I just now indicated that I agree with. Just checking on the status of this. I'm not seeing the driver changes in mainline yet. Any update? Kevin -- 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 v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
David, On Wed, Aug 17, 2016 at 4:35 AM, Kishon Vijay Abraham I wrote: > > Hi Kevin, > > On Saturday 13 August 2016 02:54 AM, Kevin Hilman wrote: > > On Wed, May 25, 2016 at 6:18 AM, Sekhar Nori wrote: > >> On Monday 23 May 2016 08:44 PM, David Lechner wrote: > >>> On 05/09/2016 06:46 PM, David Lechner wrote: > >>>> v5 changes: renamed "usbphy" to "usb_phy" or "usb-phy" as appropriate > >>>> > > > > [...] > > > >>> > >>> What should I be doing to keep this moving along? > >> > >> We need the related driver changes to be applied first. I could then use > >> an immutable branch to push the platform changes against. > >> > >> I did take a look at the patches and they look good to me. Except the > >> one comment from Sergei which I just now indicated that I agree with. > > > > Just checking on the status of this. I'm not seeing the driver > > changes in mainline yet. > > > > Any update? > > phy driver is already merged which actually introduced a compilation error. > The > fix for it is currently queued in linux-phy -fixes and it should be merged in > this -rc cycle. > I think it would be better if David Lechner re-spins the series re-based on > top > of latest mainline kernel and then merged by Bin/Alan. Does this work for you? Kevin -- 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/7] clk: gxbb: expose USB clocks
On Wed, Sep 7, 2016 at 3:14 PM, Stephen Boyd wrote: > On 09/07, Martin Blumenstingl wrote: >> On Wed, Sep 7, 2016 at 2:33 AM, Stephen Boyd wrote: >> > On 09/04, Martin Blumenstingl wrote: >> >> USB0_DDR_BRIDGE and USB1_DDR_BRIDGE1 are needed for the related >> >> dwc2 usb controller. USB, USB0 and USB1 are needed for the PHYs. >> >> Expose these clocks to DT and comment out in clk driver. >> >> >> >> Signed-off-by: Jerome Brunet >> >> Signed-off-by: Martin Blumenstingl >> > >> > Is authorship correct on this patch? Did Jerome author it >> > instead? >> We (Jerome and I) have both worked on this patch, that's why you have >> two signed-off-by's. >> Or is this simply about the order (author = from address should be >> listed first)? > > Yes. Typically author is listed first in the signed-off-by chain. > And if you use git-format-patch + git-send-email, there would also be a From: Jerome Brunet at the beginning so that when it gets applied by the maintainer, authorship is correct. Kevin -- 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/7] clk: gxbb: expose USB clocks
Stephen Boyd writes: > On 09/04, Martin Blumenstingl wrote: >> USB0_DDR_BRIDGE and USB1_DDR_BRIDGE1 are needed for the related >> dwc2 usb controller. USB, USB0 and USB1 are needed for the PHYs. >> Expose these clocks to DT and comment out in clk driver. >> >> Signed-off-by: Jerome Brunet >> Signed-off-by: Martin Blumenstingl >> --- > > Assuming authorship is resolved: > > Acked-by: Stephen Boyd After some clarification from Jerome (Martin is the primary author), I've applied this, with the SoB order swapped. Kevin -- 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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Ben Dooks writes:
> On 08/09/16 20:52, Martin Blumenstingl wrote:
>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman wrote:
>>>> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
>>>> + if (IS_ERR(phy)) {
>>>> + dev_err(&pdev->dev, "failed to create PHY\n");
>>>> + return PTR_ERR(phy);
>>>> + }
>>>> +
>>>> + if (usb_reset_refcnt++ == 0) {
>>>> + ret = device_reset(&pdev->dev);
>>>> + if (ret) {
>>>> + dev_err(&phy->dev, "Failed to reset USB PHY\n");
>>>> + return ret;
>>>> + }
>>>> + }
>>>
>>> The ref count + reset here looks like something that could/should be
>>> handled in a runtime PM callback.
>> Unfortunately that doesn't work (as Jerome found out) because both
>> PHYs are sharing the same reset line.
>> So if the second PHY would call device_reset then it would also reset
>> the first PHY!
>>
>> There's a comment above the declaration of usb_reset_refcnt which
>> tries to explain this:
>> "The PHYs are sharing a common reset line -> we are only allowed to
>> reset once for all PHYs."
>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>> {" line to make it easier to see?
>>
>
> pm-runtime has refcounting in it. When one of the nodes turns on,
> the pm-runtime will call your driver to say there is a user when
> this first use turns up.
>
> If all the sub-phys turn off and drop their refcount then the driver
> is called to say there are no more users and you can go to sleep.
After a chat w/Martin on IRC, It turns out runtime PM wont help here.
The reason is because there are physically two PHY devices[1]. Those 2
devices will be treated independely by runtime PM, and have separate
use-counting, which means doing what I proposed would cause a reset to
happen when either device was probed.
So, I think it's OK as it is.
Kevin
[1] from the DT patch:
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 2e8a3d9..02dfc54 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -151,6 +151,34 @@
#size-cells = <2>;
ranges;
+ usb-phys@c000 {
+ compatible = "simple-bus";
+ reg = <0x0 0xc000 0x0 0x40>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x0 0x0 0x0 0xc000 0x0 0x40>;
+
+ usb0_phy: usb_phy@0 {
+ compatible = "amlogic,meson-gxbb-usb2-phy";
+ #phy-cells = <0>;
+ reg = <0x0 0x0 0x0 0x20>;
+ resets = <&reset 34>;
+ clocks = <&clkc CLKID_USB &clkc CLKID_USB0>;
+ clock-names = "usb_general", "usb";
+ status = "disabled";
+ };
+
+ usb1_phy: usb_phy@20 {
+ compatible = "amlogic,meson-gxbb-usb2-phy";
+ #phy-cells = <0>;
+ reg = <0x0 0x20 0x0 0x20>;
+ resets = <&reset 34>;
+ clocks = <&clkc CLKID_USB &clkc CLKID_USB1>;
+ clock-names = "usb_general", "usb";
+ status = "disabled";
+ };
+ };
+
--
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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Martin Blumenstingl writes:
> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks wrote:
>> On 08/09/16 21:42, Kevin Hilman wrote:
>>>
>>> Ben Dooks writes:
>>>
>>>> On 08/09/16 20:52, Martin Blumenstingl wrote:
>>>>>
>>>>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman
>>>>> wrote:
>>>>>>>
>>>>>>> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
>>>>>>> + if (IS_ERR(phy)) {
>>>>>>> + dev_err(&pdev->dev, "failed to create PHY\n");
>>>>>>> + return PTR_ERR(phy);
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (usb_reset_refcnt++ == 0) {
>>>>>>> + ret = device_reset(&pdev->dev);
>>>>>>> + if (ret) {
>>>>>>> + dev_err(&phy->dev, "Failed to reset USB PHY\n");
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> + }
>>>>>>
>>>>>>
>>>>>> The ref count + reset here looks like something that could/should be
>>>>>> handled in a runtime PM callback.
>>>>>
>>>>> Unfortunately that doesn't work (as Jerome found out) because both
>>>>> PHYs are sharing the same reset line.
>>>>> So if the second PHY would call device_reset then it would also reset
>>>>> the first PHY!
>>>>>
>>>>> There's a comment above the declaration of usb_reset_refcnt which
>>>>> tries to explain this:
>>>>> "The PHYs are sharing a common reset line -> we are only allowed to
>>>>> reset once for all PHYs."
>>>>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>>>>> {" line to make it easier to see?
>>>>>
>>>>
>>>> pm-runtime has refcounting in it. When one of the nodes turns on,
>>>> the pm-runtime will call your driver to say there is a user when
>>>> this first use turns up.
>>>>
>>>> If all the sub-phys turn off and drop their refcount then the driver
>>>> is called to say there are no more users and you can go to sleep.
>>>
>>>
>>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>>
>>> The reason is because there are physically two PHY devices[1]. Those 2
>>> devices will be treated independely by runtime PM, and have separate
>>> use-counting, which means doing what I proposed would cause a reset to
>>> happen when either device was probed.
>>>
>>> So, I think it's OK as it is.
>>
>>
>> Surely you can do pm_runtime_get/put on the phy's parent platform
>> device and do it that way?
> could you please be more specific with that (do you mean pdev->dev.parent)?
> so we would use pm_runtime_{get_sync,put} with the parent, while we
> would still define the runtime_resume in our driver.
You'd also need to do get/put on the children, but yes, that's what Ben
is suggesting.
However, the problem with all of the solutions proposed (runtime PM ones
included) is that we're forcing a board-specific design issue (2 devices
sharing a reset line) into a driver that should not have any
board-specific assumptions in it.
For example, if this driver is used on another platform where different
PHYs have different reset lines, then one of them (the unlucky one who
is not probed first) will never get reset. So any form of per-device
ref-counting is not a portable solution.
I'm not sure yet how the reset framework is supposed to handle shared
reset lines, but that needs some investigation. I quick glance and it
seems that reset controllers can have shared lines, so that should be
investigated.
Kevin
--
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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Martin Blumenstingl writes: > On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote: >> However, the problem with all of the solutions proposed (runtime PM ones >> included) is that we're forcing a board-specific design issue (2 devices >> sharing a reset line) into a driver that should not have any >> board-specific assumptions in it. >> >> For example, if this driver is used on another platform where different >> PHYs have different reset lines, then one of them (the unlucky one who >> is not probed first) will never get reset. So any form of per-device >> ref-counting is not a portable solution. > > maybe we should also consider Ben's solution: he played with the USB > PHY on his Meson8b board. His approach was to have only one USB PHY > driver instance which exposes two PHYs. > The downside of this: the driver would have to know the offset of the > PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle > the reset using runtime PM without any hacks. > I checked the USB PHY reference driver: it seems that there will be a > new USB PHY with the GXL/GXM SoCs. > So maybe we could live with the assumption that the PHYs are at > consecutive addresses. But isn't that also forcing us to make board-specific assumptions inside the driver. Kevin -- 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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Martin Blumenstingl writes:
> On Fri, Sep 9, 2016 at 10:36 PM, Martin Blumenstingl
> wrote:
>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman wrote:
>>> Martin Blumenstingl writes:
>>>
>>>> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks
>>>> wrote:
>>>>> On 08/09/16 21:42, Kevin Hilman wrote:
>>>>>>
>>>>>> Ben Dooks writes:
>>>>>>
>>>>>>> On 08/09/16 20:52, Martin Blumenstingl wrote:
>>>>>>>>
>>>>>>>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
>>>>>>>>>> + if (IS_ERR(phy)) {
>>>>>>>>>> + dev_err(&pdev->dev, "failed to create PHY\n");
>>>>>>>>>> + return PTR_ERR(phy);
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + if (usb_reset_refcnt++ == 0) {
>>>>>>>>>> + ret = device_reset(&pdev->dev);
>>>>>>>>>> + if (ret) {
>>>>>>>>>> + dev_err(&phy->dev, "Failed to reset USB
>>>>>>>>>> PHY\n");
>>>>>>>>>> + return ret;
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The ref count + reset here looks like something that could/should be
>>>>>>>>> handled in a runtime PM callback.
>>>>>>>>
>>>>>>>> Unfortunately that doesn't work (as Jerome found out) because both
>>>>>>>> PHYs are sharing the same reset line.
>>>>>>>> So if the second PHY would call device_reset then it would also reset
>>>>>>>> the first PHY!
>>>>>>>>
>>>>>>>> There's a comment above the declaration of usb_reset_refcnt which
>>>>>>>> tries to explain this:
>>>>>>>> "The PHYs are sharing a common reset line -> we are only allowed to
>>>>>>>> reset once for all PHYs."
>>>>>>>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>>>>>>>> {" line to make it easier to see?
>>>>>>>>
>>>>>>>
>>>>>>> pm-runtime has refcounting in it. When one of the nodes turns on,
>>>>>>> the pm-runtime will call your driver to say there is a user when
>>>>>>> this first use turns up.
>>>>>>>
>>>>>>> If all the sub-phys turn off and drop their refcount then the driver
>>>>>>> is called to say there are no more users and you can go to sleep.
>>>>>>
>>>>>>
>>>>>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>>>>>
>>>>>> The reason is because there are physically two PHY devices[1]. Those 2
>>>>>> devices will be treated independely by runtime PM, and have separate
>>>>>> use-counting, which means doing what I proposed would cause a reset to
>>>>>> happen when either device was probed.
>>>>>>
>>>>>> So, I think it's OK as it is.
>>>>>
>>>>>
>>>>> Surely you can do pm_runtime_get/put on the phy's parent platform
>>>>> device and do it that way?
>>>> could you please be more specific with that (do you mean pdev->dev.parent)?
>>>> so we would use pm_runtime_{get_sync,put} with the parent, while we
>>>> would still define the runtime_resume in our driver.
>>>
>>> You'd also need to do get/put on the children, but yes, that's what Ben
>>> is suggesting.
>>>
>>> However, the problem with all of the solutions proposed (runtime PM ones
>>> included) is that we're forcing a board-specific design issue (2 devices
>>> sharing a reset line) into a driver that should not have any
>>> board-specific assumptions in it.
>>>
>>> For example, if this driver is used on another platform where different
>>> PHYs have different reset lines, then one of them (the unlucky one who
>>> is not probed first) will never get reset. So any form of per-device
>>> ref-counting is not a portable solution.
>> indeed, so in simple words we would need something like
>> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
>> remember internally if any action has already been executed: if not it
>> does a _reset, _assert or _deassert and otherwise it does nothing.
> for now I've implemented something less hacky: I made the reset
> optional and only specified it for phy0.
That's slightly better, but could misbehave if devices are probed/loaded
in different order? But, that shouldn't be a blocker for the driver.
> During Jerome's tests the reset was not needed, while on my board it's
> required to bring both PHYs up.
> Additionally the USB PHY reference driver does not have any reset
> logic for newer SoCs (GXL), so making the reset optional doesn't sound
> that bad to me.
Agreed.
Kevin
--
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 v6 0/3] da8xx USB PHY platform devices and clocks
Bin Liu writes: > On Tue, Sep 06, 2016 at 02:19:52PM -0500, Bin Liu wrote: >> Hi Greg and Alan, >> >> On Mon, Sep 05, 2016 at 03:00:30PM -0500, David Lechner wrote: >> > Just resending to get these merged into usb. The phy parts of this patch >> > series >> > have already been merged into Linus' tree. >> > >> > I have rebased on 4.8-rc5 but there have not been any changes to these >> > since >> > the last time I submitted. >> > >> > David Lechner (3): >> > usb: ohci-da8xx: Remove code that references mach >> > usb: musb: da8xx: Use devm in probe >> > usb: musb: da8xx: Remove mach code >> >> How do we take care this kinda patch sets, which touch multiple modules? >> I take the two for musb or just acked-by them? > > I am taking patch 2/3 & 3/3 for musb. Greg, can you pick up PATCH 1/3? It's already been ack'd by Alan. Thanks, Kevin -- 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 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
Martin Blumenstingl writes: > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel wrote: [...] >>> I added Philipp and Hans to this thread - maybe they can comment on this. >>> To sum it up, our problem is: >>> - there are two separate USB PHYs on Meson GXBB >>> - both are sharing the same reset line (provided by the reset-meson driver) >>> - during initialization of the PHYs we must only call >>> reset_control_reset(rstc) once (if we do it for the first *and* second >>> PHY then the first PHY gets confused once the second PHY uses the >>> reset because the first PHY's state is reset as well) >> >> If you have an initially asserted reset line and you can enable the >> first module by deasserting the reset via reset_control_deassert (and >> reset_control_assert to signal when the module may be disabled again >> after use), shared resets are for you. >> >> If you need a reset pulse or have no direct control over the reset line, >> (device_reset), the reset framework currently has no solution for this. >> The ugly thing about reset_control_once would be that it can't re-reset >> modules when unloading and reloading driver modules. > > The corresponding reset driver in question is reset-meson, which only > implements reset (assert/deassert are not implemented). However, I > don't know if this is due to hardware design. > I think the hardware implements the latter, but maybe Neil can give > more information here (I currently don't have access to my board so I > cannot test how the hardware actually behaves). It's implemented that way because the hardware only supports a reset pulse. Kevin -- 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 v2 5/6] ARM64: meson-gxbb-p20x: Enable USB Nodes
Martin Blumenstingl writes:
> From: Jerome Brunet
>
> Enable both gxbb USB controller and add a 5V regulator for the OTG port
> VBUS
>
> Signed-off-by: Jerome Brunet
nit: subject should have "ARM64: dts:" prefix.
> ---
> arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 29
>
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> index ce105fe..4493bce 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> @@ -93,6 +93,18 @@
> compatible = "mmc-pwrseq-emmc";
> reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
> };
> +
> + usb_vbus: regulator-usb0-vbus {
nit: I like to use the signal name from the schematics for the node name
(and for regulator-name below). In the schematics, that signal is named
USB_PWR.
> + compatible = "regulator-fixed";
> +
> + regulator-name = "USB0_VBUS";
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> +
> + gpio = <&gpio GPIODV_24 GPIO_ACTIVE_HIGH>;
Please add a comment above this line with the schematic signal name: USB_PWR_EN.
> + enable-active-high;
> + };
> };
Kevin
--
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 v2 1/6] usb: dwc2: add support for Meson8b and GXBB SoCs
On Wed, Sep 14, 2016 at 11:12 AM, John Youn wrote: > On 9/14/2016 9:11 AM, Kevin Hilman wrote: >> Hi John, >> >> Martin Blumenstingl writes: >> >>> From: Jerome Brunet >>> >>> Add compatible strings for amlogic Meson8b and GXBB SoCs with the >>> corresponding configuration parameters. >>> >>> Signed-off-by: Jerome Brunet >>> Signed-off-by: Martin Blumenstingl >> >> Are you OK with adding this platform for v4.9? I know you mentioned >> you're working on new bindings to replace the current way, but since >> that hasn't been posted AFAICT, it would be nice to get this merged now >> and we can help test the new bindings when you're ready. >> >> If you're OK with that, and with your Ack, I can take merge the driver >> changes through the arm-soc tree along with the rest of the DT patches. >> > > Sure. Unfortunately, I wasn't able to complete it for 4.9. > > You can add my acked-by on this: > > Acked-by: John Youn Great, thanks! I assume you're OK with PATCH 2/2 for the bindings as well (at least for now until you rework them?) Kevin -- 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 v2 1/6] usb: dwc2: add support for Meson8b and GXBB SoCs
On Wed, Sep 14, 2016 at 11:26 AM, John Youn wrote: > On 9/14/2016 11:17 AM, Kevin Hilman wrote: >> On Wed, Sep 14, 2016 at 11:12 AM, John Youn wrote: >>> On 9/14/2016 9:11 AM, Kevin Hilman wrote: >>>> Hi John, >>>> >>>> Martin Blumenstingl writes: >>>> >>>>> From: Jerome Brunet >>>>> >>>>> Add compatible strings for amlogic Meson8b and GXBB SoCs with the >>>>> corresponding configuration parameters. >>>>> >>>>> Signed-off-by: Jerome Brunet >>>>> Signed-off-by: Martin Blumenstingl >>>> >>>> Are you OK with adding this platform for v4.9? I know you mentioned >>>> you're working on new bindings to replace the current way, but since >>>> that hasn't been posted AFAICT, it would be nice to get this merged now >>>> and we can help test the new bindings when you're ready. >>>> >>>> If you're OK with that, and with your Ack, I can take merge the driver >>>> changes through the arm-soc tree along with the rest of the DT patches. >>>> >>> >>> Sure. Unfortunately, I wasn't able to complete it for 4.9. >>> >>> You can add my acked-by on this: >>> >>> Acked-by: John Youn >> >> Great, thanks! I assume you're OK with PATCH 2/2 for the bindings as >> well (at least for now until you rework them?) >> >> Kevin >> > > Do you mean PATCH 2/6 from this series? Yes. > It doesn't seem to add or remove any bindings in dwc2 so I'm fine with > it. Thanks. Kevin -- 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 v2 5/6] ARM64: meson-gxbb-p20x: Enable USB Nodes
Kevin Hilman writes:
> Martin Blumenstingl writes:
>
>> From: Jerome Brunet
>>
>> Enable both gxbb USB controller and add a 5V regulator for the OTG port
>> VBUS
>>
>> Signed-off-by: Jerome Brunet
>
> nit: subject should have "ARM64: dts:" prefix.
>
>> ---
>> arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 29
>>
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> index ce105fe..4493bce 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> @@ -93,6 +93,18 @@
>> compatible = "mmc-pwrseq-emmc";
>> reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
>> };
>> +
>> +usb_vbus: regulator-usb0-vbus {
>
> nit: I like to use the signal name from the schematics for the node name
> (and for regulator-name below). In the schematics, that signal is named
> USB_PWR.
>
>
>> +compatible = "regulator-fixed";
>> +
>> +regulator-name = "USB0_VBUS";
>> +regulator-min-microvolt = <500>;
>> +regulator-max-microvolt = <500>;
>> +
>> +gpio = <&gpio GPIODV_24 GPIO_ACTIVE_HIGH>;
>
> Please add a comment above this line with the schematic signal name:
> USB_PWR_EN.
>
For the record... after a chat with Martin on IRC, I just fixed these up
myself locally, so...
Applied,
Kevin
--
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 v2 4/6] ARM64: meson-gxbb: add USB Nodes
Martin Blumenstingl writes: > Add the nodes for the dwc2 USB controller and the related USB PHYs. > Currently we force usb0 to host mode because OTG is currently not > working in our PHY driver. > > Signed-off-by: Martin Blumenstingl > Signed-off-by: Jerome Brunet Applied (after adding a "dts: " to the subject. Kevin -- 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 v2 6/6] ARM64: meson-gxbb-vega-s95: Enable USB Nodes
Martin Blumenstingl writes: > Enable both gxbb USB controller and add a 5V regulator for the OTG port > VBUS > > Signed-off-by: Martin Blumenstingl > Signed-off-by: Jerome Brunet Applied (after adding a "dts: " to the subject. Kevin -- 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 v2 3/6] phy: meson: add USB2 PHY support for Meson8b and GXBB
Kishon Vijay Abraham I writes: > Hi Kevin, > > On Wednesday 14 September 2016 09:36 PM, Kevin Hilman wrote: >> Kishon, >> >> Martin Blumenstingl writes: >> >>> This is a new driver for the USB PHY found in Meson8b and GXBB SoCs. >>> >>> Signed-off-by: Martin Blumenstingl >>> Signed-off-by: Jerome Brunet >>> Tested-by: Kevin Hilman >> >> Will you be picking this up for v4.9? > > It's already late for 4.9. Generally send pull request to Greg around -rc6. > This can go only in 4.10. That's fine. Does that mean you have it queued someplace? I don't see it in any of your branches. Kevin -- 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 v2 2/6] Documentation: dt-bindings: Add documentation for the Meson USB2 PHYs
Rob Herring writes:
> On Sun, Sep 11, 2016 at 03:41:07PM +0200, Martin Blumenstingl wrote:
>> Add the documentation for the bindings for the Meson8b and GXBB USB2
>> PHYs.
>>
>> Signed-off-by: Martin Blumenstingl
>> ---
>> .../devicetree/bindings/phy/meson-usb2-phy.txt | 27
>> ++
>> 1 file changed, 27 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>> b/Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>> new file mode 100644
>> index 000..9da5ea2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/meson-usb2-phy.txt
>> @@ -0,0 +1,27 @@
>> +* Amlogic USB2 PHY
>> +
>> +Required properties:
>> +- compatible: Depending on the platform this should be one of:
>> +"amlogic,meson8b-usb2-phy"
>> +"amlogic,meson-gxbb-usb2-phy"
>> +- reg: The base address and length of the registers
>> +- #phys-cells: should be 0 (see phy-bindings.txt in this directory)
>> +- clocks: phandle and clock identifier for the phy clocks
>> +- clock-names: "usb_general" and "usb"
>> +
>> +Optional properties:
>> +- resets: reference to the reset controller
>> +- phy-supply: see phy-bindings.txt in this directory
>> +
>> +
>> +Example:
>> +
>> +usb0_phy: usb_phy@0 {
>
> usb-phy@0
>
> With that,
>
> Acked-by: Rob Herring
>
Oops, I had already merged this one.
Martin, can you send a fixup patch for the underscores?
Thanks,
Kevin
--
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] ARM64: dts: meson-gxbb-odroidc2: Enable USB Nodes
Brian Kim writes:
> Enable both gxbb USB controller and add a 5V regulator for the OTG port
> VBUS
>
> Signed-off-by: Brian Kim
Thanks for the patch.
In the future, please state what branch the patch should apply to when
not using mainline. Because of the sd_emmc nodes in your patch, I could
tell that it was based on my integ branch so was able to figure it out,
but it's very helpful to maintainers if you state the branch and/or any
dependencies explicity.
> ---
> .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 29
> ++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index 8d89edc..997c671 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -64,6 +64,18 @@
> reg = <0x0 0x0 0x0 0x8000>;
> };
>
> + usb_pwr: regulator-usb-pwrs {
minor nit: since this is specific to the OTG part, can you call this
usb_otg_pwr? ...
> + compatible = "regulator-fixed";
> +
> + regulator-name = "USB_PWR";
... and rename this also?
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> +
> + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
Thanks
Kevin
--
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 v6 1/3] usb: ohci-da8xx: Remove code that references mach
Hi Greg,
On Mon, Sep 5, 2016 at 10:00 PM, David Lechner wrote:
> Including mach/* is frowned upon in device drivers, so get rid of it.
>
> This replaces usb20_clk and code that pokes CFGCHIP2 with a proper phy
> driver.
>
> Signed-off-by: David Lechner
> Acked-by: Alan Stern
Can you pick up this patch?
Patches 2 & 3 from this series were picked up by Bin for musb, but
this one is still outstanding.
Thanks,
Kevin
> ---
> drivers/usb/host/Kconfig | 1 +
> drivers/usb/host/ohci-da8xx.c | 102
> +++---
> 2 files changed, 56 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 2e710a4..1f0cdab8 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -482,6 +482,7 @@ config USB_OHCI_HCD_DAVINCI
> bool "OHCI support for TI DaVinci DA8xx"
> depends on ARCH_DAVINCI_DA8XX
> depends on USB_OHCI_HCD=y
> + select PHY_DA8XX_USB
> default y
> help
> Enables support for the DaVinci DA8xx integrated OHCI
> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
> index e5c33bc..3656d7c 100644
> --- a/drivers/usb/host/ohci-da8xx.c
> +++ b/drivers/usb/host/ohci-da8xx.c
> @@ -15,58 +15,50 @@
> #include
> #include
> #include
> -
> -#include
> +#include
> #include
>
> #ifndef CONFIG_ARCH_DAVINCI_DA8XX
> #error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX."
> #endif
>
> -#define CFGCHIP2 DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG)
> -
> static struct clk *usb11_clk;
> -static struct clk *usb20_clk;
> +static struct phy *usb11_phy;
>
> /* Over-current indicator change bitmask */
> static volatile u16 ocic_mask;
>
> -static void ohci_da8xx_clock(int on)
> +static int ohci_da8xx_enable(void)
> {
> - u32 cfgchip2;
> -
> - cfgchip2 = __raw_readl(CFGCHIP2);
> - if (on) {
> - clk_enable(usb11_clk);
> -
> - /*
> -* If USB 1.1 reference clock is sourced from USB 2.0 PHY, we
> -* need to enable the USB 2.0 module clocking, start its PHY,
> -* and not allow it to stop the clock during USB 2.0 suspend.
> -*/
> - if (!(cfgchip2 & CFGCHIP2_USB1PHYCLKMUX)) {
> - clk_enable(usb20_clk);
> -
> - cfgchip2 &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
> - cfgchip2 |= CFGCHIP2_PHY_PLLON;
> - __raw_writel(cfgchip2, CFGCHIP2);
> -
> - pr_info("Waiting for USB PHY clock good...\n");
> - while (!(__raw_readl(CFGCHIP2) & CFGCHIP2_PHYCLKGD))
> - cpu_relax();
> - }
> + int ret;
>
> - /* Enable USB 1.1 PHY */
> - cfgchip2 |= CFGCHIP2_USB1SUSPENDM;
> - } else {
> - clk_disable(usb11_clk);
> - if (!(cfgchip2 & CFGCHIP2_USB1PHYCLKMUX))
> - clk_disable(usb20_clk);
> + ret = clk_prepare_enable(usb11_clk);
> + if (ret)
> + return ret;
>
> - /* Disable USB 1.1 PHY */
> - cfgchip2 &= ~CFGCHIP2_USB1SUSPENDM;
> - }
> - __raw_writel(cfgchip2, CFGCHIP2);
> + ret = phy_init(usb11_phy);
> + if (ret)
> + goto err_phy_init;
> +
> + ret = phy_power_on(usb11_phy);
> + if (ret)
> + goto err_phy_power_on;
> +
> + return 0;
> +
> +err_phy_power_on:
> + phy_exit(usb11_phy);
> +err_phy_init:
> + clk_disable_unprepare(usb11_clk);
> +
> + return ret;
> +}
> +
> +static void ohci_da8xx_disable(void)
> +{
> + phy_power_off(usb11_phy);
> + phy_exit(usb11_phy);
> + clk_disable_unprepare(usb11_clk);
> }
>
> /*
> @@ -92,7 +84,9 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
>
> dev_dbg(dev, "starting USB controller\n");
>
> - ohci_da8xx_clock(1);
> + result = ohci_da8xx_enable();
> + if (result < 0)
> + return result;
>
> /*
> * DA8xx only have 1 port connected to the pins but the HC root hub
> @@ -101,8 +95,10 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
> ohci->num_ports = 1;
>
> result = ohci_init(ohci);
> - if (result < 0)
> + if (result < 0) {
> + ohci_da8xx_disable();
> return result;
> + }
>
> /*
> * Since we're providing a board-specific root hub port power control
> @@ -129,7 +125,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
> static void ohci_da8xx_stop(struct usb_hcd *hcd)
> {
> ohci_stop(hcd);
> - ohci_da8xx_clock(0);
> + ohci_da8xx_disable();
> }
>
> static int ohci_da8xx_start(struct usb_hcd *hcd)
> @@ -301,12 +297,18 @@ static int usb_hcd_da8xx_probe(const struct hc_driver
> *driver,
> return -ENODEV;
>
Re: [PATCH v2] ARM64: dts: meson-gxbb-odroidc2: Enable USB Nodes
Brian Kim writes: > Enable both gxbb USB controller and add a 5V regulator for the OTG port > VBUS > > Signed-off-by: Brian Kim > --- > This patch was written on Kevin Hilman's repository[1] and branch[2]: > [1] https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git > [2] v4.8/integ Applied to the v4.10/dt64 branch, and will be included shortly in the integration branch. Kevin -- 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: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime
Alexandre Bailon writes:
> Currently, MUSB DA8xx glue driver doesn't have PM runtime support.
> Because the CPPI 4.1 is using the same clock as MUSB DA8xx and
> CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime
> to the DA8xx glue driver in order to let the CPPI 4.1 driver manage
> the clock by using PM runtime.
>
> Signed-off-by: Alexandre Bailon
Dumb question: even if the clock is shared with cppi4, doesn't the
use-couting in the clock API handle it so that things should function
fine even without this patch?
Some other comments/questions below...
> ---
> drivers/usb/musb/da8xx.c | 27 ---
> 1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index ed28afd..89e12f6 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -30,7 +30,6 @@
> */
>
> #include
> -#include
> #include
> #include
> #include
> @@ -86,7 +85,6 @@ struct da8xx_glue {
> struct device *dev;
> struct platform_device *musb;
> struct platform_device *usb_phy;
> - struct clk *clk;
> struct phy *phy;
> };
>
> @@ -376,11 +374,7 @@ static int da8xx_musb_init(struct musb *musb)
>
> musb->mregs += DA8XX_MENTOR_CORE_OFFSET;
>
> - ret = clk_prepare_enable(glue->clk);
> - if (ret) {
> - dev_err(glue->dev, "failed to enable clock\n");
> - return ret;
> - }
> + pm_runtime_get_sync(musb->controller->parent);
>
> /* Returns zero if e.g. not clocked */
> rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG);
> @@ -423,7 +417,7 @@ static int da8xx_musb_init(struct musb *musb)
> err_phy_power_on:
> phy_exit(glue->phy);
> fail:
> - clk_disable_unprepare(glue->clk);
> + pm_runtime_put(musb->controller->parent);
> return ret;
> }
>
> @@ -435,7 +429,7 @@ static int da8xx_musb_exit(struct musb *musb)
>
> phy_power_off(glue->phy);
> phy_exit(glue->phy);
> - clk_disable_unprepare(glue->clk);
> + pm_runtime_put(musb->controller->parent);
>
> usb_put_phy(musb->xceiv);
>
> @@ -519,7 +513,6 @@ static int da8xx_probe(struct platform_device *pdev)
> struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> struct da8xx_glue *glue;
> struct platform_device_info pinfo;
> - struct clk *clk;
> struct device_node *np = pdev->dev.of_node;
> int ret;
>
> @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev)
> if (!glue)
> return -ENOMEM;
>
> - clk = devm_clk_get(&pdev->dev, "usb20");
> - if (IS_ERR(clk)) {
> - dev_err(&pdev->dev, "failed to get clock\n");
> - return PTR_ERR(clk);
> - }
Something isn't quite right here.
This clk_get uses a con_id "usb20", but when converting to runtime PM,
we rely on the pm_clk layer (used on davinci for runtime PM) to do clock
lookups by the default con_ids. I guess this still probably works
because we fallback to the NULL con_id.
> glue->phy = devm_phy_get(&pdev->dev, "usb-phy");
> if (IS_ERR(glue->phy)) {
> if (PTR_ERR(glue->phy) != -EPROBE_DEFER)
> @@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device *pdev)
> }
>
> glue->dev = &pdev->dev;
> - glue->clk = clk;
>
> if (IS_ENABLED(CONFIG_OF) && np) {
> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> @@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device *pdev)
> pinfo.data = pdata;
> pinfo.size_data = sizeof(*pdata);
>
> + pm_runtime_enable(&pdev->dev);
> +
> glue->musb = platform_device_register_full(&pinfo);
> ret = PTR_ERR_OR_ZERO(glue->musb);
> if (ret) {
> @@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device *pdev)
>
> platform_device_unregister(glue->musb);
> usb_phy_generic_unregister(glue->usb_phy);
> + pm_runtime_disable(&pdev->dev);
>
> return 0;
> }
> @@ -616,7 +605,7 @@ static int da8xx_suspend(struct device *dev)
> ret = phy_power_off(glue->phy);
> if (ret)
> return ret;
> - clk_disable_unprepare(glue->clk);
> + pm_runtime_put_sync(dev);
I'm a bit lots in the MUSB layering here, but does this 'dev' correspond
to musb->controller->parent?
Kevin
> return 0;
> }
> @@ -626,7 +615,7 @@ static int da8xx_resume(struct device *dev)
> int ret;
> struct da8xx_glue *glue = dev_get_drvdata(dev);
>
> - ret = clk_prepare_enable(glue->clk);
> + ret = pm_runtime_get_sync(dev);
> if (ret)
> return ret;
> return phy_power_on(glue->phy);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...
Re: [PATCH 4/4] PM: Eliminate CONFIG_PM_RUNTIME
"Rafael J. Wysocki" writes: [...] > Fixed up patch is appended, thanks! > > > --- > From: Rafael J. Wysocki > Subject: PM: Eliminate CONFIG_PM_RUNTIME > > Having switched over all of the users of CONFIG_PM_RUNTIME to use > CONFIG_PM directly, turn the latter into a user-selectable option > and drop the former entirely from the tree. > > Signed-off-by: Rafael J. Wysocki > Reviewed-by: Ulf Hansson Acked-by: Kevin Hilman I assume you're planning to get this in early in the v3.19-rc cycle, correct? That way we can hopefully avoid conflicts with the various defconfig changes we're taking through the arm-soc tree. Also, thanks for taking care of all the tree-wide changes. This is a great change. Kevin -- 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: [RESEND PATCH 2/2] ARM: at91/dt: update udc compatible strings
Nicolas Ferre writes: > From: Boris Brezillon > > at91sam9g45, at91sam9x5 and sama5 SoCs should not use > "atmel,at91sam9rl-udc" for their USB device compatible property since > this compatible is attached to a specific hardware bug fix. > > Signed-off-by: Boris Brezillon > Acked-by: Alexandre Belloni > Tested-by: Bo Shen > Acked-by: Nicolas Ferre > Cc: #4.0+ > --- > Hi, > > This patch was forgotten while dealing with the series "usb: atmel_usba_udc: > Rework errata handling". This patch and the previous one should be added to > mainline as fixes, the soonest. In fact, the errata handling is now broken > because of this desynchronization. > > We'll try to queue it during 4.2-rc phase for inclusion in arm-soc tree. I've picked both of these up for the arm-soc/late branch which I'll try to get in before -rc1, if not it will make it for -rc2. Thanks, Kevin -- 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: [RFT 0/3] usb: usb3503: Fix probing on Arndale board (missing phy)
Hi Krzystof,
Krzysztof Kozlowski writes:
> Introduction
>
> This patchset tries to fix probing of usb3503 on Arndale board
> if the Samsung PHY driver is probed later (or built as a module).
>
> *The patchset was not tested on Arndale board.*
> I don't have that board. Please test it and say if the usb3503 deferred probe
> works fine and the issue is solved.
FYI... I built this series on top of next-20151009 and using
exynos_defconfig. I booted it on my arndale, and I still don't see the
networking come up. Full boot log attached.
Kevin
Connected to arndale console [channel connected] (~$quit to exit)
(user:khilman) is already connected
# PYBOOT: console: connected.
~$hardreset
Command(arndale console)> hardreset
(user:khilman) Reboot arndale
U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250
CPU:Exynos5250@1000MHz
Board: for ARNDALE5250
I2C: ready
DRAM: 2 GiB
WARNING: Caches not enabled
Checking Boot Mode ... SDMMC
MMC: EXYNOS DWMMC: 0, EXYNOS DWMMC: 1, EXYNOS DWMMC: 2
In:serial
Out: serial
Err: serial
Net: No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 1 Ethernet Device(s) found
Hit any key to stop autoboot:
# PYBOOT: u-boot: taking control.
3 0
ARNDALE5250 #
ARNDALE5250 # version
version
U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250
arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.2-1ubuntu1) 4.7.2
GNU ld (GNU Binutils for Ubuntu) 2.22.90.20120919
ARNDALE5250 # setenv bootargs console=tty0 console=ttySAC2,115200n8 rw
root=/dev/mmcblk1p3 rootwait rootfstype=ext4
setenv bootargs console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3
rootwait rootfstype=ext4
ARNDALE5250 # setenv netargs 'setenv bootargs ${bootargs}
"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:::none:192.168.1.3"'
setenv netargs 'setenv bootargs ${bootargs}
"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:::none:192.168.1.3"'
ARNDALE5250 # if test -n ${initenv}; then run initenv; fi
if test -n ${initenv}; then run initenv; fi
ARNDALE5250 # if test -n ${preboot}; then run preboot; fi
if test -n ${preboot}; then run preboot; fi
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 1 Ethernet Device(s) found
ARNDALE5250 # setenv autoload no; setenv autoboot no
setenv autoload no; setenv autoboot no
ARNDALE5250 #dhcp
dhcp
Waiting for Ethernet connection... done.
BOOTP broadcast 1
DHCP client bound to address 192.168.1.167
ARNDALE5250 #setenv serverip 192.168.1.2
setenv serverip 192.168.1.2
ARNDALE5250 # if test -n ${netargs}; then run netargs; fi
if test -n ${netargs}; then run netargs; fi
ARNDALE5250 # tftp 0x4100 192.168.1.2:tmp/arndale-o13inu/zImage
tftp 0x4100 192.168.1.2:tmp/arndale-o13inu/zImage
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.167
Filename 'tmp/arndale-o13inu/zImage'.
Load address: 0x4100
Loading: *#
#
#
#
###
done
Bytes transferred = 4209056 (4039a0 hex)
ARNDALE5250 # tftp 0x4200
192.168.1.2:tmp/arndale-o13inu/initrd-vFFjKf.cpio.gz
tftp 0x4200 192.168.1.2:tmp/arndale-o13inu/initrd-vFFjKf.cpio.gz
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.167
Filename 'tmp/arndale-o13inu/initrd-vFFjKf.cpio.gz'.
Load address: 0x4200
Loading: *#
#
#
#
#
done
Bytes transferred = 5001043 (4c4f53 hex)
ARNDALE5250 #tftp 0x41f0 192.168.1.2:tmp/arndale-o13inu/tmp_2g_yE.dtb
tftp 0x41f0 192.168.1.2:tmp/arndale-o13inu/tmp_2g_yE.dtb
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.167
Filename 'tmp/arndale-o13inu/tmp_2g_yE.dtb'.
Load address: 0x41f0
Loading: *###
done
Bytes transferred = 42207 (a4df hex)
ARNDALE5250 # printenv bootargs
printenv bootargs
bootargs=console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait
rootfstype=ext4
ip=192.168.1.167:192.168.1.2:192.168.1.254:255.255.255.0:::none:192.168.1.3
ARNDALE5250 #bootz 0x4100 - 0x41f0
# PYBOOT:
Re: [PATCH] usb: dwc3: meson-g12a: fix suspend resume regulator unbalanced disables
Neil Armstrong writes:
> When going in suspend, in Device mode, then resuming back leads
> to the following:
>
> unbalanced disables for USB_PWR_EN
> WARNING: CPU: 0 PID: 163 at ../drivers/regulator/core.c:2590
> _regulator_disable+0x104/0x180
> Hardware name: Amlogic Meson G12A U200 Development Board (DT)
> [...]
> pc : _regulator_disable+0x104/0x180
> lr : _regulator_disable+0x104/0x180
> [...]
> Call trace:
> _regulator_disable+0x104/0x180
> regulator_disable+0x40/0x78
> dwc3_meson_g12a_otg_mode_set+0x84/0xb0
> dwc3_meson_g12a_irq_thread+0x58/0xb8
> irq_thread_fn+0x28/0x80
> irq_thread+0x118/0x1b8
> kthread+0xf4/0x120
> ret_from_fork+0x10/0x18
>
> This disables the regulator if enabled on suspend, and the reverse on
> resume.
>
> Fixes: c3376f72 ("usb: dwc3: Add Amlogic G12A DWC3 glue")
> Reported-by: Kevin Hilman
> Signed-off-by: Neil Armstrong
Tested-by: Kevin Hilman
