Re: [PATCH 1/3] i2c: exynos5: add High Speed I2C controller driver
Hi,
On Tue, Nov 27, 2012 at 06:30:34PM +0530, Naveen Krishna Chatradhi wrote:
> diff --git a/drivers/i2c/busses/i2c-exynos5.c
> b/drivers/i2c/busses/i2c-exynos5.c
> new file mode 100644
> index 000..5983aa9
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-exynos5.c
> @@ -0,0 +1,758 @@
> +/* linux/drivers/i2c/busses/i2c-exynos5.c
> + *
> + * Copyright (C) 2012 Samsung Electronics Co., Ltd.
> + *
> + * Exynos5 series High Speed I2C controller driver
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
are you sure you want to include this header ? Just making sure...
> +#include "i2c-exynos5.h"
> +
> +#define HSI2C_POLLING 0
> +#define HSI2C_FAST_SPD 0
> +#define HSI2C_HIGH_SPD 1
> +
> +/* Max time to wait for bus to become idle after a xfer */
> +#define EXYNOS5_I2C_TIMEOUT (msecs_to_jiffies(1000))
> +
> +struct exynos5_i2c {
> + unsigned intsuspended:1;
> +
> + struct i2c_msg *msg;
> + struct completion msg_complete;
> + unsigned intmsg_byte_ptr;
> +
> + unsigned intirq;
> +
> + void __iomem*regs;
> + struct clk *clk;
> + struct device *dev;
> + struct resource *ioarea;
you don't need to save the struct resource here, see more below.
> + struct i2c_adapter adap;
I would put this as the first memeber of the structure, so that a
container_of() gets optmized into a cast.
> + unsigned intbus_number;
> + unsigned intspeed_mode;
> + unsigned intfast_speed;
> + unsigned inthigh_speed;
> + int operation_mode;
> + int gpios[2];
> +};
> +
> +static struct platform_device_id exynos5_driver_ids[] = {
> + {
> + .name = "exynos5-hs-i2c",
> + .driver_data= 0,
no need to zero-initialize.
> + }, { },
> +};
> +MODULE_DEVICE_TABLE(platform, exynos5_driver_ids);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id exynos5_i2c_match[] = {
> + { .compatible = "samsung,exynos5-hs-i2c", .data = (void *)0 },
remove the (void *)0 initialization. the variable is static and will be
zero-initialized.
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos5_i2c_match);
> +#endif
> +
> +static inline void dump_i2c_register(struct exynos5_i2c *i2c)
> +{
> + dev_dbg(i2c->dev, "Register dump(%d) :\n %x\n %x\n %x\n %x\n"
> + " %x\n %x\n %x\n %x\n %x\n"
> + " %x\n %x\n %x\n %x\n %x\n"
> + " %x\n %x\n %x\n %x\n %x\n"
> + " %x\n %x\n %x\n %x\n %x\n"
> + , i2c->suspended
> + , readl(i2c->regs + HSI2C_CTL)
> + , readl(i2c->regs + HSI2C_FIFO_CTL)
> + , readl(i2c->regs + HSI2C_TRAILIG_CTL)
> + , readl(i2c->regs + HSI2C_CLK_CTL)
> + , readl(i2c->regs + HSI2C_CLK_SLOT)
> + , readl(i2c->regs + HSI2C_INT_ENABLE)
> + , readl(i2c->regs + HSI2C_INT_STATUS)
> + , readl(i2c->regs + HSI2C_ERR_STATUS)
> + , readl(i2c->regs + HSI2C_FIFO_STATUS)
> + , readl(i2c->regs + HSI2C_TX_DATA)
> + , readl(i2c->regs + HSI2C_RX_DATA)
> + , readl(i2c->regs + HSI2C_CONF)
> + , readl(i2c->regs + HSI2C_AUTO_CONFING)
> + , readl(i2c->regs + HSI2C_TIMEOUT)
> + , readl(i2c->regs + HSI2C_MANUAL_CMD)
> + , readl(i2c->regs + HSI2C_TRANS_STATUS)
> + , readl(i2c->regs + HSI2C_TIMING_HS1)
> + , readl(i2c->regs + HSI2C_TIMING_HS2)
> + , readl(i2c->regs + HSI2C_TIMING_HS3)
> + , readl(i2c->regs + HSI2C_TIMING_FS1)
> + , readl(i2c->regs + HSI2C_TIMING_FS2)
> + , readl(i2c->regs + HSI2C_TIMING_FS3)
> + , readl(i2c->regs + HSI2C_TIMING_SLA)
> + , readl(i2c->regs + HSI2C_ADDR));
> +}
I would make this dev_vdbg() since this is really only needed when
something is terribly wrong. In fact, I don't like these big register
dump functions in code. To me that's something that should be done on
debugfs.
Also, your comma character placement is a bit weird. Usually it goes to
end of the line, not to the beginning of the next.
> +static inline void exynos5_i2c_stop(struct exynos5_i2c *i2c)
> +{
> + writel(0, i2c->regs + HSI2C_INT_ENABLE);
> +
> + complete(&i2c->msg_complete);
> +}
> +
> +static inline void exynos5_disable_irq(struct exynos5_i2c *i2c)
> +{
> + unsigned long tmp = readl(i2c->regs + HSI2C_INT_STATUS);
> +
> + writel(tmp, i2c->regs + HSI2C_INT_
Re: Linux 3.8.3
Hi, On Sun, Mar 17, 2013 at 10:00:08PM +0800, Dongsheng Song wrote: > On Fri, Mar 15, 2013 at 2:47 AM, Greg KH wrote: > > > > I'm announcing the release of the 3.8.3 kernel. > > > > All users of the 3.8 kernel series must upgrade. > > > > The updated 3.8.y git tree can be found at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > > linux-3.8.y > > and can be browsed at the normal kernel.org git web browser: > > > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary > > > > thanks, > > > > greg k-h > > Hi, > > “make ARCH=arm menuconfig” no complains, but "make ARCH=arm > CROSS_COMPILE=arm-linux-gnueabihf-" failed: > > $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > make[1]: `include/generated/mach-types.h' is up to date. > CALLscripts/checksyscalls.sh > CHK include/generated/compile.h > CHK kernel/config_data.h > Kernel: arch/arm/boot/Image is ready > Kernel: arch/arm/boot/zImage is ready > Building modules, stage 2. > MODPOST 1904 modules > ERROR: "flush_tlb_kernel_range" [drivers/staging/zsmalloc/zsmalloc.ko] > undefined! > make[1]: *** [__modpost] Error 1 > make: *** [modules] Error 2 > > Please find the attachment with .config.gz file. linux-arm-kernel might have more information about this. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb host: Faraday FUSBH200 HCD driver.
Hi, (don't top-post) On Mon, Mar 18, 2013 at 06:06:18PM +0800, Yuan-Hsin Chen wrote: > Hi, > > I tried to modify fusbh200 hcd driver following ehci-platform.c. > However, the register definition of fusbh200 is partially incompatible > to ehci. For fusbh200, only the elements between "command" and > "async_next" in struct ehci_regs are consistent with ehci which means > it would cause copious modification and duplication of ehci hcd > driver. For example, there is no "configured_flag" register in > fusbh200 controller, yet, ehci hcd driver accesses "configured_flag" > in function ehci_run which would cause compile errors. Therefore, > maybe my first patch which refers to oxu210hp-hcd is a better > solution? why don't you just add a quirk flag to ehci struct so that it knows it shouldn't access CONFIGFLAG register when that's non-existent ? There are only 5 uses of configured_flag in ehci-hcd.c, so it should be easy to wrap that around a flag check. Alan, would you have a better idea ? Looks like this is a non-standard EHCI implementation. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb host: Faraday FUSBH200 HCD driver.
Hi, On Mon, Mar 18, 2013 at 10:16:56AM -0400, Alan Stern wrote: > > > it would cause copious modification and duplication of ehci hcd > > > driver. For example, there is no "configured_flag" register in > > > fusbh200 controller, yet, ehci hcd driver accesses "configured_flag" > > > in function ehci_run which would cause compile errors. Therefore, > > > maybe my first patch which refers to oxu210hp-hcd is a better > > > solution? > > > > why don't you just add a quirk flag to ehci struct so that it knows it > > shouldn't access CONFIGFLAG register when that's non-existent ? > > > > There are only 5 uses of configured_flag in ehci-hcd.c, so it should be > > easy to wrap that around a flag check. > > Two of those uses turn configured_flag on and two of them turn it off. > However, one of the uses tests its value (the first one in > ehci_resume). How would that be handled if configured_flag doesn't > exist? perhaps they don't use it because they have a root-hub TT ? In that case you assume EHCI *always* owns the port. Would that work ? -- balbi signature.asc Description: Digital signature
Re: linux-next: manual merge of the akpm tree with the usb-gadget tree
Hi,
On Tue, Mar 19, 2013 at 02:39:22PM +1100, Stephen Rothwell wrote:
> Hi Andrew,
>
> Today's linux-next merge of the akpm tree got a conflict in
> drivers/usb/gadget/amd5536udc.c between commit 6c39d90393a2 ("usb:
> gadget: amd5536udc: remove unnecessary initializations") from the tree
Looks like there's a small bug in your scripts: "from the tree" misses
"usb-gadget".
> and commit "drivers/usb/gadget/amd5536udc.c: avoid calling dma_pool_create()
> with NULL dev" from the akpm tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
>
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
>
> diff --cc drivers/usb/gadget/amd5536udc.c
> index f52dcfe,142616a..000
> --- a/drivers/usb/gadget/amd5536udc.c
> +++ b/drivers/usb/gadget/amd5536udc.c
> @@@ -3232,6 -3235,12 +3232,10 @@@ static int udc_pci_probe
> pci_set_master(pdev);
> pci_try_set_mwi(pdev);
>
> + dev->phys_addr = resource;
> + dev->irq = pdev->irq;
> + dev->pdev = pdev;
> -dev->gadget.dev.parent = &pdev->dev;
> -dev->gadget.dev.dma_mask = pdev->dev.dma_mask;
> +
fix looks alright, thank you
--
balbi
signature.asc
Description: Digital signature
Re: USB: simplify clock lookup for mv ehci/otg/udc
Hi, On Tue, Mar 19, 2013 at 02:44:46PM +0100, Arnd Bergmann wrote: > While going over the suggested changes for the ehci-mv separation patch, > we noticed that the driver uses a variable number of clock names it gets > passed from the platform code, which is highly unusual behavior and adds > a lot of extra complexity. > > Even though there is a comment in the mv_udc driver stating that some SoCs > require multiple clocks, none of the code in the upstream kernel provides > more than one, and even if an out of tree SoC port needs multiple clocks, > it is still wrong to pass them them through platform data, since they are > a property of the device, not a property of the platform. > > This patch attempts to clean up the situation by turning the one clock > that is passed into the ehci/udc/otg devices into an anomymous one and > removing the clkname array from the platform data. Another simplification > is to always call clk_prepare_enable/clk_disable_unprepare directly, > since that is a valid operation on a NULL clk pointer if the platform > has not attacked a clk to the device. > > Signed-off-by: Arnd Bergmann needs to be rebased on my -next branch. Also, it would be really good if dependencies between drivers and arch code would be cut to a minimum. -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/7] ARM: tegra: update device trees for USB binding rework
On Wed, Mar 20, 2013 at 05:47:46PM +0530, Venu Byravarasu wrote:
> > -Original Message-
> > From: kishon [mailto:kis...@ti.com]
> > Sent: Wednesday, March 20, 2013 4:53 PM
> > To: Venu Byravarasu
> > Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu;
> > ba...@ti.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > swar...@wwwdotorg.org; linux-te...@vger.kernel.org; devicetree-
> > disc...@lists.ozlabs.org
> > Subject: Re: [PATCH 2/7] ARM: tegra: update device trees for USB binding
> > rework
> >
> > Hi,
> >
> > On Monday 18 March 2013 05:59 PM, Venu Byravarasu wrote:
> > > This patch updates all Tegra board files so that they contain all the
> > > properties required by the updated USB DT binding. Note that this patch
> > > only adds the new properties and does not yet remove the old properties,
> > > in order to maintain bisectability. The old properties will be removed
> > > once the driver has been updated to assume the new bindings.
> > >
> > > Signed-off-by: Venu Byravarasu
> > > ---
> > > arch/arm/boot/dts/tegra20-colibri-512.dtsi |4 +++
> > > arch/arm/boot/dts/tegra20-harmony.dts |8 +++---
> > > arch/arm/boot/dts/tegra20-iris-512.dts |4 +++
> > > arch/arm/boot/dts/tegra20-paz00.dts|8 +++---
> > > arch/arm/boot/dts/tegra20-seaboard.dts | 13 +++---
> > > arch/arm/boot/dts/tegra20-trimslice.dts| 12 +++---
> > > arch/arm/boot/dts/tegra20-ventana.dts |7 +++--
> > > arch/arm/boot/dts/tegra20.dtsi | 32
> > > +--
> > > 8 files changed, 57 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> > b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> > > index cb73e62..af5a7ae 100644
> > > --- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> > > +++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> > > @@ -443,6 +443,10 @@
> > > nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
> > > };
> > >
> > > + usb-phy@c5004000 {
> > This node doesn't have a *reg* property. So "@c5004000" is not needed.
> > This comment applies to all the nodes which doesn't have *reg* property.
>
> Thanks Kishon for the comments.
> As I've 3 usb-phy DT nodes, how to differentiate between them if I remove
> this @Address ?
then add reg property :-)
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCHv2 1/1] usb: f_rndis: Avoid to use ERROR macro if cdev can be null
On Tue, Mar 19, 2013 at 02:22:56PM +0100, Michal Nazarewicz wrote: > On Tue, Mar 19 2013, oskar.and...@sonymobile.com wrote: > > The udc_irq service runs the isr_tr_complete_handler which in turn > > "nukes" the endpoints, including a call to rndis_response_complete, > > if appropriate. If the rndis_msg_parser fails here, an error will > > be printed using a dev_err call (through the ERROR() macro). > > > > However, if the usb cable was just disconnected the device (cdev) > > might not be available and will be null. Since the dev_err macro will > > dereference the cdev pointer we get a null pointer exception. > > > > Reviewed-by: Radovan Lekanovic > > Signed-off-by: Truls Bengtsson > > Signed-off-by: Oskar Andero > > Acked-by: Michal Nazarewicz > > I think this is the best solution. Adding if statements around it would > just add noise. alright, please re-send without RFC tag and with Michal's acked-by so I can apply. -- balbi signature.asc Description: Digital signature
Re: [PATCH 0/5] usb: musb/otg: cleanup and fixes
On Wed, Mar 13, 2013 at 02:17:05PM +0530, Kishon Vijay Abraham I wrote: > This series has some misc cleanup and fixes. The fix solves the cold > plug issue in omap3 which some have reported. Developed these patches on > Linux 3.9-rc2 after applying > http://www.spinics.net/lists/linux-usb/msg81563.html > (Grazvydas Ignotas patch series) > > Tested for g_zero enumeration in pandaboard and beagleboard in both > cold plug and hot plug case. Any kind of testing for this series is welcome. > > Kishon Vijay Abraham I (5): > usb: musb: omap: remove the check before calling otg_set_vbus > usb: musb: omap: always glue have the correct vbus/id status > usb: otg: twl4030: use devres API for regulator get and request irq > usb: musb: omap: add usb_phy_init in omap2430_musb_init > usb: otg: twl4030: fix cold plug on OMAP3 > > drivers/usb/musb/omap2430.c | 22 ++ > drivers/usb/otg/twl4030-usb.c | 35 --- > 2 files changed, 22 insertions(+), 35 deletions(-) this needs to be rebased against my 'next' branch. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 01/21] usb: phy: nop: Add some parameters to platform data
On Wed, Mar 20, 2013 at 05:44:40PM +0200, Roger Quadros wrote: > Add clk_rate parameter to platform data. If supplied, the > NOP phy driver will program the clock to that rate during probe. > > Also add 2 flags, needs_vcc and needs_reset. > If the flag is set and the regulator couldn't be found > then the driver will bail out with -EPROBE_DEFER. > > Signed-off-by: Roger Quadros > Acked-by: Felipe Balbi Hi Tony, maybe you might prefer to merge commit 1f0972f from my next branch which is exactly this patch. Basically, if you: $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next $ git merge 1f0972f you get $SUBJECT and can apply the others without fear of conflicts later. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 0/5] usb: musb/otg: cleanup and fixes
On Wed, Mar 20, 2013 at 08:52:36PM +0530, kishon wrote: > Felipe, > > On Wednesday 20 March 2013 06:42 PM, Felipe Balbi wrote: > >On Wed, Mar 13, 2013 at 02:17:05PM +0530, Kishon Vijay Abraham I wrote: > >>This series has some misc cleanup and fixes. The fix solves the cold > >>plug issue in omap3 which some have reported. Developed these patches on > >>Linux 3.9-rc2 after applying > >>http://www.spinics.net/lists/linux-usb/msg81563.html > >>(Grazvydas Ignotas patch series) > >> > >>Tested for g_zero enumeration in pandaboard and beagleboard in both > >>cold plug and hot plug case. Any kind of testing for this series is welcome. > >> > >>Kishon Vijay Abraham I (5): > >> usb: musb: omap: remove the check before calling otg_set_vbus > >> usb: musb: omap: always glue have the correct vbus/id status > >> usb: otg: twl4030: use devres API for regulator get and request irq > >> usb: musb: omap: add usb_phy_init in omap2430_musb_init > >> usb: otg: twl4030: fix cold plug on OMAP3 > >> > >> drivers/usb/musb/omap2430.c | 22 ++ > >> drivers/usb/otg/twl4030-usb.c | 35 --- > >> 2 files changed, 22 insertions(+), 35 deletions(-) > > > >this needs to be rebased against my 'next' branch. > > The v2 of this series is already in your testing branch. You still > want it to be rebased to your 'next' branch? sorry, no need. I just forgot to mark this one as read. My bad. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 01/21] usb: phy: nop: Add some parameters to platform data
On Wed, Mar 20, 2013 at 09:13:24AM -0700, Tony Lindgren wrote: > * Felipe Balbi [130320 09:00]: > > On Wed, Mar 20, 2013 at 05:44:40PM +0200, Roger Quadros wrote: > > > Add clk_rate parameter to platform data. If supplied, the > > > NOP phy driver will program the clock to that rate during probe. > > > > > > Also add 2 flags, needs_vcc and needs_reset. > > > If the flag is set and the regulator couldn't be found > > > then the driver will bail out with -EPROBE_DEFER. > > > > > > Signed-off-by: Roger Quadros > > > Acked-by: Felipe Balbi > > > > Hi Tony, > > > > maybe you might prefer to merge commit 1f0972f from my next branch which > > is exactly this patch. Basically, if you: > > > > $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next > > $ git merge 1f0972f > > > > you get $SUBJECT and can apply the others without fear of conflicts > > later. > > OK thanks will use commit 1f0972f, so let's consider that commit immutable. yeah, once it hits my 'next' branch, I don't rebase anymore. cheers ps: that's valid for 'next' and 'fixes' -- balbi signature.asc Description: Digital signature
Re: [PATCH 03/33] gadget: remove only user of aio retry
Hi,
On Thu, Mar 21, 2013 at 09:35:24AM -0700, Kent Overstreet wrote:
> From: Zach Brown
>
> This removes the only in-tree user of aio retry. This will let us remove
> the retry code from the aio core.
>
> Removing retry is relatively easy as the USB gadget wasn't using it to
> retry IOs at all. It always fully submitted the IO in the context of the
> initial io_submit() call. It only used the AIO retry facility to get the
> submitter's mm context for copying the result of a read back to user
> space. This is easy to implement with use_mm() and a work struct, much
> like kvm does with async_pf_execute() for get_user_pages().
>
> Signed-off-by: Zach Brown
> Signed-off-by: Kent Overstreet
> Cc: Felipe Balbi
> Cc: Greg Kroah-Hartman
> Cc: Mark Fasheh
> Cc: Joel Becker
> Cc: Rusty Russell
> Cc: Jens Axboe
> Cc: Asai Thambi S P
> Cc: Selvan Mani
> Cc: Sam Bradshaw
> Cc: Jeff Moyer
> Cc: Al Viro
> Cc: Benjamin LaHaise
> Cc: Theodore Ts'o
> Signed-off-by: Andrew Morton
I don't have any objections with the approach. Let's see if anyone from
linux-usb has anything to say, though
> drivers/usb/gadget/inode.c | 38 +-
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c
> index e2b2e9c..a1aad43 100644
> --- a/drivers/usb/gadget/inode.c
> +++ b/drivers/usb/gadget/inode.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -513,6 +514,9 @@ static long ep_ioctl(struct file *fd, unsigned code,
> unsigned long value)
> struct kiocb_priv {
> struct usb_request *req;
> struct ep_data *epdata;
> + struct kiocb*iocb;
> + struct mm_struct*mm;
> + struct work_struct work;
> void*buf;
> const struct iovec *iv;
> unsigned long nr_segs;
> @@ -540,15 +544,12 @@ static int ep_aio_cancel(struct kiocb *iocb, struct
> io_event *e)
> return value;
> }
>
> -static ssize_t ep_aio_read_retry(struct kiocb *iocb)
> +static ssize_t ep_copy_to_user(struct kiocb_priv *priv)
> {
> - struct kiocb_priv *priv = iocb->private;
> ssize_t len, total;
> void*to_copy;
> int i;
>
> - /* we "retry" to get the right mm context for this: */
> -
> /* copy stuff into user buffers */
> total = priv->actual;
> len = 0;
> @@ -568,9 +569,26 @@ static ssize_t ep_aio_read_retry(struct kiocb *iocb)
> if (total == 0)
> break;
> }
> +
> + return len;
> +}
> +
> +static void ep_user_copy_worker(struct work_struct *work)
> +{
> + struct kiocb_priv *priv = container_of(work, struct kiocb_priv, work);
> + struct mm_struct *mm = priv->mm;
> + struct kiocb *iocb = priv->iocb;
> + size_t ret;
> +
> + use_mm(mm);
> + ret = ep_copy_to_user(priv);
> + unuse_mm(mm);
> +
> + /* completing the iocb can drop the ctx and mm, don't touch mm after */
> + aio_complete(iocb, ret, ret);
> +
> kfree(priv->buf);
> kfree(priv);
> - return len;
> }
>
> static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
> @@ -596,14 +614,14 @@ static void ep_aio_complete(struct usb_ep *ep, struct
> usb_request *req)
> aio_complete(iocb, req->actual ? req->actual : req->status,
> req->status);
> } else {
> - /* retry() won't report both; so we hide some faults */
> + /* ep_copy_to_user() won't report both; we hide some faults */
> if (unlikely(0 != req->status))
> DBG(epdata->dev, "%s fault %d len %d\n",
> ep->name, req->status, req->actual);
>
> priv->buf = req->buf;
> priv->actual = req->actual;
> - kick_iocb(iocb);
> + schedule_work(&priv->work);
> }
> spin_unlock(&epdata->dev->lock);
>
> @@ -633,8 +651,10 @@ fail:
> return value;
> }
> iocb->private = priv;
> + priv->iocb = iocb;
> priv->iv = iv;
> priv->nr_segs = nr_segs;
> + INIT_WORK(&priv->work, ep_user_copy_worker);
>
> value = get_ready_ep(iocb->ki_filp->f_flags, epdata);
> if (unlikely(value < 0)
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hi, On Fri, Feb 01, 2013 at 09:30:03PM +, Russell King - ARM Linux wrote: > > > > I guess to make the MUSB side simpler we would need musb-dma-engine glue > > > > to map dmaengine to the private MUSB API. Then we would have some > > > > starting point to also move inventra (and anybody else) to dmaengine > > > > API. > > > > > >Why? Inventra is a dedicated device's private DMA controller, why make > > > universal DMA driver for it? > > > > because it doesn't make sense to support multiple DMA APIs. We can check > > from MUSB's registers if it was configured with Inventra DMA support and > > based on that we can register MUSB's own DMA Engine to dmaengine API. > > Hang on. This is one of the DMA implementations which is closely > coupled with the USB and only the USB? If it is... > > I thought this had been discussed _extensively_ before. I thought the > resolution on it was: > 1. It would not use the DMA engine API. > 2. It would not live in arch/arm. > 3. It would be placed nearby the USB driver it's associated with. > > (1) because we don't use APIs just for the hell of it - think. Do we > use the DMA engine API for PCI bus mastering ethernet controllers? No. > Do we use it for PCI bus mastering SCSI controllers? No. Because the > DMA is integral to the rest of the device. that's not really a fair comparison, however. MUSB is used with several DMA engines. The only DMA engine which is really coupled with MUSB is the Inventra DMA engine which comes as an optional feature to the IP. Many users have opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. Granted, CPPI 4.1 makes some assumptions about the fact that it's handling USB tranfers, but nevertheless, the IP can be, and in fact is, used with many different DMA engines and driver needs to cope with it. Current DMA abstraction is quite poor, for example there's no way to compile support for multiple DMA engines. Code also makes certain, IMO unnecessary, assumptions about the underlying DMA engine (abstraction is poor, as said above but it we could follow MUSB's programming guide when it comes to programming DMA transfers). Considering all of the above, it's far better to use DMA engine and get rid of all the mess. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 4/4] drivers: usb: start using the control module driver
Hi, On Fri, Feb 01, 2013 at 11:14:24AM -0800, Tony Lindgren wrote: > * Felipe Balbi [130125 02:30]: > > Hi, > > > > On Fri, Jan 25, 2013 at 03:54:00PM +0530, Kishon Vijay Abraham I wrote: > > > Start using the control module driver for powering on the PHY and for > > > writing to the mailbox instead of writing to the control module > > > registers on their own. > > > > > > Signed-off-by: Kishon Vijay Abraham I > > > --- > > > Documentation/devicetree/bindings/usb/omap-usb.txt |4 ++ > > > Documentation/devicetree/bindings/usb/usb-phy.txt |7 +- > > > arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 13 > > > > I'm taking this patch but I'm leaving out the omap_hwmod_44xx_data.c > > change just to kill dependency. Can you send that single change as a > > separate patch which Tony can queue ? > > For the USB patches, please also leave out patches touching > arch/arm/mach-omap2/devices.c. Those are almost guaranteed to > cause pointless merge conflicts with other branches. > > I suggest you set up few immutable branches: > > 1. Minimal platform_data changes for all your USB changes > > This should contain include/linux/platform_data changes and > changes to arch/arm/*omap* so me and Paul can merge it in too > to avoid merge conflicts. > > 2. The rest of the driver/usb changes > > This can then be based on #1 branch above. > > 3. Changes for the .dts files for Benoit > > These can be queued separately from #1 and #2 above. I'm done with all USB stuff for this merge window. The patches which I didn't take have no dependencies on any drivers/ part. You can easily queue this through your tree. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hi, On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: > > opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, > > Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. > > > Granted, CPPI 4.1 makes some assumptions about the fact that it's > > handling USB tranfers, > >What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just HW makes the asumptions > > but nevertheless, the IP can be, and in fact is, > > used with many different DMA engines and driver needs to cope with it. > >What IP, CPPI 4.1 or MUSB? MUSB > > Current DMA abstraction is quite poor, for example there's no way to > > compile support for multiple DMA engines. Code also makes certain, IMO > > unnecessary, assumptions about the underlying DMA engine (abstraction is > > poor, as said above but it we could follow MUSB's programming guide when > > it comes to programming DMA transfers). > >Don't know, I was quite content with the abstraction when writing CPPI 4.1 > driver for MUSB... look closer. The whole: if (is_cppi()) foo(); else if (is_inventra()) bar(); else if (is_omap_sdma()) baz(); is bogus. > > Considering all of the above, it's far better to use DMA engine and get > > rid of all the mess. > >In my eyes, getting rid of the mess doesn't justify breaking the rules that > Russell formulated above. MUSB is no PCI, there is no single, standard interface to the DMA engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA engine), every DMA engine comes with its own set of registers, its own programming model and so forth. -- balbi signature.asc Description: Digital signature
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hi, On Mon, Feb 04, 2013 at 08:54:17PM +0300, Sergei Shtylyov wrote: > > On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: > >>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, > >>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. > > >>> Granted, CPPI 4.1 makes some assumptions about the fact that it's > >>> handling USB tranfers, > > >>What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's > >> just > > > HW makes the asumptions > >Not true at all. There is a separate set of registers (at offset 0) which > copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. CPPI 4.1 was made for USB transfers. > It's just the way the driver was written that it used both sets of registers > but > this needs to be changed into more abstacted accesses to the USB-specific > part, > to cope with it being different on different platfroms, like AM35x. The driver > as it was last posted, just needs rework now. are you saying you will do the work ? > >>Don't know, I was quite content with the abstraction when writing CPPI > >> 4.1 > >> driver for MUSB... > > > look closer. The whole: > > > if (is_cppi()) > > foo(); > > else if (is_inventra()) > > bar(); > > else if (is_omap_sdma()) > > baz(); > > > is bogus. > >That part -- yes. There were attempt to get rid of this, but without > changing > the DMA API. It was left halfway done after my only critical comment, IIRC. > Will > we ever see the continuation of this effort? patches are welcome -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 4/6] ARM: dts: omap5: add dwc3 omap dt data
On Tue, Feb 05, 2013 at 01:59:26PM +0530, kishon wrote:
> On Sunday 27 January 2013 01:17 AM, Sergei Shtylyov wrote:
> >Hello.
> >
> >On 25-01-2013 15:11, Kishon Vijay Abraham I wrote:
> >
> >>Add dwc3 omap glue data to the omap5 dt data file. The information about
> >>the dt node added here is available @
> >>Documentation/devicetree/bindings/usb/omap-usb.txt
> >
> >>Signed-off-by: Kishon Vijay Abraham I
> >>---
> >> arch/arm/boot/dts/omap5.dtsi | 11 +++
> >> 1 file changed, 11 insertions(+)
> >
> >>diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> >>index 5f59bf2..1703a72 100644
> >>--- a/arch/arm/boot/dts/omap5.dtsi
> >>+++ b/arch/arm/boot/dts/omap5.dtsi
> >>@@ -513,6 +513,17 @@
> >> ti,type = <2>;
> >> };
> >>
> >>+omap_dwc3@4a02 {
> >>+compatible = "ti,dwc3";
> >>+ti,hwmods = "usb_otg_ss";
> >>+reg = <0x4a02 0x1ff>;
> >
> >Shoudn't the "reg" length be 0x200 here? It's length, not limit.
>
> I think 0x1ff is correct. I got the data from hwmod data.
hwmod is utterly wrong. Looking at TRM, it says the size here is 64KiB
(0x1), so is the size for dwc3 itself. Please don't blindly trust
hwmod, make sure you read data from TRM ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 01/30] USB: EHCI: split ehci-omap out to a separate driver
On Mon, Jan 28, 2013 at 01:30:02PM +0200, Roger Quadros wrote:
> From: Alan Stern
>
> This patch (as1645) converts ehci-omap over to the new "ehci-hcd is a
> library" approach, so that it can coexist peacefully with other EHCI
> platform drivers and can make use of the private area allocated at
> the end of struct ehci_hcd.
>
> Signed-off-by: Alan Stern
for ehci-omap:
Acked-by: Felipe Balbi
> ---
> drivers/usb/host/Kconfig |2 +-
> drivers/usb/host/Makefile|1 +
> drivers/usb/host/ehci-hcd.c |6 +---
> drivers/usb/host/ehci-omap.c | 76
> +++---
> 4 files changed, 37 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 3a21c5d..11e102e 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -155,7 +155,7 @@ config USB_EHCI_MXC
> Variation of ARC USB block used in some Freescale chips.
>
> config USB_EHCI_HCD_OMAP
> - bool "EHCI support for OMAP3 and later chips"
> + tristate "EHCI support for OMAP3 and later chips"
> depends on USB_EHCI_HCD && ARCH_OMAP
> default y
> ---help---
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 001fbff..56de410 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
> obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
> obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o
> obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
> +obj-$(CONFIG_USB_EHCI_HCD_OMAP) += ehci-omap.o
>
> obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
> obj-$(CONFIG_USB_ISP116X_HCD)+= isp116x-hcd.o
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 09537b2..5a35246 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1251,11 +1251,6 @@ MODULE_LICENSE ("GPL");
> #define PLATFORM_DRIVER ehci_hcd_sh_driver
> #endif
>
> -#ifdef CONFIG_USB_EHCI_HCD_OMAP
> -#include "ehci-omap.c"
> -#definePLATFORM_DRIVER ehci_hcd_omap_driver
> -#endif
> -
> #ifdef CONFIG_PPC_PS3
> #include "ehci-ps3.c"
> #define PS3_SYSTEM_BUS_DRIVER ps3_ehci_driver
> @@ -1345,6 +1340,7 @@ MODULE_LICENSE ("GPL");
> !IS_ENABLED(CONFIG_USB_EHCI_HCD_PLATFORM) && \
> !IS_ENABLED(CONFIG_USB_CHIPIDEA_HOST) && \
> !IS_ENABLED(CONFIG_USB_EHCI_MXC) && \
> + !IS_ENABLED(CONFIG_USB_EHCI_HCD_OMAP) && \
> !defined(PLATFORM_DRIVER) && \
> !defined(PS3_SYSTEM_BUS_DRIVER) && \
> !defined(OF_PLATFORM_DRIVER) && \
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index b96a4bf..30fc482 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -36,6 +36,9 @@
> * - convert to use hwmod and runtime PM
> */
>
> +#include
> +#include
> +#include
> #include
> #include
> #include
> @@ -43,6 +46,10 @@
> #include
> #include
> #include
> +#include
> +#include
> +
> +#include "ehci.h"
>
> #include
>
> @@ -57,9 +64,11 @@
> #define EHCI_INSNREG05_ULPI_EXTREGADD_SHIFT 8
> #define EHCI_INSNREG05_ULPI_WRDATA_SHIFT0
>
> -/*-*/
> +#define DRIVER_DESC "OMAP-EHCI Host Controller driver"
>
> -static const struct hc_driver ehci_omap_hc_driver;
> +static const char hcd_name[] = "ehci-omap";
> +
> +/*-*/
>
>
> static inline void ehci_write(void __iomem *base, u32 reg, u32 val)
> @@ -166,6 +175,12 @@ static void disable_put_regulator(
> /* configure so an HC device and id are always provided */
> /* always called with process context; sleeping is OK */
>
> +static struct hc_driver __read_mostly ehci_omap_hc_driver;
> +
> +static const struct ehci_driver_overrides ehci_omap_overrides __initdata = {
> + .reset =omap_ehci_init,
> +};
> +
> /**
> * ehci_hcd_omap_probe - initialize TI-based HCDs
> *
> @@ -315,56 +330,33 @@ static struct platform_driver ehci_hcd_omap_driver = {
> /*.suspend = ehci_hcd_omap_suspend, */
> /*.resume = ehci_hcd_omap_resume, */
> .driver = {
> - .name = "ehci-omap",
> + .name = hcd_name,
&
Re: [PATCH v3 10/30] USB: ehci-omap: Use PHY APIs to get the PHY device and put it out of suspend
On Tue, Jan 29, 2013 at 10:30:05AM -0500, Alan Stern wrote: > On Tue, 29 Jan 2013, Roger Quadros wrote: > > > For each port that is in PHY mode we obtain a PHY device using the USB PHY > > library and put it out of suspend. > > > > It is up to platform code to associate the PHY to the controller's > > port and it is upto the PHY driver to manage the PHY's resources. > > s/upto/up to/ > > > Also remove wired spacing around declarations we come across. > > s/wired/weird/ -- not that people care about misspellings in the > Changelog. You don't have to resubmit the patch just to fix these two > items. > > > Signed-off-by: Roger Quadros > > Acked-by: Alan Stern Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 02/30] usb: phy: nop: use devm_kzalloc()
On Mon, Jan 28, 2013 at 01:30:03PM +0200, Roger Quadros wrote:
> Use resource managed kzalloc.
>
> Signed-off-by: Roger Quadros
> ---
Acked-by: Felipe Balbi
> drivers/usb/otg/nop-usb-xceiv.c | 16
> 1 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index a3ce24b..7ffb0c8 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -100,15 +100,13 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> enum usb_phy_type type = USB_PHY_TYPE_USB2;
> int err;
>
> - nop = kzalloc(sizeof *nop, GFP_KERNEL);
> + nop = devm_kzalloc(&pdev->dev, sizeof *nop, GFP_KERNEL);
> if (!nop)
> return -ENOMEM;
>
> - nop->phy.otg = kzalloc(sizeof *nop->phy.otg, GFP_KERNEL);
> - if (!nop->phy.otg) {
> - kfree(nop);
> + nop->phy.otg = devm_kzalloc(&pdev->dev, sizeof *nop->phy.otg,
> GFP_KERNEL);
> + if (!nop->phy.otg)
> return -ENOMEM;
> - }
>
> if (pdata)
> type = pdata->type;
> @@ -127,7 +125,7 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> if (err) {
> dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
> err);
> - goto exit;
> + return err;
> }
>
> platform_set_drvdata(pdev, nop);
> @@ -135,10 +133,6 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> ATOMIC_INIT_NOTIFIER_HEAD(&nop->phy.notifier);
>
> return 0;
> -exit:
> - kfree(nop->phy.otg);
> - kfree(nop);
> - return err;
> }
>
> static int nop_usb_xceiv_remove(struct platform_device *pdev)
> @@ -148,8 +142,6 @@ static int nop_usb_xceiv_remove(struct platform_device
> *pdev)
> usb_remove_phy(&nop->phy);
>
> platform_set_drvdata(pdev, NULL);
> - kfree(nop->phy.otg);
> - kfree(nop);
>
> return 0;
> }
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 03/30] usb: phy: nop: Manage PHY clock
Hi,
On Mon, Jan 28, 2013 at 01:30:04PM +0200, Roger Quadros wrote:
> If the PHY has a clock associated to it then manage the clock.
> We just enable the clock in .init() and disable it in .shutdown().
>
> Add clk_rate parameter in platform data and configure the
> clock rate during probe if supplied.
>
> Signed-off-by: Roger Quadros
Acked-by: Felipe Balbi
> ---
> drivers/usb/otg/nop-usb-xceiv.c | 54
> -
> include/linux/usb/nop-usb-xceiv.h |1 +
> 2 files changed, 54 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index 7ffb0c8..849eb9d 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -32,10 +32,12 @@
> #include
> #include
> #include
> +#include
>
> struct nop_usb_xceiv {
> struct usb_phy phy;
> struct device *dev;
> + struct clk *clk;
> };
>
> static struct platform_device *pd;
> @@ -64,6 +66,24 @@ static int nop_set_suspend(struct usb_phy *x, int suspend)
> return 0;
> }
>
> +static int nop_init(struct usb_phy *phy)
> +{
> + struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
> +
> + if (!IS_ERR(nop->clk))
> + clk_enable(nop->clk);
> +
> + return 0;
> +}
> +
> +static void nop_shutdown(struct usb_phy *phy)
> +{
> + struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
> +
> + if (!IS_ERR(nop->clk))
> + clk_disable(nop->clk);
> +}
> +
> static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
> {
> if (!otg)
> @@ -111,10 +131,34 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> if (pdata)
> type = pdata->type;
>
> + nop->clk = devm_clk_get(&pdev->dev, "main_clk");
> + if (IS_ERR(nop->clk)) {
> + dev_dbg(&pdev->dev, "Can't get phy clock: %ld\n",
> + PTR_ERR(nop->clk));
> + }
> +
> + if (!IS_ERR(nop->clk) && pdata && pdata->clk_rate) {
> + err = clk_set_rate(nop->clk, pdata->clk_rate);
> + if (err) {
> + dev_err(&pdev->dev, "Error setting clock rate\n");
> + return err;
> + }
> + }
> +
> + if (!IS_ERR(nop->clk)) {
> + err = clk_prepare(nop->clk);
> + if (err) {
> + dev_err(&pdev->dev, "Error preparing clock\n");
> + return err;
> + }
> + }
> +
> nop->dev= &pdev->dev;
> nop->phy.dev= nop->dev;
> nop->phy.label = "nop-xceiv";
> nop->phy.set_suspend= nop_set_suspend;
> + nop->phy.init = nop_init;
> + nop->phy.shutdown = nop_shutdown;
> nop->phy.state = OTG_STATE_UNDEFINED;
>
> nop->phy.otg->phy = &nop->phy;
> @@ -125,7 +169,7 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> if (err) {
> dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
> err);
> - return err;
> + goto err_add;
> }
>
> platform_set_drvdata(pdev, nop);
> @@ -133,12 +177,20 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> ATOMIC_INIT_NOTIFIER_HEAD(&nop->phy.notifier);
>
> return 0;
> +
> +err_add:
> + if (!IS_ERR(nop->clk))
> + clk_unprepare(nop->clk);
> + return err;
> }
>
> static int nop_usb_xceiv_remove(struct platform_device *pdev)
> {
> struct nop_usb_xceiv *nop = platform_get_drvdata(pdev);
>
> + if (!IS_ERR(nop->clk))
> + clk_unprepare(nop->clk);
> +
> usb_remove_phy(&nop->phy);
>
> platform_set_drvdata(pdev, NULL);
> diff --git a/include/linux/usb/nop-usb-xceiv.h
> b/include/linux/usb/nop-usb-xceiv.h
> index 28884c7..3265b61 100644
> --- a/include/linux/usb/nop-usb-xceiv.h
> +++ b/include/linux/usb/nop-usb-xceiv.h
> @@ -5,6 +5,7 @@
>
> struct nop_usb_xceiv_platform_data {
> enum usb_phy_type type;
> + unsigned long clk_rate;
> };
>
> #if defined(CONFIG_NOP_USB_XCEIV) || (defined(CONFIG_NOP_USB_XCEIV_MODULE)
> && defined(MODULE))
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 04/30] usb: phy: nop: Handle power supply regulator for the PHY
On Mon, Jan 28, 2013 at 01:30:05PM +0200, Roger Quadros wrote:
> We use "vcc" as the supply name for the PHY's power supply.
> The power supply will be enabled during .init() and disabled
> during .shutdown()
>
> Signed-off-by: Roger Quadros
Acked-by: Felipe Balbi
> ---
> drivers/usb/otg/nop-usb-xceiv.c | 18 ++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index 849eb9d..0a9628c 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -33,11 +33,13 @@
> #include
> #include
> #include
> +#include
>
> struct nop_usb_xceiv {
> struct usb_phy phy;
> struct device *dev;
> struct clk *clk;
> + struct regulator*vcc;
> };
>
> static struct platform_device *pd;
> @@ -70,6 +72,11 @@ static int nop_init(struct usb_phy *phy)
> {
> struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
>
> + if (!IS_ERR(nop->vcc)) {
> + if (regulator_enable(nop->vcc))
> + dev_err(phy->dev, "Failed to enable power\n");
> + }
> +
> if (!IS_ERR(nop->clk))
> clk_enable(nop->clk);
>
> @@ -82,6 +89,11 @@ static void nop_shutdown(struct usb_phy *phy)
>
> if (!IS_ERR(nop->clk))
> clk_disable(nop->clk);
> +
> + if (!IS_ERR(nop->vcc)) {
> + if (regulator_disable(nop->vcc))
> + dev_err(phy->dev, "Failed to disable power\n");
> + }
> }
>
> static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
> @@ -153,6 +165,12 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> }
> }
>
> + nop->vcc = devm_regulator_get(&pdev->dev, "vcc");
> + if (IS_ERR(nop->vcc)) {
> + dev_dbg(&pdev->dev, "Error getting vcc regulator: %ld\n",
> + PTR_ERR(nop->vcc));
> + }
> +
> nop->dev= &pdev->dev;
> nop->phy.dev= nop->dev;
> nop->phy.label = "nop-xceiv";
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 05/30] usb: phy: nop: Handle RESET for the PHY
On Mon, Jan 28, 2013 at 01:30:06PM +0200, Roger Quadros wrote:
> We expect the RESET line to be modeled as a regulator with supply
> name "reset". The regulator should be modeled such that enabling
> the regulator brings the PHY device out of RESET and disabling the
> regulator holds the device in RESET.
>
> They PHY will be held in RESET in .shutdown() and brought out of
> RESET in .init().
>
> Signed-off-by: Roger Quadros
Acked-by: Felipe Balbi
> ---
> drivers/usb/otg/nop-usb-xceiv.c | 19 +++
> 1 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index 0a9628c..3060ed0 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -40,6 +40,7 @@ struct nop_usb_xceiv {
> struct device *dev;
> struct clk *clk;
> struct regulator*vcc;
> + struct regulator*reset;
> };
>
> static struct platform_device *pd;
> @@ -80,6 +81,12 @@ static int nop_init(struct usb_phy *phy)
> if (!IS_ERR(nop->clk))
> clk_enable(nop->clk);
>
> + if (!IS_ERR(nop->reset)) {
> + /* De-assert RESET */
> + if (regulator_enable(nop->reset))
> + dev_err(phy->dev, "Failed to de-assert reset\n");
> + }
> +
> return 0;
> }
>
> @@ -87,6 +94,12 @@ static void nop_shutdown(struct usb_phy *phy)
> {
> struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
>
> + if (!IS_ERR(nop->reset)) {
> + /* Assert RESET */
> + if (regulator_disable(nop->reset))
> + dev_err(phy->dev, "Failed to assert reset\n");
> + }
> +
> if (!IS_ERR(nop->clk))
> clk_disable(nop->clk);
>
> @@ -171,6 +184,12 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> PTR_ERR(nop->vcc));
> }
>
> + nop->reset = devm_regulator_get(&pdev->dev, "reset");
> + if (IS_ERR(nop->reset)) {
> + dev_dbg(&pdev->dev, "Error getting reset regulator: %ld\n",
> + PTR_ERR(nop->reset));
> + }
> +
> nop->dev= &pdev->dev;
> nop->phy.dev= nop->dev;
> nop->phy.label = "nop-xceiv";
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 06/30] usb: phy: nop: use new PHY API to register PHY
On Mon, Jan 28, 2013 at 01:30:07PM +0200, Roger Quadros wrote:
> We would need to support multiple PHYs of the same type
> so use the new PHY API usb_add_phy_dev() to register the PHY.
>
> Signed-off-by: Roger Quadros
Acked-by: Felipe Balbi
> ---
> drivers/usb/otg/nop-usb-xceiv.c |3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index 3060ed0..ac027a1 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -197,12 +197,13 @@ static int nop_usb_xceiv_probe(struct platform_device
> *pdev)
> nop->phy.init = nop_init;
> nop->phy.shutdown = nop_shutdown;
> nop->phy.state = OTG_STATE_UNDEFINED;
> + nop->phy.type = type;
>
> nop->phy.otg->phy = &nop->phy;
> nop->phy.otg->set_host = nop_set_host;
> nop->phy.otg->set_peripheral= nop_set_peripheral;
>
> - err = usb_add_phy(&nop->phy, type);
> + err = usb_add_phy_dev(&nop->phy);
> if (err) {
> dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
> err);
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 07/30] mfd: omap-usb-host: update nports in platform_data
On Mon, Jan 28, 2013 at 01:30:08PM +0200, Roger Quadros wrote: > EHCI driver would need to know the number of ports available > on the platform. We set the nports parameter of platform_data > based on IP version if it was not already provided. > > Signed-off-by: Roger Quadros > Acked-by: Samuel Ortiz Acked-by: Felipe Balbi > --- > drivers/mfd/omap-usb-host.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c > index 6b5edf6..0874352 100644 > --- a/drivers/mfd/omap-usb-host.c > +++ b/drivers/mfd/omap-usb-host.c > @@ -575,6 +575,7 @@ static int usbhs_omap_probe(struct platform_device *pdev) >omap->usbhs_rev, omap->nports); > break; > } > + pdata->nports = omap->nports; > } > > i = sizeof(struct clk *) * omap->nports; > -- > 1.7.4.1 > -- balbi signature.asc Description: Digital signature
Re: [PATCH 08/30] mfd: omap-usb-host: Remove PHY reset handling code
On Mon, Jan 28, 2013 at 01:30:09PM +0200, Roger Quadros wrote:
> PHY reset GPIO handling will be done in the PHY driver
>
> Signed-off-by: Roger Quadros
> Acked-by: Samuel Ortiz
Acked-by: Felipe Balbi
> ---
> drivers/mfd/omap-usb-host.c | 47
> ---
> 1 files changed, 0 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 0874352..502a779 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -430,24 +430,10 @@ static unsigned omap_usbhs_rev2_hostconfig(struct
> usbhs_hcd_omap *omap,
> static void omap_usbhs_init(struct device *dev)
> {
> struct usbhs_hcd_omap *omap = dev_get_drvdata(dev);
> - struct usbhs_omap_platform_data *pdata = omap->pdata;
> unsignedreg;
>
> dev_dbg(dev, "starting TI HSUSB Controller\n");
>
> - if (pdata->phy_reset) {
> - if (gpio_is_valid(pdata->reset_gpio_port[0]))
> - gpio_request_one(pdata->reset_gpio_port[0],
> - GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
> -
> - if (gpio_is_valid(pdata->reset_gpio_port[1]))
> - gpio_request_one(pdata->reset_gpio_port[1],
> - GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
> -
> - /* Hold the PHY in RESET for enough time till DIR is high */
> - udelay(10);
> - }
> -
> pm_runtime_get_sync(dev);
>
> reg = usbhs_read(omap->uhh_base, OMAP_UHH_HOSTCONFIG);
> @@ -476,37 +462,8 @@ static void omap_usbhs_init(struct device *dev)
> dev_dbg(dev, "UHH setup done, uhh_hostconfig=%x\n", reg);
>
> pm_runtime_put_sync(dev);
> - if (pdata->phy_reset) {
> - /* Hold the PHY in RESET for enough time till
> - * PHY is settled and ready
> - */
> - udelay(10);
> -
> - if (gpio_is_valid(pdata->reset_gpio_port[0]))
> - gpio_set_value_cansleep
> - (pdata->reset_gpio_port[0], 1);
> -
> - if (gpio_is_valid(pdata->reset_gpio_port[1]))
> - gpio_set_value_cansleep
> - (pdata->reset_gpio_port[1], 1);
> - }
> -}
> -
> -static void omap_usbhs_deinit(struct device *dev)
> -{
> - struct usbhs_hcd_omap *omap = dev_get_drvdata(dev);
> - struct usbhs_omap_platform_data *pdata = omap->pdata;
> -
> - if (pdata->phy_reset) {
> - if (gpio_is_valid(pdata->reset_gpio_port[0]))
> - gpio_free(pdata->reset_gpio_port[0]);
> -
> - if (gpio_is_valid(pdata->reset_gpio_port[1]))
> - gpio_free(pdata->reset_gpio_port[1]);
> - }
> }
>
> -
> /**
> * usbhs_omap_probe - initialize TI-based HCDs
> *
> @@ -710,8 +667,6 @@ static int usbhs_omap_probe(struct platform_device *pdev)
> return 0;
>
> err_alloc:
> - omap_usbhs_deinit(&pdev->dev);
> -
> for (i = 0; i < omap->nports; i++) {
> if (!IS_ERR(omap->utmi_clk[i]))
> clk_put(omap->utmi_clk[i]);
> @@ -756,8 +711,6 @@ static int usbhs_omap_remove(struct platform_device *pdev)
> struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
> int i;
>
> - omap_usbhs_deinit(&pdev->dev);
> -
> for (i = 0; i < omap->nports; i++) {
> if (!IS_ERR(omap->utmi_clk[i]))
> clk_put(omap->utmi_clk[i]);
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 09/30] USB: ehci-omap: Use devm_request_and_ioremap()
On Mon, Jan 28, 2013 at 01:30:10PM +0200, Roger Quadros wrote:
> Make use of devm_request_and_ioremap() and correct comment.
>
> Signed-off-by: Roger Quadros
Acked-by: Felipe Balbi
> ---
> drivers/usb/host/ehci-omap.c | 19 +--
> 1 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 30fc482..fd2f5450 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -216,23 +216,17 @@ static int ehci_hcd_omap_probe(struct platform_device
> *pdev)
>
> res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM, "ehci");
> - if (!res) {
> - dev_err(dev, "UHH EHCI get resource failed\n");
> - return -ENODEV;
> - }
> -
> - regs = ioremap(res->start, resource_size(res));
> + regs = devm_request_and_ioremap(dev, res);
> if (!regs) {
> - dev_err(dev, "UHH EHCI ioremap failed\n");
> - return -ENOMEM;
> + dev_err(dev, "Resource request/ioremap failed\n");
> + return -EADDRNOTAVAIL;
> }
>
> hcd = usb_create_hcd(&ehci_omap_hc_driver, dev,
> dev_name(dev));
> if (!hcd) {
> - dev_err(dev, "failed to create hcd with err %d\n", ret);
> - ret = -ENOMEM;
> - goto err_io;
> + dev_err(dev, "Failed to create HCD\n");
> + return -ENOMEM;
> }
>
> hcd->rsrc_start = res->start;
> @@ -285,8 +279,6 @@ err_pm_runtime:
> pm_runtime_put_sync(dev);
> usb_put_hcd(hcd);
>
> -err_io:
> - iounmap(regs);
> return ret;
> }
>
> @@ -306,7 +298,6 @@ static int ehci_hcd_omap_remove(struct platform_device
> *pdev)
>
> usb_remove_hcd(hcd);
> disable_put_regulator(dev->platform_data);
> - iounmap(hcd->regs);
> usb_put_hcd(hcd);
>
> pm_runtime_put_sync(dev);
> --
> 1.7.4.1
>
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 10/30] USB: ehci-omap: Use PHY APIs to get the PHY device and put it out of suspend
On Tue, Jan 29, 2013 at 10:30:05AM -0500, Alan Stern wrote: > On Tue, 29 Jan 2013, Roger Quadros wrote: > > > For each port that is in PHY mode we obtain a PHY device using the USB PHY > > library and put it out of suspend. > > > > It is up to platform code to associate the PHY to the controller's > > port and it is upto the PHY driver to manage the PHY's resources. > > s/upto/up to/ > > > Also remove wired spacing around declarations we come across. > > s/wired/weird/ -- not that people care about misspellings in the > Changelog. You don't have to resubmit the patch just to fix these two > items. > > > Signed-off-by: Roger Quadros > > Acked-by: Alan Stern Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 11/30] usb: ehci-omap: Remove PHY reset handling code
On Mon, Jan 28, 2013 at 12:34:16PM -0500, Alan Stern wrote: > On Mon, 28 Jan 2013, Roger Quadros wrote: > > > Reset GPIO handling for the PHY must be done in the PHY > > driver. We use the PHY helpers instead to reset the PHY. > > > > Signed-off-by: Roger Quadros > > Acked-by: Alan Stern Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 12/30] usb: ehci-omap: Remove PHY regulator handling code
On Mon, Jan 28, 2013 at 12:35:25PM -0500, Alan Stern wrote: > On Mon, 28 Jan 2013, Roger Quadros wrote: > > > PHY regulator handling must be done in the PHY driver > > > > Signed-off-by: Roger Quadros > > Acked-by: Alan Stern Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 13/30] ARM: OMAP2+: omap4panda: Provide USB Host's PHY platform data
On Mon, Jan 28, 2013 at 01:30:14PM +0200, Roger Quadros wrote: > Add platform device and data for 'nop-usb-xceiv'. This will be used > as PHY for HS USB port 1, so provide binding information for it. > > Get rid of managing the PHY clock as it will be done by the PHY driver. > For that to work we create a clock alias that links the PHY clock name > to the PHY device name. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 14/30] ARM: OMAP2+: omap4panda: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:15PM +0200, Roger Quadros wrote: > Model RESET and Power for HS USB Port 1 as GPIO fixed regulators > and link them to the 'nop-usb-xceiv' PHY by making them as "reset" > and "vcc" supplies. > > The RESET and Power will then be managed by the PHY driver. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 16/30] ARM: OMAP3: 3430SDP: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:17PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used > as PHYs for HS USB ports 1 and 2 so provide binding information > for them. > > Model RESET for HS USB Ports 1 and 2 as GPIO fixed regulators and > link them to the 2 PHYs we just created. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 15/30] ARM: OMAP3: Beagle: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:16PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 2, so provide binding information for it. > > Model RESET and Power for HS USB Port 2 as GPIO fixed regulators > and link them to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 19/30] ARM: OMAP: AM3517evm: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:20PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used as a > PHY for HS USB Port 1 and 2, so provide binding information for them. > > Model RESET for HS USB Port 1 as GPIO fixed regulator and link it > to the 'nop-usb-xceiv' PHY on port 1. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 20/30] ARM: OMAP3: cm-t35: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:21PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used > as PHYs for HS USB ports 1 and 2 so provide binding information > for them. > > Model RESET for HS USB Ports 1 and 2 as GPIO fixed regulators and > link them to the 2 PHYs we just created. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 21/30] ARM: OMAP3: cm-t3517: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:22PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used > as PHYs for HS USB ports 1 and 2 so provide binding information > for them. > > Model RESET for HS USB Ports 1 and 2 as GPIO fixed regulators and > link them to the 2 PHYs we just created. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 22/30] ARM: OMAP: devkit8000: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:23PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 1, so provide binding information for it. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 23/30] ARM: OMAP3: igep0020: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:24PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used > as PHYs for HS USB ports 1 and 2 so provide binding information > for them. > > Model RESET for HS USB Ports 1 and 2 as GPIO fixed regulators and > link them to the 2 PHYs we just created. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 24/30] ARM: OMAP3: omap3evm: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:25PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 2, so provide binding information for it. > > Model RESET for HS USB Port 2 as GPIO fixed regulator and link > it to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 25/30] ARM: OMAP3: omap3pandora: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:26PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 2, so provide binding information for it. > > Model RESET for HS USB Port 2 as GPIO fixed regulator and link > it to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 26/30] ARM: OMAP3: omap3stalker: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:27PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 2, so provide binding information for it. > > Model RESET for HS USB Port 2 as GPIO fixed regulator and link > it to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 27/30] ARM: OMAP3: omap3touchbook: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:28PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used as a > PHY for HS USB Ports 1 and 2, so provide binding information for them. > > Model RESET for HS USB Port 2 as GPIO fixed regulator and link it > to the respective 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 17/30] ARM: OMAP3: 3630SDP: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:18PM +0200, Roger Quadros wrote: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used > as PHYs for HS USB ports 1 and 2 so provide binding information > for them. > > Model RESET for HS USB Ports 1 and 2 as GPIO fixed regulators and > link them to the 2 PHYs we just created. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 18/30] ARM: OMAP: AM3517crane: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:19PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 1, so provide binding information for it. > > Model RESET and Power for HS USB Port 1 as GPIO fixed regulators > and link them to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 28/30] ARM: OMAP3: overo: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:29PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 2, so provide binding information for it. > > Model RESET for HS USB Port 2 as GPIO fixed regulator and link > it to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 29/30] ARM: OMAP: zoom: Adapt to ehci-omap changes
On Mon, Jan 28, 2013 at 01:30:30PM +0200, Roger Quadros wrote: > Add platform device for 'nop-usb-xceiv'. This will be used as a > PHY for HS USB Port 2, so provide binding information for it. > > Model RESET for HS USB Port 2 as GPIO fixed regulator and link > it to the 'nop-usb-xceiv' PHY. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 30/30] ARM: OMAP: USB: Remove unused fields from struct usbhs_omap_platform_data
On Mon, Jan 28, 2013 at 01:30:31PM +0200, Roger Quadros wrote: > All users have been adapted to the changes in ehci-omap. We can now > get rid of the unused fields from usbhs_omap_platform_data. > > Signed-off-by: Roger Quadros Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 31/31] USB: ehci-omap: Select NOP USB transceiver driver
On Mon, Feb 04, 2013 at 04:03:10PM -0500, Alan Stern wrote: > On Mon, 4 Feb 2013, Roger Quadros wrote: > > > In PHY mode we need to have the nop-usb-xceiv transceiver > > driver to operate, so select it in Kconfig. > > > > CC: Alan Stern > > Signed-off-by: Roger Quadros > > --- > > drivers/usb/host/Kconfig |1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > > index 11e102e..2d2975d 100644 > > --- a/drivers/usb/host/Kconfig > > +++ b/drivers/usb/host/Kconfig > > @@ -157,6 +157,7 @@ config USB_EHCI_MXC > > config USB_EHCI_HCD_OMAP > > tristate "EHCI support for OMAP3 and later chips" > > depends on USB_EHCI_HCD && ARCH_OMAP > > + select NOP_USB_XCEIV > > default y > > ---help--- > > Enables support for the on-chip EHCI controller on > > Acked-by: Alan Stern Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 4/6] ARM: dts: omap5: add dwc3 omap dt data
On Tue, Feb 05, 2013 at 02:37:05PM +0530, Santosh Shilimkar wrote:
> On Tuesday 05 February 2013 02:16 PM, kishon wrote:
> >Hi,
> >
> >On Tuesday 05 February 2013 02:08 PM, Felipe Balbi wrote:
> >>On Tue, Feb 05, 2013 at 01:59:26PM +0530, kishon wrote:
> >>>On Sunday 27 January 2013 01:17 AM, Sergei Shtylyov wrote:
> >>>>Hello.
> >>>>
> >>>>On 25-01-2013 15:11, Kishon Vijay Abraham I wrote:
> >>>>
> >>>>>Add dwc3 omap glue data to the omap5 dt data file. The information
> >>>>>about
> >>>>>the dt node added here is available @
> >>>>>Documentation/devicetree/bindings/usb/omap-usb.txt
> >>>>
> >>>>>Signed-off-by: Kishon Vijay Abraham I
> >>>>>---
> >>>>> arch/arm/boot/dts/omap5.dtsi | 11 +++
> >>>>> 1 file changed, 11 insertions(+)
> >>>>
> >>>>>diff --git a/arch/arm/boot/dts/omap5.dtsi
> >>>>>b/arch/arm/boot/dts/omap5.dtsi
> >>>>>index 5f59bf2..1703a72 100644
> >>>>>--- a/arch/arm/boot/dts/omap5.dtsi
> >>>>>+++ b/arch/arm/boot/dts/omap5.dtsi
> >>>>>@@ -513,6 +513,17 @@
> >>>>> ti,type = <2>;
> >>>>> };
> >>>>>
> >>>>>+omap_dwc3@4a02 {
> >>>>>+compatible = "ti,dwc3";
> >>>>>+ti,hwmods = "usb_otg_ss";
> >>>>>+reg = <0x4a02 0x1ff>;
> >>>>
> >>>>Shoudn't the "reg" length be 0x200 here? It's length, not limit.
> >>>
> >>>I think 0x1ff is correct. I got the data from hwmod data.
> >>
> >>hwmod is utterly wrong. Looking at TRM, it says the size here is 64KiB
> >>(0x1), so is the size for dwc3 itself. Please don't blindly trust
> >>hwmod, make sure you read data from TRM ;-)
> >
> >hmm..ok. But it has only 17 registers :-D
> >
> As Felipe said, it should be 0x200. And if you are
> interested in lesser space, there is no need to map entire 64 KB
> address space if it isn't being used.
We get really close to using all of it. For DWC3, the last address is at
0xcc10 (a 32-bit register).
For dwc3-wrapper, then it's correct, we don't use it, I would still map
the whole thing to make it future proof.
Any draw-backs from mapping all 64K ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 12/15] USB: gadget/freescale: disable non-multiplatform drivers
Hi, On Tue, Feb 05, 2013 at 09:00:27AM -0800, Greg Kroah-Hartman wrote: > On Tue, Feb 05, 2013 at 04:27:18PM +, Arnd Bergmann wrote: > > On Monday 21 January 2013, Greg Kroah-Hartman wrote: > > > On Mon, Jan 21, 2013 at 08:41:38PM +0200, Felipe Balbi wrote: > > > > Hi, > > > > > > > > On Mon, Jan 21, 2013 at 05:16:05PM +, Arnd Bergmann wrote: > > > > > Both the fsl_mxc gadget and the imx_udc gadget drivers fail to build > > > > > without the mach/hardware.h file that is not available when building > > > > > for multiplatform. Let's disable these drivers for v3.8 in combination > > > > > with CONFIG_ARCH_MULTIPLATFORM, and fix them properly in v3.9 unless > > > > > someone has an better solution. > > > > > > > > > > Without this patch, building allyesconfig results in: > > > > > > > > > > drivers/usb/gadget/fsl_mxc_udc.c:21:27: fatal error: mach/hardware.h: > > > > > No such file or directory > > > > > > > > > > Signed-off-by: Arnd Bergmann > > > > > Cc: Felipe Balbi > > > > > Cc: Shawn Guo > > > > > Cc: Greg Kroah-Hartman > > > > > Cc: linux-...@vger.kernel.org > > > > > > > > NAK, I prefer to see a real fix for the problem (which in fact is > > > > already in my fixes branch). > > > > > > I'll pull that branch now, sorry for the delay. > > > > Apparently it hasn't made it into the upstream kernel yet, and it also > > doesn't > > seem to be in linux-next. Maybe this got lost while you were travelling? > > Hm, no, I thought I got Felipe's fixes branch up into Linus's tree > already. Felipe, did I miss something from you? If so, please resend > or let me know, as I don't have anything pending on my side. I don't have anything pending in my fixes branch: $ git log fixes ^linus/master $ git show-branch linus/master [linus/master] Merge branch 'fix-max-write' of git://git.kernel.org/pub/scm/linux/kernel/git/teigland/linux-dlm It's building find for me: $ crossmake drivers/usb/gadget/fsl_udc_core.o \ drivers/usb/gadget/fsl_mxc_udc.o > /dev/null (crossmake is an alias to make ARCH=arm) Arnd, are you maybe missing a merge of v3.8-rc6 ? I can see that imx_udc.c is broken still, but there are no maintainers for that driver. I'm adding Sascha to Cc list, maybe he knows someone who can help, but if this driver isn't fixed in 2 merge windows, I will schedule for removal from tree and someone else will have to re-introduce it later without all the bogus includes. build breaks --- sound/soc/fsl/imx-pcm.c: In function 'snd_imx_pcm_mmap': sound/soc/fsl/imx-pcm.c:28:2: error: 'KBUILD_MODNAME' undeclared (first use in this function) sound/soc/fsl/imx-pcm.c:28:2: note: each undeclared identifier is reported only once for each function it appears in make[3]: *** [sound/soc/fsl/imx-pcm.o] Error 1 drivers/video/omap2/dss/dss.c: In function 'dss_calc_clock_div': drivers/video/omap2/dss/dss.c:572:20: error: 'CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK' undeclared (first use in this function) drivers/video/omap2/dss/dss.c:572:20: note: each undeclared identifier is reported only once for each function it appears in make[4]: *** [drivers/video/omap2/dss/dss.o] Error 1 -- balbi signature.asc Description: Digital signature
Re: [PATCH 12/15] USB: gadget/freescale: disable non-multiplatform drivers
On Tue, Feb 05, 2013 at 05:55:11PM -0800, Greg Kroah-Hartman wrote: > On Tue, Feb 05, 2013 at 10:54:00PM +, Arnd Bergmann wrote: > > On Tuesday 05 February 2013, Felipe Balbi wrote: > > > [linus/master] Merge branch 'fix-max-write' of > > > git://git.kernel.org/pub/scm/linux/kernel/git/teigland/linux-dlm > > > > > > It's building find for me: > > > > > > $ crossmake drivers/usb/gadget/fsl_udc_core.o \ > > > drivers/usb/gadget/fsl_mxc_udc.o > /dev/null > > > > > > (crossmake is an alias to make ARCH=arm) > > > > > > Arnd, are you maybe missing a merge of v3.8-rc6 ? > > > > > > I can see that imx_udc.c is broken still, but there are no maintainers > > > for that driver. I'm adding Sascha to Cc list, maybe he knows someone > > > who can help, but if this driver isn't fixed in 2 merge windows, I will > > > schedule for removal from tree and someone else will have to > > > re-introduce it later without all the bogus includes. > > > > Ah, I see what happened now: I submitted a patch that lumped together > > two patches, disabling both fsl_mxc_udc and imx_udc. You already had > > a fix for the first one, so I dropped my patch, but now I see the > > build error for the second one that my patch was avoiding. > > > > The last patch to imx_udc that seems to have seen more than just > > build testing was probably "USB: gadget: imx_udc: don't queue more > > data when zlp is to be sent", while fsl_mxc_udc looks actively > > maintained. It's not completely clear to me whether these > > are actually two drivers for the same hardware, of whether > > imx_udc is the i.mx1 variant and fsl_mxc_udc is the i.mx2 variant. > > > > What I can say is that no platform in the kernel currently defines > > an "imx_udc" platform_device, so it is certainly unused, and has > > been since at least e08300043e in 2010. > > > > I would suggest that we mark the imx_udc driver as 'depends on > > BROKEN' right away, since no in-tree user needs it, and any > > out of tree user is already broken in 3.8. I also wouldn't > > mind removing the driver unless the imx maintainers have > > a plan for it. > > Sure, someone just send me a patch for that and I'll be glad to mark it > BROKEN. coming in a minute -- balbi signature.asc Description: Digital signature
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Tue, Feb 05, 2013 at 10:26:30PM +, Arnd Bergmann wrote: > On Tuesday 05 February 2013, Tony Lindgren wrote: > > * Felipe Balbi [130204 07:46]: > > > > > > Current DMA abstraction is quite poor, for example there's no way to > > > compile support for multiple DMA engines. Code also makes certain, IMO > > > unnecessary, assumptions about the underlying DMA engine (abstraction is > > > poor, as said above but it we could follow MUSB's programming guide when > > > it comes to programming DMA transfers). > > > > > > Considering all of the above, it's far better to use DMA engine and get > > > rid of all the mess. > > > > How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now? > > That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM > > using PIO while sorting out the DMA related issues. > > Sounds ok to me. Someone still needs to do the work to make the non-DMA > variants of MUSB coexist in one kernel, but as we discussed erlier, that > should be much simpler. Yeah, I'm cooking some patches to at least make it buildable. Dropping unnecessary dependencies and marking da8xx and davinci as depending on BROKEN seems to be the easiest way; those two glues hasn't seen a real patch since 2010 or so. -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/5] ARM: OMAP2: MUSB: Specify omap4 has mailbox
Hi, On Wed, Feb 06, 2013 at 10:57:13AM +, Russell King - ARM Linux wrote: > On Wed, Feb 06, 2013 at 11:28:11AM +0530, Kishon Vijay Abraham I wrote: > > Added has_mailbox to the musb platform data to specify that omap uses > > an external mailbox (in control module) to communicate with the musb > > core during device connect and disconnect. > > So, I've been through your five patches looking for any other users of > "has_mailbox" and I can find none. You introduce this in this patch, > and you set it. But I don't see anything that uses it. > > Write only variables are not useful. looking back at what I sent Greg this was probably my fault. When taking Kishon's patches (in particular $SUBJECT) I removed some of arch/arm/ code and ended up forgetting to add the include file which introduces has_mailbox. This means that my lack of attention has introduced a compile break on Greg's tree. Oh well, s**t happens. In any case, the user is already in Greg's tree and there's no way Greg will rebase his tree to amend $SUBJECT there, so it will have to go separately. I didn't even notice this before because I had the diff in my tree, just forgot to git add and/or git reset. Too late now -- balbi signature.asc Description: Digital signature
Re: [PATCH 3/5] ARM: OMAP2: MUSB: Specify omap4 has mailbox
Hi again, On Wed, Feb 06, 2013 at 01:03:55PM +0200, Felipe Balbi wrote: > Hi, > > On Wed, Feb 06, 2013 at 10:57:13AM +, Russell King - ARM Linux wrote: > > On Wed, Feb 06, 2013 at 11:28:11AM +0530, Kishon Vijay Abraham I wrote: > > > Added has_mailbox to the musb platform data to specify that omap uses > > > an external mailbox (in control module) to communicate with the musb > > > core during device connect and disconnect. > > > > So, I've been through your five patches looking for any other users of > > "has_mailbox" and I can find none. You introduce this in this patch, > > and you set it. But I don't see anything that uses it. > > > > Write only variables are not useful. > > looking back at what I sent Greg this was probably my fault. When taking > Kishon's patches (in particular $SUBJECT) I removed some of arch/arm/ > code and ended up forgetting to add the include file which introduces > has_mailbox. > > This means that my lack of attention has introduced a compile break on > Greg's tree. Oh well, s**t happens. In any case, the user is already in > Greg's tree and there's no way Greg will rebase his tree to amend > $SUBJECT there, so it will have to go separately. > > I didn't even notice this before because I had the diff in my tree, just > forgot to git add and/or git reset. Too late now there's a little more to it. When running allmodconfig, CONFIG_ARCH_MULTIPLATFORM is set but none of the other ARCHes (ARCH_OMAP, ARCH_AT91, ARCH_VERSATILE, etc) are set, so it turned out that the driver wasn't even included on my build test. Russell, is this expected (the MULTIPLATFORM thing) ? Just so I fix my build test scripts to use another defconfig instead of allmod and allyes. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb host: Faraday FUSBH200 HCD driver.
On Wed, Feb 06, 2013 at 07:24:01PM +0800, Yuan-Hsin Chen wrote: > From: Yuan-Hsin Chen > > USB2.0 HCD driver for Faraday FUSBH200 > > Signed-off-by: Yuan-Hsin Chen just use ehci-platform.c and avoid the duplication -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/4] ARM: tegra: Unify tegra{20,30,114}_init_early()
Hi,
On Fri, Feb 08, 2013 at 09:29:31AM +0200, Hiroshi Doyu wrote:
> @@ -56,18 +56,21 @@ int tegra_cpu_disable(unsigned int cpu)
> return cpu == 0 ? -EPERM : 0;
> }
>
> -#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> -extern void tegra20_hotplug_shutdown(void);
> -void __init tegra20_hotplug_init(void)
> +void __init tegra_hotplug_init(void)
> {
> - tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
> -}
> + switch (tegra_chip_id) {
> +#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> + case TEGRA20:
> + tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
> + break;
> #endif
> -
> -#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> -extern void tegra30_hotplug_shutdown(void);
> -void __init tegra30_hotplug_init(void)
> -{
> - tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
> -}
> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
how about using:
#if IS_BUILTIN(CONFIG_ARCH_TEGRA_3x_SOC)
instead ?
> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
> index 4ffae54..970ebd5 100644
> --- a/arch/arm/mach-tegra/sleep.h
> +++ b/arch/arm/mach-tegra/sleep.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2010-2012, NVIDIA Corporation. All rights reserved.
> + * Copyright (c) 2010-2013, NVIDIA Corporation. All rights reserved.
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms and conditions of the GNU General Public License,
> @@ -124,11 +124,11 @@ int tegra_sleep_cpu_finish(unsigned long);
> void tegra_disable_clean_inv_dcache(void);
>
> #ifdef CONFIG_HOTPLUG_CPU
> -void tegra20_hotplug_init(void);
> -void tegra30_hotplug_init(void);
> +void tegra20_hotplug_shutdown(void);
> +void tegra30_hotplug_shutdown(void);
aren't these two called only by tegra_hotplug_init() now ? That means
they don't need to be exposed.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v6 08/10] ARM: dt: Add references to tegra_car clocks
Hi,
On Fri, Feb 08, 2013 at 03:36:40PM +0200, Peter De Schrijver wrote:
> tegra_car: clock {
> - compatible = "nvidia,tegra114-car, nvidia,tegra30-car";
> + compatible = "nvidia,tegra114-car";
trailing change or intentional ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] wlcore: remove newly introduced alloc/OOM messages
On Mon, Feb 11, 2013 at 10:02:29AM +0200, Luciano Coelho wrote: > In commit 0d2e7a5c (wireless: Remove unnecessary alloc/OOM messages, > alloc cleanups) OOM messages after alloc were removed from the wlcore > modules. > > Commit afb43e6d (wlcore: remove if_ops from platform_data) > reintroduced a couple of those. This patch removes them. > > Signed-off-by: Luciano Coelho > --- > > John, > > Can you apply this directly on wireless-next after you process my latest > pull-request? please don't... patch isn't really what commit log says :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] wlcore: remove newly introduced alloc/OOM messages
On Mon, Feb 11, 2013 at 10:21:18AM +0200, Luciano Coelho wrote: > In commit 0d2e7a5c (wireless: Remove unnecessary alloc/OOM messages, > alloc cleanups) OOM messages after alloc were removed from the wlcore > modules. > > Commit afb43e6d (wlcore: remove if_ops from platform_data) > reintroduced a couple of those. This patch removes them. > > Signed-off-by: Luciano Coelho looks good now :-) Reviewed-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/2] ARM: OMAP2+: Allow clock alias provision from device tree
Hi,
On Mon, Feb 11, 2013 at 05:44:23PM +0200, Roger Quadros wrote:
> @@ -36,12 +39,76 @@ static struct of_device_id omap_dt_match_table[]
> __initdata = {
> { }
> };
>
> +static int __init omap_create_clk_alias(struct device_node *np)
if the rest of the folks agree with the aproach, looks like most of this
can be made a bit more generic ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
On Mon, Apr 01, 2013 at 07:24:00PM +0530, Vivek Gautam wrote:
> Adding APIs to handle runtime power management on PHY
> devices. PHY consumers may need to wake-up/suspend PHYs
> when they work across autosuspend.
>
> Signed-off-by: Vivek Gautam
> ---
> include/linux/usb/phy.h | 141
> +++
> 1 files changed, 141 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index 6b5978f..01bf9c1 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -297,4 +297,145 @@ static inline const char *usb_phy_type_string(enum
> usb_phy_type type)
> return "UNKNOWN PHY TYPE";
> }
> }
> +
> +static inline void usb_phy_autopm_enable(struct usb_phy *x)
> +{
> + if (!x || !x->dev) {
> + dev_err(x->dev, "no PHY or attached device available\n");
> + return;
> + }
wrong indentation, also, I'm not sure we should allow calls with NULL
pointers. Perhaps a WARN() so we get API offenders early enough ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 02/11] USB: dwc3: Adjust runtime pm to allow autosuspend
On Mon, Apr 01, 2013 at 07:24:01PM +0530, Vivek Gautam wrote: > The current code in the dwc3 probe effectively disables runtime pm > from ever working because it calls a get() that was never put() until > device removal. Change the runtime pm code to match the standard > formula and allow runtime pm to function. > > Signed-off-by: Vivek Gautam > CC: Doug Anderson > --- > drivers/usb/dwc3/core.c |8 +++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index e2325ad..3a6993c 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -491,6 +491,11 @@ static int dwc3_probe(struct platform_device *pdev) > > dwc->needs_fifo_resize = of_property_read_bool(node, "tx-fifo-resize"); > > + /* Setting device state as 'suspended' initially, wrong comment style. > + * to make sure we know device state prior to > + * pm_runtime_enable > + */ > + pm_runtime_set_suspended(dev); didn't Alan mention this should be done at the Bus level ? In that case, shouldn't you have call pm_runtime_set_active/suspended() based on DT's status=okay or status=disabled ? Or something similar ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 04/11] usb: dwc3: Add runtime power management callbacks
Hi, On Mon, Apr 01, 2013 at 07:24:03PM +0530, Vivek Gautam wrote: > +#else > +#define dwc3_runtime_suspend NULL > +#define dwc3_runtime_resume NULL this #else branch is unnecessary. Look at the definition for SET_RUNTIME_PM_OPS() -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 05/11] usb: dwc3: exynos: Enable runtime power management
On Mon, Apr 01, 2013 at 07:24:04PM +0530, Vivek Gautam wrote: > Enabling runtime power management on dwc3-exynos > letting dwc3 controller to be autosuspended on exynos > platform when not in use. > > Signed-off-by: Vivek Gautam > --- > drivers/usb/dwc3/dwc3-exynos.c | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > index 1ea7bd8..1ae81a0 100644 > --- a/drivers/usb/dwc3/dwc3-exynos.c > +++ b/drivers/usb/dwc3/dwc3-exynos.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -138,6 +139,11 @@ static int dwc3_exynos_probe(struct platform_device > *pdev) > exynos->dev = dev; > exynos->clk = clk; > > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + pm_runtime_get_sync(dev); > + pm_runtime_forbid(dev); don't you want to use autosuspend() to avoid consecutive suspend/resume calls when controller is under use ? -- balbi signature.asc Description: Digital signature
Re: linux-next: manual merge of the usb-gadget tree with the tree
Hi,
On Tue, Apr 02, 2013 at 05:11:43PM +1100, Stephen Rothwell wrote:
> Hi Felipe,
>
> Today's linux-next merge of the usb-gadget tree got a conflict in
> drivers/usb/chipidea/udc.c between commit adf0f735e61a ("usb: chipidea:
> move debug files creation/removal to the core") from the usb tree and
> commit dc9e2873b740 ("usb: chipidea: let udc-core manage gadget->dev")
> from the usb-gadget tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
looks alright to me, thank you.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
Hi,
On Tue, Apr 02, 2013 at 04:04:01PM +0530, Vivek Gautam wrote:
> > On Mon, Apr 01, 2013 at 07:24:00PM +0530, Vivek Gautam wrote:
> >> Adding APIs to handle runtime power management on PHY
> >> devices. PHY consumers may need to wake-up/suspend PHYs
> >> when they work across autosuspend.
> >>
> >> Signed-off-by: Vivek Gautam
> >> ---
> >> include/linux/usb/phy.h | 141
> >> +++
> >> 1 files changed, 141 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> >> index 6b5978f..01bf9c1 100644
> >> --- a/include/linux/usb/phy.h
> >> +++ b/include/linux/usb/phy.h
> >> @@ -297,4 +297,145 @@ static inline const char *usb_phy_type_string(enum
> >> usb_phy_type type)
> >> return "UNKNOWN PHY TYPE";
> >> }
> >> }
> >> +
> >> +static inline void usb_phy_autopm_enable(struct usb_phy *x)
> >> +{
> >> + if (!x || !x->dev) {
> >> + dev_err(x->dev, "no PHY or attached device available\n");
> >> + return;
> >> + }
> >
> > wrong indentation, also, I'm not sure we should allow calls with NULL
> > pointers. Perhaps a WARN() so we get API offenders early enough ?
>
> True, bad coding style :-(
> We should be handling dev_err with a NULL pointer.
> Will just keep here:
> if (WARN_ON(!x->dev))
> return ;
right, but I guess:
if (WARN(!x || !x->dev, "Invalid parameters\n"))
return -EINVAL;
would be better ??
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
Hi,
On Wed, Apr 03, 2013 at 11:48:39AM +0530, Vivek Gautam wrote:
> >> Adding APIs to handle runtime power management on PHY
> >> devices. PHY consumers may need to wake-up/suspend PHYs
> >> when they work across autosuspend.
> >>
> >> Signed-off-by: Vivek Gautam
> >> ---
> >> include/linux/usb/phy.h | 141
> >> +++
> >> 1 files changed, 141 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> >> index 6b5978f..01bf9c1 100644
> >> --- a/include/linux/usb/phy.h
> >> +++ b/include/linux/usb/phy.h
> >> @@ -297,4 +297,145 @@ static inline const char *usb_phy_type_string(enum
> >> usb_phy_type type)
> >> return "UNKNOWN PHY TYPE";
> >> }
> >> }
> >> +
> >> +static inline void usb_phy_autopm_enable(struct usb_phy *x)
> >> +{
> >> + if (!x || !x->dev) {
> >> + dev_err(x->dev, "no PHY or attached device available\n");
> >> + return;
> >> + }
> >> +
> >> + pm_runtime_enable(x->dev);
> >> +}
> >
> >
> > IMO we need not have wrapper APIs for runtime_enable and runtime_disable
> > here. Generally runtime_enable and runtime_disable is done in probe and
> > remove of a driver respectively. So it's better to leave the
> > runtime_enable/runtime_disable to be done in *phy provider* driver than
> > having an API for it to be done by *phy user* driver. Felipe, what do you
> > think?
>
> Thanks!!
> That's very true, runtime_enable() and runtime_disable() calls are made by
> *phy_provider* only. But a querry here.
> Wouldn't in any case a PHY consumer might want to disable runtime_pm on PHY ?
> Say, when consumer failed to suspend the PHY properly
> (*put_sync(phy->dev)* fails), how much sure is the consumer about the
> state of PHY ?
no no, wait a minute. We might not want to enable runtime pm for the PHY
until the UDC says it can handle runtime pm, no ? I guess this makes a
bit of sense (at least in my head :-p).
Imagine if PHY is runtime suspended but e.g. DWC3 isn't runtime pm
enabled... Does it make sense to leave that control to the USB
controller drivers ?
I'm open for suggestions
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 02/11] USB: dwc3: Adjust runtime pm to allow autosuspend
Hi, On Wed, Apr 03, 2013 at 11:35:43AM +0530, Vivek Gautam wrote: > >> The current code in the dwc3 probe effectively disables runtime pm > >> from ever working because it calls a get() that was never put() until > >> device removal. Change the runtime pm code to match the standard > >> formula and allow runtime pm to function. > >> > >> Signed-off-by: Vivek Gautam > >> CC: Doug Anderson > >> --- > >> drivers/usb/dwc3/core.c |8 +++- > >> 1 files changed, 7 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > >> index e2325ad..3a6993c 100644 > >> --- a/drivers/usb/dwc3/core.c > >> +++ b/drivers/usb/dwc3/core.c > >> @@ -491,6 +491,11 @@ static int dwc3_probe(struct platform_device *pdev) > >> > >> dwc->needs_fifo_resize = of_property_read_bool(node, > >> "tx-fifo-resize"); > >> > >> + /* Setting device state as 'suspended' initially, > > > > wrong comment style. > Yea :-( will fix this. > > > > >> + * to make sure we know device state prior to > >> + * pm_runtime_enable > >> + */ > >> + pm_runtime_set_suspended(dev); > > > > didn't Alan mention this should be done at the Bus level ? In that case, > > shouldn't you have call pm_runtime_set_active/suspended() based on > > DT's status=okay or status=disabled ? Or something similar ? > > True, we should be doing this at bus level. But he did also mention to > let pm core > know of the state of the device before enabling the runtime pm. Right ? > Moreover immediately after pm_runtime_enable(), we do pm_runtime_get_sync() > so that device comes to active state. > I am possibly missing things out here, not able to grab this whole > picture completely :-( > > Wouldn't DT's status=disabled actually be disabling the device as a whole ? > So, how much will runtime power management on the device be affecting ? indeed, maybe we can keep it like this, but it would be nice to have OF core handle this for us based on whatever data. -- balbi signature.asc Description: Digital signature
Re: [PATCH 04/12] usb: phy: ab8500-usb: fix eye diagram for ab8500 v2.0
On Wed, Apr 03, 2013 at 10:45:05AM +0200, Fabio Baltieri wrote:
> From: Sakethram Bommisetti
>
> AB8500 v2.0 has eye diagram issues when drawing more than 100mA from
> VBUS. Force charging current to 100mA in case of standard host.
>
> Signed-off-by: Sakethram Bommisetti
> Acked-by: Linus Walleij
> Signed-off-by: Fabio Baltieri
> ---
> drivers/usb/phy/phy-ab8500-usb.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-ab8500-usb.c
> b/drivers/usb/phy/phy-ab8500-usb.c
> index 5b92a59..1fd0780 100644
> --- a/drivers/usb/phy/phy-ab8500-usb.c
> +++ b/drivers/usb/phy/phy-ab8500-usb.c
> @@ -485,6 +485,18 @@ static void ab8500_usb_phy_disable_work(struct
> work_struct *work)
> ab8500_usb_peri_phy_dis(ab);
> }
>
> +static unsigned ab8500_eyediagram_workaroud(struct ab8500_usb *ab, unsigned
> mA)
> +{
> + /* AB8500 V2 has eye diagram issues when drawing more than 100mA from
wrong comment style. Please run *all* patches through checkpatch.pl
--strict and sparse. I can wait 2 more hours before closing my tree for
v3.10.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 04/12] usb: phy: ab8500-usb: fix eye diagram for ab8500 v2.0
On Wed, Apr 03, 2013 at 11:53:36AM +0300, Felipe Balbi wrote:
> On Wed, Apr 03, 2013 at 10:45:05AM +0200, Fabio Baltieri wrote:
> > From: Sakethram Bommisetti
> >
> > AB8500 v2.0 has eye diagram issues when drawing more than 100mA from
> > VBUS. Force charging current to 100mA in case of standard host.
> >
> > Signed-off-by: Sakethram Bommisetti
> > Acked-by: Linus Walleij
> > Signed-off-by: Fabio Baltieri
> > ---
> > drivers/usb/phy/phy-ab8500-usb.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/usb/phy/phy-ab8500-usb.c
> > b/drivers/usb/phy/phy-ab8500-usb.c
> > index 5b92a59..1fd0780 100644
> > --- a/drivers/usb/phy/phy-ab8500-usb.c
> > +++ b/drivers/usb/phy/phy-ab8500-usb.c
> > @@ -485,6 +485,18 @@ static void ab8500_usb_phy_disable_work(struct
> > work_struct *work)
> > ab8500_usb_peri_phy_dis(ab);
> > }
> >
> > +static unsigned ab8500_eyediagram_workaroud(struct ab8500_usb *ab,
> > unsigned mA)
> > +{
> > + /* AB8500 V2 has eye diagram issues when drawing more than 100mA from
>
> wrong comment style. Please run *all* patches through checkpatch.pl
> --strict and sparse. I can wait 2 more hours before closing my tree for
> v3.10.
you know what ? this is simple enough, I'll fix it myself.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 04/12] usb: phy: ab8500-usb: fix eye diagram for ab8500 v2.0
On Wed, Apr 03, 2013 at 11:12:00AM +0200, Fabio Baltieri wrote:
> Hi Felipe,
>
> On Wed, Apr 03, 2013 at 11:58:58AM +0300, Felipe Balbi wrote:
> > On Wed, Apr 03, 2013 at 11:53:36AM +0300, Felipe Balbi wrote:
> > > On Wed, Apr 03, 2013 at 10:45:05AM +0200, Fabio Baltieri wrote:
> > > > From: Sakethram Bommisetti
> > > >
> > > > AB8500 v2.0 has eye diagram issues when drawing more than 100mA from
> > > > VBUS. Force charging current to 100mA in case of standard host.
> > > >
> > > > Signed-off-by: Sakethram Bommisetti
> > > >
> > > > Acked-by: Linus Walleij
> > > > Signed-off-by: Fabio Baltieri
> > > > ---
> > > > drivers/usb/phy/phy-ab8500-usb.c | 14 ++
> > > > 1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/phy/phy-ab8500-usb.c
> > > > b/drivers/usb/phy/phy-ab8500-usb.c
> > > > index 5b92a59..1fd0780 100644
> > > > --- a/drivers/usb/phy/phy-ab8500-usb.c
> > > > +++ b/drivers/usb/phy/phy-ab8500-usb.c
> > > > @@ -485,6 +485,18 @@ static void ab8500_usb_phy_disable_work(struct
> > > > work_struct *work)
> > > > ab8500_usb_peri_phy_dis(ab);
> > > > }
> > > >
> > > > +static unsigned ab8500_eyediagram_workaroud(struct ab8500_usb *ab,
> > > > unsigned mA)
> > > > +{
> > > > + /* AB8500 V2 has eye diagram issues when drawing more than
> > > > 100mA from
> > >
> > > wrong comment style. Please run *all* patches through checkpatch.pl
> > > --strict and sparse. I can wait 2 more hours before closing my tree for
> > > v3.10.
> >
> > you know what ? this is simple enough, I'll fix it myself.
>
> Ok, I was rushing a rebase but you've been faster! :-)
>
> Anyway, paches were already sparse and checkpatch checked, but I ignored
> a warning for "mA" camelcase (as that's how it's commonly spelled for
> regulator) and new line indentation (where I used two-tabs instead of
> open parenthesis to be coherent with the rest of the source).
>
> For comment style, checkpatch does not give me any warning for some
> reson. The in-line open style is already used in the net/ subsystem so
> I was assuming that it was accepted here too. If this is not the case
> maybe checkpatch.pl has to be fixed somehow.
right, only the net subsystem has this inline open style :-)
Nevermind, it was so minor that it didn't even hurt me :-p
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 1/5 v2] USB: regroup all depends on USB within an if USB block
On Wed, Apr 03, 2013 at 02:11:13PM +0200, Florian Fainelli wrote: > Le 04/02/13 20:06, Alan Stern a écrit : > >On Tue, 2 Apr 2013, Florian Fainelli wrote: > > > >>This patch removes the depends on USB from all config symbols in > >>drivers/usb/host/Kconfig and replace that with an if USB / endif block > >>as suggested by Alan Stern. Some source ... Kconfig lines have been > >>shuffled around to permit a better regroupment of the Kconfig files > >>depending on "config USB" item. No functionnal change is introduced. > > > >This looks almost right. The only problem I see is in > >drivers/usb/core/Kconfig. The USB_OTG_WHITELIST and > >USB_OTG_BLACKLIST_HUB symbols don't have to depend on USB or USB_OTG, > >because they can be set if EXPERT is enabled. > > > >To avoid these issues, I think the best approach is to move all the > >USB_OTG* entries over to drivers/usb/otg/Kconfig, where by rights they > >should have been all along. > > > >Felipe, do you agree? > > Make sense, I will make this a sixth patch to this serie if this > sounds right with you? Pleae don't. Look at my 'next' branch. I deleted drivers/usb/otg/ completely. OTG should be part of usbcore and I have plans of adding generic (and optional) OTG hooks in there. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/5 v2] USB: regroup all depends on USB within an if USB block
On Wed, Apr 03, 2013 at 02:18:20PM +0200, Florian Fainelli wrote: > Le 04/03/13 14:15, Felipe Balbi a écrit : > >On Wed, Apr 03, 2013 at 02:11:13PM +0200, Florian Fainelli wrote: > >>Le 04/02/13 20:06, Alan Stern a écrit : > >>>On Tue, 2 Apr 2013, Florian Fainelli wrote: > >>> > >>>>This patch removes the depends on USB from all config symbols in > >>>>drivers/usb/host/Kconfig and replace that with an if USB / endif block > >>>>as suggested by Alan Stern. Some source ... Kconfig lines have been > >>>>shuffled around to permit a better regroupment of the Kconfig files > >>>>depending on "config USB" item. No functionnal change is introduced. > >>> > >>>This looks almost right. The only problem I see is in > >>>drivers/usb/core/Kconfig. The USB_OTG_WHITELIST and > >>>USB_OTG_BLACKLIST_HUB symbols don't have to depend on USB or USB_OTG, > >>>because they can be set if EXPERT is enabled. > >>> > >>>To avoid these issues, I think the best approach is to move all the > >>>USB_OTG* entries over to drivers/usb/otg/Kconfig, where by rights they > >>>should have been all along. > >>> > >>>Felipe, do you agree? > >> > >>Make sense, I will make this a sixth patch to this serie if this > >>sounds right with you? > > > >Pleae don't. Look at my 'next' branch. I deleted drivers/usb/otg/ > >completely. OTG should be part of usbcore and I have plans of adding > >generic (and optional) OTG hooks in there. > > Ok, then I will leave it as it is now and simply address the comment > your made initially, does that fit both you and Alan? Thanks! Fine by me, if something else needs to go, then we can tackle it during -rc or for v3.11. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
On Wed, Apr 03, 2013 at 06:23:49PM +0530, Kishon Vijay Abraham I wrote:
> The PHY framework provides a set of APIs for the PHY drivers to
> create/destroy a PHY and APIs for the PHY users to obtain a reference to the
> PHY with or without using phandle. To obtain a reference to the PHY without
> using phandle, the platform specfic intialization code (say from board file)
> should have already called phy_bind with the binding information. The binding
> information consists of phy's device name, phy user device name and an index.
> The index is used when the same phy user binds to mulitple phys.
>
> PHY drivers should create the PHY by passing phy_descriptor that has
> describes the PHY (label, type etc..) and ops like init, exit, suspend,
> resume,
> power_on, power_off.
>
> The documentation for the generic PHY framework is added in
> Documentation/phy.txt and the documentation for the sysfs entry is added
> in Documentation/ABI/testing/sysfs-class-phy and the documentation for
> dt binding is can be found at
> Documentation/devicetree/bindings/phy/phy-bindings.txt
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> Documentation/ABI/testing/sysfs-class-phy | 15 +
> .../devicetree/bindings/phy/phy-bindings.txt | 67 +++
> Documentation/phy.txt | 113
> MAINTAINERS|7 +
> drivers/Kconfig|2 +
> drivers/Makefile |2 +
> drivers/phy/Kconfig| 13 +
> drivers/phy/Makefile |5 +
> drivers/phy/phy-core.c | 616
>
> include/linux/phy/phy.h| 228
> 10 files changed, 1068 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-phy
> create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
> create mode 100644 Documentation/phy.txt
> create mode 100644 drivers/phy/Kconfig
> create mode 100644 drivers/phy/Makefile
> create mode 100644 drivers/phy/phy-core.c
> create mode 100644 include/linux/phy/phy.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-phy
> b/Documentation/ABI/testing/sysfs-class-phy
> new file mode 100644
> index 000..b735467
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-phy
> @@ -0,0 +1,15 @@
> +What:/sys/class/phy//label
> +Date:Apr 2013
> +KernelVersion: 3.10
> +Contact: Kishon Vijay Abraham I
> +Description:
> + This is a read-only file for getting the label of the phy.
> +
> +What:/sys/class/phy//phy_bind
> +Date:Apr 2013
> +KernelVersion: 3.10
> +Contact: Kishon Vijay Abraham I
> +Description:
> + This is a read-only file for reading the phy binding
> + information. It contains the device name of the controller,
> + the index and the device name of the PHY in that order.
> diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt
> b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> new file mode 100644
> index 000..e7b246a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
> @@ -0,0 +1,67 @@
> +This document explains only the dt data binding. For general information
> about
> +PHY subsystem refer Documentation/phy.txt
> +
> +PHY device node
> +===
> +
> +Required Properties:
> +#phy-cells: Number of cells in a PHY specifier; The meaning of all those
> + cells is defined by the binding for the phy node. The PHY
> + provider can use the values in cells to find the appropriate
> + PHY.
> +
> +For example:
> +
> +phys: phy {
> +compatible = "xxx";
> +reg = <...>;
> +.
> +.
> +#phy-cells = <1>;
> +.
> +.
> +};
> +
> +That node describes an IP block that implements 2 different PHYs. In order to
> +differentiate between these 2 PHYs, an additonal specifier should be given
> +while trying to get a reference to it.
> +
> +PHY user node
> +=
> +
> +Required Properties:
> +phys : the phandle for the PHY device (used by the PHY subsystem)
> +
> +Optional properties:
> +phy-names : the names of the PHY corresponding to the PHYs present in the
> + *phys* phandle
> +
> +Example 1:
> +usb1: usb_otg_ss@xxx {
> +compatible = "xxx";
> +reg = ;
> +.
> +.
> +phys = <&usb2_phy>, <&usb3_phy>;
> +phy-names = "usb2phy", "usb3phy";
> +.
> +.
> +};
> +
> +This node represents a controller that uses two PHYs one for usb2 and one for
> +usb3.
> +
> +Example 2:
> +usb2: usb_otg_ss@xxx {
> +compatible = "xxx";
> +reg = ;
> +.
> +.
> +phys = <&phys 1>;
> +.
> +.
> +};
> +
> +This node represents a controller that uses one of the PHYs which is defined
> +previously. Note that the phy handle h
Re: [PATCH v5 2/6] usb: phy: omap-usb2: use the new generic PHY framework
On Wed, Apr 03, 2013 at 06:23:50PM +0530, Kishon Vijay Abraham I wrote:
> Used the generic PHY framework API to create the PHY. omap_usb2_suspend
> is split into omap_usb_suspend and omap_usb_resume in order to align
> with the new framework.
>
> However using the old USB PHY library cannot be completely removed
> because OTG is intertwined with PHY and moving to the new framework
> will break OTG. Once we have a separate OTG state machine, we
> can get rid of the USB PHY library.
>
> Signed-off-by: Kishon Vijay Abraham I
> ---
> drivers/usb/phy/omap-usb2.c | 48
> +++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
> index 844ab68..4e48db4 100644
> --- a/drivers/usb/phy/omap-usb2.c
> +++ b/drivers/usb/phy/omap-usb2.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> /**
> * omap_usb2_set_comparator - links the comparator present in the sytem with
> @@ -119,9 +120,49 @@ static int omap_usb2_suspend(struct usb_phy *x, int
> suspend)
> return 0;
> }
>
> +static int omap_usb_suspend(struct phy *x)
> +{
> + struct omap_usb *phy = dev_get_drvdata(&x->dev);
> +
> + if (!phy->is_suspended) {
> + omap_control_usb_phy_power(phy->control_dev, 0);
> + pm_runtime_put_sync(phy->dev);
> + phy->is_suspended = 1;
> + }
> +
> + return 0;
> +}
> +
> +static int omap_usb_resume(struct phy *x)
> +{
> + u32 ret;
> + struct omap_usb *phy = dev_get_drvdata(&x->dev);
> +
> + if (phy->is_suspended) {
> + ret = pm_runtime_get_sync(phy->dev);
> + if (ret < 0) {
> + dev_err(phy->dev, "get_sync failed with err %d\n",
> + ret);
> + return ret;
> + }
> + omap_control_usb_phy_power(phy->control_dev, 1);
> + phy->is_suspended = 0;
> + }
> +
> + return 0;
> +}
> +
> +static struct phy_ops ops = {
const ? Maybe provide a:
#define DEFINE_PHY_OPS(name)\
const struct phy_ops #name_phy_ops = {
macro ? This will force people to add the const keyword :-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
HI,
On Wed, Apr 03, 2013 at 06:42:48PM +0530, Vivek Gautam wrote:
> >> >> Adding APIs to handle runtime power management on PHY
> >> >> devices. PHY consumers may need to wake-up/suspend PHYs
> >> >> when they work across autosuspend.
> >> >>
> >> >> Signed-off-by: Vivek Gautam
> >> >> ---
> >> >> include/linux/usb/phy.h | 141
> >> >> +++
> >> >> 1 files changed, 141 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> >> >> index 6b5978f..01bf9c1 100644
> >> >> --- a/include/linux/usb/phy.h
> >> >> +++ b/include/linux/usb/phy.h
> >> >> @@ -297,4 +297,145 @@ static inline const char *usb_phy_type_string(enum
> >> >> usb_phy_type type)
> >> >> return "UNKNOWN PHY TYPE";
> >> >> }
> >> >> }
> >> >> +
> >> >> +static inline void usb_phy_autopm_enable(struct usb_phy *x)
> >> >> +{
> >> >> + if (!x || !x->dev) {
> >> >> + dev_err(x->dev, "no PHY or attached device
> >> >> available\n");
> >> >> + return;
> >> >> + }
> >> >> +
> >> >> + pm_runtime_enable(x->dev);
> >> >> +}
> >> >
> >> >
> >> > IMO we need not have wrapper APIs for runtime_enable and runtime_disable
> >> > here. Generally runtime_enable and runtime_disable is done in probe and
> >> > remove of a driver respectively. So it's better to leave the
> >> > runtime_enable/runtime_disable to be done in *phy provider* driver than
> >> > having an API for it to be done by *phy user* driver. Felipe, what do you
> >> > think?
> >>
> >> Thanks!!
> >> That's very true, runtime_enable() and runtime_disable() calls are made by
> >> *phy_provider* only. But a querry here.
> >> Wouldn't in any case a PHY consumer might want to disable runtime_pm on
> >> PHY ?
> >> Say, when consumer failed to suspend the PHY properly
> >> (*put_sync(phy->dev)* fails), how much sure is the consumer about the
> >> state of PHY ?
> >
> > no no, wait a minute. We might not want to enable runtime pm for the PHY
> > until the UDC says it can handle runtime pm, no ? I guess this makes a
> > bit of sense (at least in my head :-p).
> >
> > Imagine if PHY is runtime suspended but e.g. DWC3 isn't runtime pm
> > enabled... Does it make sense to leave that control to the USB
> > controller drivers ?
> >
> > I'm open for suggestions
>
> Of course unless the PHY consumer can handle runtime PM for PHY,
> PHY should not ideally be going into runtime_suspend.
>
> Actually trying out few things, here are my observations
>
> Enabling runtime_pm on PHY pushes PHY to go into runtime_suspend state.
> But a device detection wakes up DWC3 controller, and if i don't wake
> up PHY (using get_sync(phy->dev)) here
> in runtime_resume() callback of DWC3, i don't get PHY back in active state.
> So it becomes the duty of DWC3 controller to handle PHY's sleep and wake-up.
> Thereby it becomes logical that DWC3 controller has the right to
> enable runtime_pm
> of PHY.
>
> But there's a catch here. if there are multiple consumers of PHY (like
> USB2 type PHY can
> have DWC3 controller as well as EHCI/OHCI or even HSGadget) then in that case,
> only one of the consumer can enable runtime_pm on PHY. So who decides this.
>
> Aargh!! lot of confusion here :-(
hmmm, maybe add a flag to struct usb_phy and check it on
usb_phy_autopm_enable() ??
How does usbcore handle it ? They request class drivers to pass
supports_autosuspend, but while we should have a similar flag, that's
not enough. We also need a flag to tell us when pm_runtime has already
been enabled.
So how about:
usb_phy_autopm_enable()
{
if (!phy->suports_autosuspend)
return -ENOSYS;
if (phy->autosuspend_enabled)
return 0;
phy->autosuspend_enabled = true;
return pm_runtime_enable(phy->dev);
}
???
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
Hi,
On Wed, Apr 03, 2013 at 04:54:14PM +0300, Felipe Balbi wrote:
> > >> >> +static inline void usb_phy_autopm_enable(struct usb_phy *x)
> > >> >> +{
> > >> >> + if (!x || !x->dev) {
> > >> >> + dev_err(x->dev, "no PHY or attached device
> > >> >> available\n");
> > >> >> + return;
> > >> >> + }
> > >> >> +
> > >> >> + pm_runtime_enable(x->dev);
> > >> >> +}
> > >> >
> > >> >
> > >> > IMO we need not have wrapper APIs for runtime_enable and
> > >> > runtime_disable
> > >> > here. Generally runtime_enable and runtime_disable is done in probe and
> > >> > remove of a driver respectively. So it's better to leave the
> > >> > runtime_enable/runtime_disable to be done in *phy provider* driver than
> > >> > having an API for it to be done by *phy user* driver. Felipe, what do
> > >> > you
> > >> > think?
> > >>
> > >> Thanks!!
> > >> That's very true, runtime_enable() and runtime_disable() calls are made
> > >> by
> > >> *phy_provider* only. But a querry here.
> > >> Wouldn't in any case a PHY consumer might want to disable runtime_pm on
> > >> PHY ?
> > >> Say, when consumer failed to suspend the PHY properly
> > >> (*put_sync(phy->dev)* fails), how much sure is the consumer about the
> > >> state of PHY ?
> > >
> > > no no, wait a minute. We might not want to enable runtime pm for the PHY
> > > until the UDC says it can handle runtime pm, no ? I guess this makes a
> > > bit of sense (at least in my head :-p).
> > >
> > > Imagine if PHY is runtime suspended but e.g. DWC3 isn't runtime pm
> > > enabled... Does it make sense to leave that control to the USB
> > > controller drivers ?
> > >
> > > I'm open for suggestions
> >
> > Of course unless the PHY consumer can handle runtime PM for PHY,
> > PHY should not ideally be going into runtime_suspend.
> >
> > Actually trying out few things, here are my observations
> >
> > Enabling runtime_pm on PHY pushes PHY to go into runtime_suspend state.
> > But a device detection wakes up DWC3 controller, and if i don't wake
> > up PHY (using get_sync(phy->dev)) here
> > in runtime_resume() callback of DWC3, i don't get PHY back in active state.
> > So it becomes the duty of DWC3 controller to handle PHY's sleep and wake-up.
> > Thereby it becomes logical that DWC3 controller has the right to
> > enable runtime_pm
> > of PHY.
> >
> > But there's a catch here. if there are multiple consumers of PHY (like
> > USB2 type PHY can
> > have DWC3 controller as well as EHCI/OHCI or even HSGadget) then in that
> > case,
> > only one of the consumer can enable runtime_pm on PHY. So who decides this.
> >
> > Aargh!! lot of confusion here :-(
>
>
> hmmm, maybe add a flag to struct usb_phy and check it on
> usb_phy_autopm_enable() ??
>
> How does usbcore handle it ? They request class drivers to pass
> supports_autosuspend, but while we should have a similar flag, that's
> not enough. We also need a flag to tell us when pm_runtime has already
> been enabled.
>
> So how about:
>
> usb_phy_autopm_enable()
> {
> if (!phy->suports_autosuspend)
> return -ENOSYS;
>
> if (phy->autosuspend_enabled)
> return 0;
>
> phy->autosuspend_enabled = true;
> return pm_runtime_enable(phy->dev);
> }
>
> ???
and of course I missed the fact that pm_runtime_enable returns nothing
:-) But you got my point.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 0/3] check regulator_enable() return value
Hi, On Wed, Apr 03, 2013 at 04:02:24PM +0200, Fabio Baltieri wrote: > While testing your 'next' branch merged with today's next I got some new > warnings, caused by a recently introduced __must_check in: > > c8801a8 regulator: core: Mark all get and enable calls as __must_check > > These patches introduces a check for regulator_enable() return value in > all three affected USB phy drivers, and issue a dev_err() in case it > fails. > > TWL4030 and TWL6030 patches has been build-tested only. Sorry but I can't change my tree anymore, we can send these during v3.10-rc. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
Hi,
On Wed, Apr 03, 2013 at 07:40:44PM +0530, Vivek Gautam wrote:
> >> > >> >> +static inline void usb_phy_autopm_enable(struct usb_phy *x)
> >> > >> >> +{
> >> > >> >> + if (!x || !x->dev) {
> >> > >> >> + dev_err(x->dev, "no PHY or attached device
> >> > >> >> available\n");
> >> > >> >> + return;
> >> > >> >> + }
> >> > >> >> +
> >> > >> >> + pm_runtime_enable(x->dev);
> >> > >> >> +}
> >> > >> >
> >> > >> >
> >> > >> > IMO we need not have wrapper APIs for runtime_enable and
> >> > >> > runtime_disable
> >> > >> > here. Generally runtime_enable and runtime_disable is done in probe
> >> > >> > and
> >> > >> > remove of a driver respectively. So it's better to leave the
> >> > >> > runtime_enable/runtime_disable to be done in *phy provider* driver
> >> > >> > than
> >> > >> > having an API for it to be done by *phy user* driver. Felipe, what
> >> > >> > do you
> >> > >> > think?
> >> > >>
> >> > >> Thanks!!
> >> > >> That's very true, runtime_enable() and runtime_disable() calls are
> >> > >> made by
> >> > >> *phy_provider* only. But a querry here.
> >> > >> Wouldn't in any case a PHY consumer might want to disable runtime_pm
> >> > >> on PHY ?
> >> > >> Say, when consumer failed to suspend the PHY properly
> >> > >> (*put_sync(phy->dev)* fails), how much sure is the consumer about the
> >> > >> state of PHY ?
> >> > >
> >> > > no no, wait a minute. We might not want to enable runtime pm for the
> >> > > PHY
> >> > > until the UDC says it can handle runtime pm, no ? I guess this makes a
> >> > > bit of sense (at least in my head :-p).
> >> > >
> >> > > Imagine if PHY is runtime suspended but e.g. DWC3 isn't runtime pm
> >> > > enabled... Does it make sense to leave that control to the USB
> >> > > controller drivers ?
> >> > >
> >> > > I'm open for suggestions
> >> >
> >> > Of course unless the PHY consumer can handle runtime PM for PHY,
> >> > PHY should not ideally be going into runtime_suspend.
> >> >
> >> > Actually trying out few things, here are my observations
> >> >
> >> > Enabling runtime_pm on PHY pushes PHY to go into runtime_suspend state.
> >> > But a device detection wakes up DWC3 controller, and if i don't wake
> >> > up PHY (using get_sync(phy->dev)) here
> >> > in runtime_resume() callback of DWC3, i don't get PHY back in active
> >> > state.
> >> > So it becomes the duty of DWC3 controller to handle PHY's sleep and
> >> > wake-up.
> >> > Thereby it becomes logical that DWC3 controller has the right to
> >> > enable runtime_pm
> >> > of PHY.
> >> >
> >> > But there's a catch here. if there are multiple consumers of PHY (like
> >> > USB2 type PHY can
> >> > have DWC3 controller as well as EHCI/OHCI or even HSGadget) then in that
> >> > case,
> >> > only one of the consumer can enable runtime_pm on PHY. So who decides
> >> > this.
> >> >
> >> > Aargh!! lot of confusion here :-(
> >>
> >>
> >> hmmm, maybe add a flag to struct usb_phy and check it on
> >> usb_phy_autopm_enable() ??
> >>
> >> How does usbcore handle it ? They request class drivers to pass
> >> supports_autosuspend, but while we should have a similar flag, that's
> >> not enough. We also need a flag to tell us when pm_runtime has already
> >> been enabled.
>
> True
>
> >>
> >> So how about:
> >>
> >> usb_phy_autopm_enable()
> >> {
> >> if (!phy->suports_autosuspend)
> >> return -ENOSYS;
> >>
> >> if (phy->autosuspend_enabled)
> >> return 0;
> >>
> >> phy->autosuspend_enabled = true;
> >> return pm_runtime_enable(phy->dev);
> >> }
> >>
> >> ???
>
> Great
>
> >
> > and of course I missed the fact that pm_runtime_enable returns nothing
> > :-) But you got my point.
>
> Yea, this is a way around this. :-)
>
> Just one more query :-)
>
> Lets suppose DWC3 enables runtime_pm on USB 2 type phy,
> it will try to go into suspend state and thereby call runtime_suspend(), if
> any.
> And PHY will come to active state only when its consumer wakes it up,
> and this consumer is operational
> only when its related PHY is in fully functional state.
> So do we have a situation in which this PHY goes into low power state
> in its runtime_suspend(),
> resulting in non-detection of devices on further attach (since PHY is
> in low power state) ?
>
> Will the controller (like EHCI/OHCI) be functional now ?
ehci/ohci need to cope with that by calling usb_phy_autopm_get_sync(),
right ? (so does DWC3 :-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
hi,
On Wed, Apr 03, 2013 at 07:48:42PM +0530, Kishon Vijay Abraham I wrote:
> >>+struct phy *of_phy_xlate(struct phy *phy, struct of_phandle_args *args)
> >>+{
> >>+ return phy;
> >>+}
> >>+EXPORT_SYMBOL_GPL(of_phy_xlate);
> >
> >so you get a PHY and just return it ? What gives ?? (maybe I skipped
> >some of the discussion...)
>
> hmm.. this is for the common case where the PHY provider implements
> only one PHY. And both phy provider and phy_instance is represented
> by struct phy *.
>
> For the case where PHY provider implements multiple PHYs (here it
> will have a single dt node), the PHY provider will implement it's own
> version of of_xlate that takes *of_phandle_args* as argument and
> finds the appropriate PHY.
got it.
> >>+struct phy *of_phy_get(struct device *dev, int index)
> >>+{
> >>+ int ret;
> >>+ struct phy *phy = NULL;
> >>+ struct phy_bind *phy_map = NULL;
> >>+ struct of_phandle_args args;
> >>+ struct device_node *node;
> >>+
> >>+ if (!dev->of_node) {
> >>+ dev_dbg(dev, "device does not have a device node entry\n");
> >>+ return ERR_PTR(-EINVAL);
> >>+ }
> >>+
> >>+ ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
> >>+ index, &args);
> >>+ if (ret) {
> >>+ dev_dbg(dev, "failed to get phy in %s node\n",
> >>+ dev->of_node->full_name);
> >>+ return ERR_PTR(-ENODEV);
> >>+ }
> >>+
> >>+ phy = of_phy_lookup(args.np);
> >>+ if (IS_ERR(phy) || !try_module_get(phy->ops->owner)) {
> >>+ phy = ERR_PTR(-EPROBE_DEFER);
> >>+ goto err0;
> >>+ }
> >>+
> >>+ phy = phy->ops->of_xlate(phy, &args);
> >
> >alright, so of_xlate() is optional, am I right ? How about not
>
> Not really. of_xlate is mandatory (it's even checked in phy_create).
> Either the PHY provider can implement it's own version or use the
> implementation above (by filling the function pointer).
alright.
> >implementing the above and have a check for of_xlate() being a valid
> >pointer here ?
>
> Having the way it is actually mandates the PHY providers to always
> provide of_xlate which IMO is better since some PHY providers wont
> accidentally be using the default implementation.
ok cool, thanks for clarifying.
> >>+ ret = -EINVAL;
> >>+ goto err0;
> >>+ }
> >>+
> >>+ if (!phy_class)
> >>+ phy_core_init();
> >
> >why don't you setup the class on module_init ? Then this would be a
> >terrible error condition here :-)
>
> This is for the case where the PHY driver gets loaded before the PHY
> framework. I could have returned EPROBE_DEFER here instead I thought
> will have it this way.
looks a bit weird IMO. Is it really possible for PHY to load before ?
Since PHY driver uses symbols from phy-core, modprobe will make sure to
load drivers based on symbol dependency, right ?
> >>+static struct device_attribute phy_dev_attrs[] = {
> >>+ __ATTR(label, 0444, phy_name_show, NULL),
> >>+ __ATTR(phy_bind, 0444, phy_bind_show, NULL),
> >
> >you could expose a human-readable 'type' string. BTW, how are you using
> >type ? USB2/USB3/etc ? Have you considered our OMAP5 SerDes pair which
>
> Actually not using *type* anywhere. Just used as an additional info
> about the PHY. It's actually not even enum. Maybe I can remove it
> completely.
makes sense.
> >are currently for USB3 and SATA (and could just as easily be used for
> >PCIe)
>
> Yeah. Me and Balaji were planning to work on it for having a single
> driver to be used for all the above.
cool :-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 0/3] check regulator_enable() return value
Hi, On Wed, Apr 03, 2013 at 07:22:38AM -0700, Greg KH wrote: > On Wed, Apr 03, 2013 at 05:06:22PM +0300, Felipe Balbi wrote: > > Hi, > > > > On Wed, Apr 03, 2013 at 04:02:24PM +0200, Fabio Baltieri wrote: > > > While testing your 'next' branch merged with today's next I got some new > > > warnings, caused by a recently introduced __must_check in: > > > > > > c8801a8 regulator: core: Mark all get and enable calls as __must_check > > > > > > These patches introduces a check for regulator_enable() return value in > > > all three affected USB phy drivers, and issue a dev_err() in case it > > > fails. > > > > > > TWL4030 and TWL6030 patches has been build-tested only. > > > > Sorry but I can't change my tree anymore, we can send these during > > v3.10-rc. > > Really? You are going to send me a tree that adds build warnings? > > Please don't. alright, I'll merge these in. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
Hi, On Wed, Apr 03, 2013 at 08:02:52PM +0530, Kishon Vijay Abraham I wrote: > + ret = -EINVAL; > + goto err0; > + } > + > + if (!phy_class) > + phy_core_init(); > >>> > >>>why don't you setup the class on module_init ? Then this would be a > >>>terrible error condition here :-) > >> > >>This is for the case where the PHY driver gets loaded before the PHY > >>framework. I could have returned EPROBE_DEFER here instead I thought > >>will have it this way. > > > >looks a bit weird IMO. Is it really possible for PHY to load before ? > > yeah. it actually happened when I tried with beagle and had all the > modules as built-in. Because twl4030 has subsys_initcall(), it loads > before PHY framework. that's a bug in twl4030. -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 2/6] usb: phy: omap-usb2: use the new generic PHY framework
Hi,
On Wed, Apr 03, 2013 at 02:55:47PM +, Arnd Bergmann wrote:
> On Wednesday 03 April 2013, Felipe Balbi wrote:
> > const ? Maybe provide a:
> >
> > #define DEFINE_PHY_OPS(name)\
> > const struct phy_ops #name_phy_ops = {
> >
> > macro ? This will force people to add the const keyword :-)
>
> Forcing people to use const structures is good, but I think it would be
> better without the macro, just by marking the argument in
> devm_phy_create() as const.
that won't force the definition of the struct to be const, however. But
I get your point.
--
balbi
signature.asc
Description: Digital signature
Re: linux-next: manual merge of the usb-gadget tree with the usb tree
On Thu, Apr 04, 2013 at 08:27:30AM +0200, Thierry Reding wrote:
> On Thu, Apr 04, 2013 at 03:18:45PM +1100, Stephen Rothwell wrote:
> > Hi Felipe,
> >
> > Today's linux-next merge of the usb-gadget tree got a conflict in
> > drivers/usb/host/ehci-tegra.c between commit 369a9a9d2af7 ("usb: host:
> > ehci-tegra: Fix oops in error cleanup") from the usb tree and commit
> > 4261b8f3538c ("usb: host: ehci-tegra: fix PHY error handling") from the
> > usb-gadget tree.
> >
> > I fixed it up (I think - see below) and can carry the fix as necessary
> > (no action is required).
> >
> > --
> > Cheers,
> > Stephen Rothwells...@canb.auug.org.au
> >
> > diff --cc drivers/usb/host/ehci-tegra.c
> > index 4f3cfb8,1d2488c..000
> > --- a/drivers/usb/host/ehci-tegra.c
> > +++ b/drivers/usb/host/ehci-tegra.c
> > @@@ -770,19 -765,15 +770,17 @@@ static int tegra_ehci_probe(struct plat
> > if (!irq) {
> > dev_err(&pdev->dev, "Failed to get IRQ\n");
> > err = -ENODEV;
> > - goto fail;
> > + goto fail_phy;
> > }
> >
> > if (pdata->operating_mode == TEGRA_USB_OTG) {
> > tegra->transceiver =
> > devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
> > - if (!IS_ERR_OR_NULL(tegra->transceiver))
> > + if (!IS_ERR(tegra->transceiver))
> > otg_set_host(tegra->transceiver->otg, &hcd->self);
> > + } else {
> > + tegra->transceiver = ERR_PTR(-ENODEV);
> > }
> > - #endif
> >
> > err = usb_add_hcd(hcd, irq, IRQF_SHARED);
> > if (err) {
> > @@@ -801,11 -792,8 +799,9 @@@
> > return err;
> >
> > fail:
> > - #ifdef CONFIG_USB_OTG_UTILS
> > - if (!IS_ERR_OR_NULL(tegra->transceiver))
> > + if (!IS_ERR(tegra->transceiver))
> > otg_set_host(tegra->transceiver->otg, NULL);
> > - #endif
> > +fail_phy:
> > usb_phy_shutdown(hcd->phy);
> > fail_io:
> > clk_disable_unprepare(tegra->clk);
>
> Looks good to me, thanks.
looks correct indeed, thanks
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 00/11] usb: dwc3/xhci/phy: Enable runtime power management
On Thu, Apr 04, 2013 at 10:34:57AM +0530, Vivek Gautam wrote: > Hi Sarah, > > > On Wed, Apr 3, 2013 at 10:57 PM, Sarah Sharp > wrote: > > Question: Do you still need this patch for 3.10? > > Felipe's 'next' is closed for 3.10, so this series won't be making it > to 3.10 now, as a whole. :-( right, besides we're still discussing what to do with the whole PHY part, right ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
Hi,
On Wed, Apr 03, 2013 at 05:54:11PM -0600, Stephen Warren wrote:
> struct phy {
> struct device *dev;
> struct module *owner;
> int (*init)(struct phy *phy);
> int (*exit)(struct phy *phy);
> int (*suspend)(struct phy *phy);
> int (*resume)(struct phy *phy);
I wonder whether it makes sense to provide ->suspend/->resume callbacks
here. We already have dev_pm_ops providing those methods and we can just
wrap pm_runtime_* calls to be called by consumers.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
Hi, On Wed, Apr 03, 2013 at 02:14:02PM -0400, Alan Stern wrote: > > > Lets suppose DWC3 enables runtime_pm on USB 2 type phy, > > > it will try to go into suspend state and thereby call runtime_suspend(), > > > if any. > > > And PHY will come to active state only when its consumer wakes it up, > > > and this consumer is operational > > > only when its related PHY is in fully functional state. > > > So do we have a situation in which this PHY goes into low power state > > > in its runtime_suspend(), > > > resulting in non-detection of devices on further attach (since PHY is > > > in low power state) ? > > > > > > Will the controller (like EHCI/OHCI) be functional now ? > > > > ehci/ohci need to cope with that by calling usb_phy_autopm_get_sync(), > > right ? (so does DWC3 :-) > > Maybe you guys have already got this all figured out -- if so, feel > free to ignore this email. > > Some subsystems handle this issue by calling pm_runtime_get_sync() > before probing a driver and pm_runtime_put_sync() after unbinding the > driver. If the driver is runtime-PM-enabled, it then does its own > put_sync near the end of its probe routine and get_sync in its release > routine. sounds a bit 'fishy' to me... So a separate entity would call pm_runtime_get_sync(), even when we don't have registered dev_pm_ops, then drivers need to check if runtime_pm is enabled and call pm_runtime_put*() conditionally before returning from probe(). One remove, we might have another issue: device is already runtime_suspended (due to e.g. autosuspend) when module is removed, a call to pm_runtime_put_sync() will be unbalanced. No ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 01/11] usb: phy: Add APIs for runtime power management
Hi, On Thu, Apr 04, 2013 at 02:26:51PM +0530, Vivek Gautam wrote: > >> > > Lets suppose DWC3 enables runtime_pm on USB 2 type phy, > >> > > it will try to go into suspend state and thereby call > >> > > runtime_suspend(), if any. > >> > > And PHY will come to active state only when its consumer wakes it up, > >> > > and this consumer is operational > >> > > only when its related PHY is in fully functional state. > >> > > So do we have a situation in which this PHY goes into low power state > >> > > in its runtime_suspend(), > >> > > resulting in non-detection of devices on further attach (since PHY is > >> > > in low power state) ? > >> > > > >> > > Will the controller (like EHCI/OHCI) be functional now ? > >> > > >> > ehci/ohci need to cope with that by calling usb_phy_autopm_get_sync(), > >> > right ? (so does DWC3 :-) > >> > >> Maybe you guys have already got this all figured out -- if so, feel > >> free to ignore this email. > >> > >> Some subsystems handle this issue by calling pm_runtime_get_sync() > >> before probing a driver and pm_runtime_put_sync() after unbinding the > >> driver. If the driver is runtime-PM-enabled, it then does its own > >> put_sync near the end of its probe routine and get_sync in its release > >> routine. > > > > sounds a bit 'fishy' to me... So a separate entity would call > > pm_runtime_get_sync(), even when we don't have registered dev_pm_ops, > > then drivers need to check if runtime_pm is enabled and call > > pm_runtime_put*() conditionally before returning from probe(). One > > remove, we might have another issue: device is already runtime_suspended > > (due to e.g. autosuspend) when module is removed, a call to > > pm_runtime_put_sync() will be unbalanced. No ? > > May be i am misinterpreting !! > If PHYs are runtime-PM enabled (PHY probe calls *runtime_enable*), > then the consumers > need to call pm_runtime_get_sync whever they want to access PHY. Alright, so here's my understanding: I suggested letting e.g. DWC3 enable the PHY's runtime_pm; Alan said that it could be done before that so that DWC3 sees an enabled PHY during probe. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/5 v3] USB: regroup all depends on USB within an if USB block
Hi, On Thu, Apr 04, 2013 at 05:57:24PM +0200, Florian Fainelli wrote: > diff --git a/drivers/usb/misc/sisusbvga/Kconfig > b/drivers/usb/misc/sisusbvga/Kconfig > index 30ea7ca..0d03a52 100644 > --- a/drivers/usb/misc/sisusbvga/Kconfig > +++ b/drivers/usb/misc/sisusbvga/Kconfig > @@ -1,7 +1,7 @@ > > config USB_SISUSBVGA > tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" > - depends on USB && (USB_MUSB_HDRC || USB_EHCI_HCD) > + depends on (USB_MUSB_HDRC || USB_EHCI_HCD) is it just me or would everybody agree that depending on MUSB or EHCI here is wrong ? > diff --git a/drivers/usb/mon/Kconfig b/drivers/usb/mon/Kconfig > index 635745f..5c6ffa2 100644 > --- a/drivers/usb/mon/Kconfig > +++ b/drivers/usb/mon/Kconfig > @@ -4,7 +4,6 @@ > > config USB_MON > tristate "USB Monitor" > - depends on USB > help > If you select this option, a component which captures the USB traffic > between peripheral-specific drivers and HC drivers will be built. > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig > index 05e5143..4636ab9 100644 > --- a/drivers/usb/musb/Kconfig > +++ b/drivers/usb/musb/Kconfig > @@ -6,7 +6,7 @@ > # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller > config USB_MUSB_HDRC > tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)' > - depends on USB && USB_GADGET > + depends on USB_GADGET great, except for the question above: Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/5 v3] USB: regroup all depends on USB within an if USB block
Hi, On Thu, Apr 04, 2013 at 01:42:18PM -0400, Alan Stern wrote: > > > diff --git a/drivers/usb/misc/sisusbvga/Kconfig > > > b/drivers/usb/misc/sisusbvga/Kconfig > > > index 30ea7ca..0d03a52 100644 > > > --- a/drivers/usb/misc/sisusbvga/Kconfig > > > +++ b/drivers/usb/misc/sisusbvga/Kconfig > > > @@ -1,7 +1,7 @@ > > > > > > config USB_SISUSBVGA > > > tristate "USB 2.0 SVGA dongle support (Net2280/SiS315)" > > > - depends on USB && (USB_MUSB_HDRC || USB_EHCI_HCD) > > > + depends on (USB_MUSB_HDRC || USB_EHCI_HCD) > > > > is it just me or would everybody agree that depending on MUSB or EHCI > > here is wrong ? > > That line certainly looks like it could be removed entirely. Perhaps > the original author can enlighten us. > > In any case, it's not relevant to the purpose of this patch set. right :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 10/30] USB: ehci-omap: Use PHY APIs to get the PHY device and put it out of suspend
Hi, On Tue, Jan 29, 2013 at 11:50:32AM +0200, Roger Quadros wrote: > For each port that is in PHY mode we obtain a PHY device using the USB PHY > library and put it out of suspend. > > It is up to platform code to associate the PHY to the controller's > port and it is upto the PHY driver to manage the PHY's resources. > > Also remove wired spacing around declarations we come across. > > Signed-off-by: Roger Quadros ideally, this would be done generically by ehci-hcd.ko itself. Alan, would you have objections provided it doesn't break anyone else ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 10/30] USB: ehci-omap: Use PHY APIs to get the PHY device and put it out of suspend
On Tue, Jan 29, 2013 at 12:08:23PM +0200, Roger Quadros wrote: > On 01/29/2013 11:57 AM, Felipe Balbi wrote: > > Hi, > > > > On Tue, Jan 29, 2013 at 11:50:32AM +0200, Roger Quadros wrote: > >> For each port that is in PHY mode we obtain a PHY device using the USB PHY > >> library and put it out of suspend. > >> > >> It is up to platform code to associate the PHY to the controller's > >> port and it is upto the PHY driver to manage the PHY's resources. > >> > >> Also remove wired spacing around declarations we come across. > >> > >> Signed-off-by: Roger Quadros > > > > ideally, this would be done generically by ehci-hcd.ko itself. Alan, > > would you have objections provided it doesn't break anyone else ? > > > Agreed, and PHY suspend/resume should be done at port granularity. right. > But considering the erratas we have in OMAP EHCI, I would still prefer to > have control of the PHY in ehci-omap. We might even have to do some ULPI > transfers in certain scenarios to work around some of the erratas. fair enough, then let's start with your change and make it more generic later. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 2/2] usb: phy: samsung: Add PHY support for USB 3.0 controller
On Tue, Jan 29, 2013 at 10:01:52PM -0800, Kukjin Kim wrote: > Vivek Gautam wrote: > > > > Adding PHY driver support for USB 3.0 controller for Samsung's > > SoCs. > > > > Signed-off-by: Vivek Gautam > > --- > > > > Changes from v3: > > - Making SAMSUNG_USB3PHY dependent on SAMSUNG_USBPHY. > > - Adding USB_DWC3 to dependencies of SAMSUNG_USB2PHY since > >dwc3 controller also looks for USB2 type PHY. > > > > drivers/usb/phy/Kconfig | 11 +- > > drivers/usb/phy/Makefile |1 + > > drivers/usb/phy/samsung-usb3.c | 349 > > ++ > > drivers/usb/phy/samsung-usbphy.h | 81 + > > 4 files changed, 441 insertions(+), 1 deletions(-) > > create mode 100644 drivers/usb/phy/samsung-usb3.c > > > > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > > index cc0d230..9325a95 100644 > > --- a/drivers/usb/phy/Kconfig > > +++ b/drivers/usb/phy/Kconfig > > @@ -52,14 +52,23 @@ config SAMSUNG_USBPHY > > help > > Enable this to support Samsung USB phy controllers for Samsung > > SoCs. > > + Further enable USB 2.0 type PHY or USB 3.0 type PHY as required > > + for USB controllers in use. > > > > if SAMSUNG_USBPHY > > > > config SAMSUNG_USB2PHY > > bool "Samsung USB 2.0 PHY controller Driver" > > - depends on USB_S3C_HSOTG || USB_EHCI_S5P || > > USB_OHCI_EXYNOS > > + depends on USB_S3C_HSOTG || USB_EHCI_S5P || > > USB_OHCI_EXYNOS || USB_DWC3 > > help > > Enable this to support Samsung USB 2.0 (High Speed) phy controller > > for Samsung SoCs. > > > > +config SAMSUNG_USB3PHY > > + bool "Samsung USB 3.0 PHY controller Driver" > > + depends on USB_DWC3 > > + help > > + Enable this to support Samsung USB 3.0 (Super Speed) phy > > controller > > + for samsung SoCs. > > + > > endif > > It mean, when USB_DWC3 is selected, we can select only one USB2PHY or > USB3PHY? right, not sure that's a wise idea. It makes enabling USB support trickier than it needs to be. How about just dropping all dependencies if it compiles cleanly on all arches ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 00/30] USB: omap-ehci: Move PHY management to PHY driver
Hi, On Wed, Jan 30, 2013 at 02:48:50PM +0200, Roger Quadros wrote: > On 01/30/2013 02:08 PM, Mohammed, Afzal wrote: > > Hi Roger, > > > > On Mon, Jan 28, 2013 at 17:00:01, Quadros, Roger wrote: > > > >> NOTE: Tested on 4460ES-B1 Panda and BeagleBoard C4 only. Other boards are > >> only > >> build tested. > > > > Please try to ensure that beagle bone USB0 works with this. > > > > Hi Afzal, > > I do not have beaglebone with me and don't seem to find beaglebone > board support in mainline. beagle bone is DT-only :-) (see arch/arm/boot/dts/am335x-bone.dts) > I would appreciate if someone who knows more about beaglebone can help > with verification. > > I could find that Robert C Nelson (now in cc) is maintaining out of tree > patches for the beaglebone. Maybe he could help with verification? Right, Afzal if you have the bone could you help Roger validating ? -- balbi signature.asc Description: Digital signature
Re: [PATCH RFC] usb: dwc3: Get PHY from platform specific dwc3 dt node.
Hi, On Thu, Jan 31, 2013 at 08:53:27PM +0530, Vivek Gautam wrote: > >> Moreover, SoCs having multiple dwc3 controllers will have multiple > >> PHYs, which eventually be added using usb_add_phy_dev(), and not > >> using usb_add_phy(). So each dwc3 controller won't be able to > >> get PHYs by simply calling devm_usb_get_phy() also. > > > > No. We have added usb_get_phy_dev() for that purpose in the case of non-dt. > > I think, instead you can have a patch to use devm_usb_get_phy_dev() here and > > in exynos platform specific code use usb_bind_phy() to bind the phy and > > controller till you change it to dt. > > > > We have dt support for dwc3-exynos, in such case we should go ahead with > of_platform_populate(), right ? > But if when i use of_platform_populate() i will not be able to set > dma_mask to dwc3->dev. :-( do you have a special need for dma_mask because OF already sets it. -- balbi signature.asc Description: Digital signature
Re: [PATCH RFC] usb: dwc3: Get PHY from platform specific dwc3 dt node.
On Thu, Jan 31, 2013 at 09:00:37PM +0530, Vivek Gautam wrote: > Hi Felipe, > > > On Thu, Jan 31, 2013 at 8:55 PM, Felipe Balbi wrote: > > Hi, > > > > On Thu, Jan 31, 2013 at 08:53:27PM +0530, Vivek Gautam wrote: > >> >> Moreover, SoCs having multiple dwc3 controllers will have multiple > >> >> PHYs, which eventually be added using usb_add_phy_dev(), and not > >> >> using usb_add_phy(). So each dwc3 controller won't be able to > >> >> get PHYs by simply calling devm_usb_get_phy() also. > >> > > >> > No. We have added usb_get_phy_dev() for that purpose in the case of > >> > non-dt. > >> > I think, instead you can have a patch to use devm_usb_get_phy_dev() here > >> > and > >> > in exynos platform specific code use usb_bind_phy() to bind the phy and > >> > controller till you change it to dt. > >> > > >> > >> We have dt support for dwc3-exynos, in such case we should go ahead with > >> of_platform_populate(), right ? > >> But if when i use of_platform_populate() i will not be able to set > >> dma_mask to dwc3->dev. :-( > > > > do you have a special need for dma_mask because OF already sets it. > > > If i am not wrong of_platform_device_create_pdata() will set > "dev->dev.coherent_dma_mask = DMA_BIT_MASK(32)" > and not dma_mask. > I fact we had some discussion sometime back when we needed the same > for dwc3-exynos in the thread: > [PATCH v2 1/2] USB: dwc3-exynos: Add support for device tree > > But couldn't get final node on it. > So suggestions here please. :-) hmm.. you're right there. Grant, Rob ? Any hints ? -- balbi signature.asc Description: Digital signature
Re: [PATCH RFC] usb: dwc3: Get PHY from platform specific dwc3 dt node.
On Fri, Feb 01, 2013 at 11:54:01AM +0530, Vivek Gautam wrote:
> Hi Balbi,
>
>
> On Fri, Feb 1, 2013 at 11:52 AM, Vivek Gautam
> wrote:
> > Hi Kishon,
> >
> >
> > On Fri, Feb 1, 2013 at 10:51 AM, kishon wrote:
> >> Hi,
> >>
> >>
> >> On Thursday 31 January 2013 09:08 PM, Felipe Balbi wrote:
> >>>
> >>> On Thu, Jan 31, 2013 at 09:00:37PM +0530, Vivek Gautam wrote:
> >>>>
> >>>> Hi Felipe,
> >>>>
> >>>>
> >>>> On Thu, Jan 31, 2013 at 8:55 PM, Felipe Balbi wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> On Thu, Jan 31, 2013 at 08:53:27PM +0530, Vivek Gautam wrote:
> >>>>>>>>
> >>>>>>>> Moreover, SoCs having multiple dwc3 controllers will have multiple
> >>>>>>>> PHYs, which eventually be added using usb_add_phy_dev(), and not
> >>>>>>>> using usb_add_phy(). So each dwc3 controller won't be able to
> >>>>>>>> get PHYs by simply calling devm_usb_get_phy() also.
> >>>>>>>
> >>>>>>>
> >>>>>>> No. We have added usb_get_phy_dev() for that purpose in the case of
> >>>>>>> non-dt.
> >>>>>>> I think, instead you can have a patch to use devm_usb_get_phy_dev()
> >>>>>>> here and
> >>>>>>> in exynos platform specific code use usb_bind_phy() to bind the phy
> >>>>>>> and
> >>>>>>> controller till you change it to dt.
> >>>>>>>
> >>>>>>
> >>>>>> We have dt support for dwc3-exynos, in such case we should go ahead
> >>>>>> with
> >>>>>> of_platform_populate(), right ?
> >>>>>> But if when i use of_platform_populate() i will not be able to set
> >>>>>> dma_mask to dwc3->dev. :-(
> >>
> >>
> >> You can do something like this
> >>
> >> static u64 dwc3_exynos_dma_mask = DMA_BIT_MASK(32);
> >>
> >> static int dwc3_exynos_set_dmamask(struct device *dev, void *c)
> >> {
> >> dev->dma_mask = &dwc3_exynos_dma_mask;
> >>
> >> return 0;
> >> }
> >>
> >> And in your probe after of_platform_populate, you can add
> >>
> >> device_for_each_child(&pdev->dev, NULL, dwc3_exynos_set_dmamask);
> >>
> >> Here pdev is the platform device of dwc3-exynos. By this way all the
> >> children of dwc3-exynos will have dma_mask set to the required value.
> >>
> >
> > Nice idea, thanks :-)
> > hmm.. so i can patch this now, and get things working ;-)
> >
>
> If this is fine with you shall i go ahead and post a patch for the same ? ;-)
should be fine, but we can wait a bit to see if DeviceTree folks reply,
your patch will only go on v3.10 anyway.
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH RFC] usb: dwc3: Get PHY from platform specific dwc3 dt node.
Hi,
On Fri, Feb 01, 2013 at 02:23:28PM +0530, Vivek Gautam wrote:
> >> > On Thu, Jan 31, 2013 at 08:53:27PM +0530, Vivek Gautam wrote:
> >>
> >> Moreover, SoCs having multiple dwc3 controllers will have multiple
> >> PHYs, which eventually be added using usb_add_phy_dev(), and not
> >> using usb_add_phy(). So each dwc3 controller won't be able to
> >> get PHYs by simply calling devm_usb_get_phy() also.
> >> >>>
> >> >>>
> >> >>> No. We have added usb_get_phy_dev() for that purpose in the case of
> >> >>> non-dt.
> >> >>> I think, instead you can have a patch to use devm_usb_get_phy_dev()
> >> >>> here and
> >> >>> in exynos platform specific code use usb_bind_phy() to bind the phy
> >> >>> and
> >> >>> controller till you change it to dt.
> >> >>>
> >> >>
> >> >> We have dt support for dwc3-exynos, in such case we should go ahead
> >> >> with
> >> >> of_platform_populate(), right ?
> >> >> But if when i use of_platform_populate() i will not be able to set
> >> >> dma_mask to dwc3->dev. :-(
> >> >>
> >> >>
> >> >> You can do something like this
> >> >>
> >> >> static u64 dwc3_exynos_dma_mask = DMA_BIT_MASK(32);
> >> >>
> >> >> static int dwc3_exynos_set_dmamask(struct device *dev, void *c)
> >> >> {
> >> >> dev->dma_mask = &dwc3_exynos_dma_mask;
> >> >>
> >> >> return 0;
> >> >> }
> >> >>
> >> >> And in your probe after of_platform_populate, you can add
> >> >>
> >> >> device_for_each_child(&pdev->dev, NULL, dwc3_exynos_set_dmamask);
> >> >>
> >> >> Here pdev is the platform device of dwc3-exynos. By this way all the
> >> >> children of dwc3-exynos will have dma_mask set to the required value.
> >> >>
> >> >
> >> > Nice idea, thanks :-)
> >> > hmm.. so i can patch this now, and get things working ;-)
> >> >
> >>
> >> If this is fine with you shall i go ahead and post a patch for the same ?
> >> ;-)
> >
> > should be fine, but we can wait a bit to see if DeviceTree folks reply,
> > your patch will only go on v3.10 anyway.
> >
>
> Yeah, sure. No problem at all.
> For the time being i will continue to use this change for my other
> development work too. :-)
Sure, makes sense ;-)
--
balbi
signature.asc
Description: Digital signature
