Re: [PATCH 00/15] musb: Add support for the Allwinner sunxi musb controller
Hi, On 10-03-15 02:46, Chen-Yu Tsai wrote: Hi Arnd, On Tue, Mar 10, 2015 at 5:44 AM, Arnd Bergmann wrote: On Monday 09 March 2015 21:40:13 Hans de Goede wrote: Hi All, This patch set has been a while in the making, so I'm very happy to present the end result here, and I hope everyone likes it. Awesome work! Before talking about merging this there are 2 things which I would like to point out: a) The musb controller in the sunxi SoCs uses some SRAM which needs to be mapped to the musb controller by poking some bits in the SRAM controller, just like the EMAC patches which were send a while back I've chosen to use syscon for this, actually 2 of the patches in this set come directly from the SRAM mapping patchset for the EMAC. I know that Maxime is not 100% in favor of using syscon: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/320221.html But I disagree with his arguments for writing a special driver for the SRAM controller: 1) syscon was specifically designed for global system control registers like this and is fine to use as long as there are no conflicts where 1 bit is of interest to multiple drivers, and there is no such conflict here 2) Maxime's other arguments seem to boil down to it would be nice / prettier to have a specific driver for this, without really proving a hard need for such a driver. But writing such a driver is going to be a lot of work, and we've a ton of other work to do, and as said there is no real need for a separate driver, syscon works fine for this. 3) I actually believe that having a specific driver for this is a bad idea, because that means inventing a whole new cross driver API for this, and getting those right is, hard, a lot of work, and even then one is still likely to get it wrong. We can avoid all this by going with the proven syscon solution. Maxime, can we please have your ack for moving forward with this using syscon? (see above for my arguments why) I'd like to understand here why we can't use the existing SRAM DT binding instead of the syscon binding. I believe you are talking about "mmio-sram"? The syscon here represents a switch, to toggle whether a block of SRAM is mapped into the CPU memory space, or to a specific devices private address space. It is not the actual SRAM. The SRAM DT binding is orthogonal to this, if not irrelevant when the block is mapped privately, as it is no longer visible from the DT's PoV. Coincidentally, on the A23 this is no longer needed. The SRAM for the FIFO is wholly owned by MUSB and not available to the CPU. What ChenYu said :) Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] musb: Do not use musb_read[b|w] / _write[b|w] wrappers in generic fifo functions
Hi, On 09-03-15 22:50, Arnd Bergmann wrote: On Monday 09 March 2015 21:40:18 Hans de Goede wrote: The generic fifo functions already use non wrapped accesses in various cases through the iowrite#_rep functions, and all platforms which override the default musb_read[b|w] / _write[b|w] functions also provide their own fifo access functions, so we can safely drop the unnecessary indirection from the fifo access functions. Signed-off-by: Hans de Goede The patch looks reasonably, but the description seem misleading. I believe the real reason why it's ok to use __raw_writew for the FIFO is that a FIFO by definition is using CPU endian access for copying byte streams from memory, which is unlike any other MMIO register that requires fixed-endian accessors. I'm not sure that that is the case here, this fifo allows reading 4 bytes at a time using 32 bit word access, so endianness may come into play. This patch is safe however since all existing users of the generic fifo_read / write helpers which this patch touches, are also using the generic musb_read[b|w] / _write[b|w] functions which are just __raw_foo wrappers, so there is no functional change, which is what the commit message tries to say. Note that sunxi needs this function because of the register address translation the sunxi_musb_readb / writeb wrappers are doing which does not know how to deal with fifo data, and besides that it should make the generic read / write fifo helpers somewhat faster by removing an indirect function call. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/19] usb: dwc2: add controller hibernation support
Hi,
On 03/09/2015 04:04 PM, Mian Yousaf Kaukab wrote:
> From: Gregory Herrero
>
> When suspending usb bus, phy driver may disable controller power.
> In this case, registers need to be saved on suspend and restored
> on resume.
>
> Signed-off-by: Gregory Herrero
> ---
> drivers/usb/dwc2/core.c | 376
>
> drivers/usb/dwc2/core.h | 84 +++
> 2 files changed, 460 insertions(+)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index d5197d4..e858062 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -56,6 +56,382 @@
> #include "core.h"
> #include "hcd.h"
>
> +#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> +/**
> + * dwc2_backup_host_registers() - Backup controller host registers.
> + * When suspending usb bus, registers needs to be backuped
> + * if controller power is disabled once suspended.
> + *
> + * @hsotg: Programming view of the DWC_otg controller
> + */
> +static int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
> +{
> + struct hregs_backup *hr;
> + int i;
> +
> + dev_dbg(hsotg->dev, "%s\n", __func__);
> +
> + /* Backup Host regs */
> + hr = hsotg->hr_backup;
> + if (!hr) {
> + hr = kmalloc(sizeof(*hr), GFP_KERNEL);
Where is kfree(hr)?
> + if (!hr) {
> + dev_err(hsotg->dev, "%s: can't allocate host regs\n",
> + __func__);
> + return -ENOMEM;
> + }
> +
> + hsotg->hr_backup = hr;
> + }
> + hr->hcfg = readl(hsotg->regs + HCFG);
> + hr->haintmsk = readl(hsotg->regs + HAINTMSK);
> + for (i = 0; i < hsotg->core_params->host_channels; ++i)
> + hr->hcintmsk[i] = readl(hsotg->regs + HCINTMSK(i));
> +
> + hr->hprt0 = readl(hsotg->regs + HPRT0);
> + hr->hfir = readl(hsotg->regs + HFIR);
> + return 0;
> +}
> +
> +/**
> + * dwc2_restore_host_registers() - Restore controller host registers.
> + * When resuming usb bus, device registers needs to be restored
> + * if controller power were disabled.
> + *
> + * @hsotg: Programming view of the DWC_otg controller
> + */
> +static int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg)
> +{
> + struct hregs_backup *hr;
> + int i;
> +
> + dev_dbg(hsotg->dev, "%s\n", __func__);
> +
> + /* Restore host regs */
> + hr = hsotg->hr_backup;
> + if (!hr) {
> + dev_err(hsotg->dev, "%s: no host registers to restore\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + writel(hr->hcfg, hsotg->regs + HCFG);
> + writel(hr->haintmsk, hsotg->regs + HAINTMSK);
> +
> + for (i = 0; i < hsotg->core_params->host_channels; ++i)
> + writel(hr->hcintmsk[i], hsotg->regs + HCINTMSK(i));
> +
> + writel(hr->hprt0, hsotg->regs + HPRT0);
> + writel(hr->hfir, hsotg->regs + HFIR);
> +
> + return 0;
> +}
> +#else
> +static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
> +{ return 0; }
> +
> +static inline int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg)
> +{ return 0; }
> +#endif
> +
> +#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \
> + IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> +/**
> + * dwc2_backup_device_registers() - Backup controller device registers.
> + * When suspending usb bus, registers needs to be backuped
> + * if controller power is disabled once suspended.
> + *
> + * @hsotg: Programming view of the DWC_otg controller
> + */
> +static int dwc2_backup_device_registers(struct dwc2_hsotg *hsotg)
> +{
> + struct dregs_backup *dr;
> + int i;
> +
> + dev_dbg(hsotg->dev, "%s\n", __func__);
> +
> + /* Backup dev regs */
> + dr = hsotg->dr_backup;
> + if (!dr) {
> + dr = kmalloc(sizeof(*dr), GFP_KERNEL);
Ditto, kfree(dr) needed.
> + if (!dr) {
> + dev_err(hsotg->dev, "%s: can't allocate device regs\n",
> + __func__);
> + return -ENOMEM;
> + }
> +
> + hsotg->dr_backup = dr;
> + }
> +
> + dr->dcfg = readl(hsotg->regs + DCFG);
> + dr->dctl = readl(hsotg->regs + DCTL);
> + dr->daintmsk = readl(hsotg->regs + DAINTMSK);
> + dr->diepmsk = readl(hsotg->regs + DIEPMSK);
> + dr->doepmsk = readl(hsotg->regs + DOEPMSK);
> +
> + for (i = 0; i < hsotg->num_of_eps; i++) {
> + /* Backup IN EPs */
> + dr->diepctl[i] = readl(hsotg->regs + DIEPCTL(i));
> +
> + /* Ensure DATA PID is correctly configured */
> + if (dr->diepctl[i] & DXEPCTL_DPID)
> + dr->diepctl[i] |= DXEPCTL_SETD1PID;
> + else
> + dr->diepctl[i] |= DXEPCTL_SETD0PID;
> +
> + dr->dieptsiz[i] = readl(hsotg->regs + DIEPTSIZ(i));
> + dr->diepdma[i] = readl(hsotg->regs + DIEPDMA(
Re: [PATCH 02/15] phy-sun4i-usb: Add a helper function to update the iscr register
Hi,
On 09-03-15 22:47, Arnd Bergmann wrote:
On Monday 09 March 2015 21:40:15 Hans de Goede wrote:
+void sun4i_usb_phy_update_iscr(struct phy *_phy, u32 clr, u32 set)
+{
+ struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+ struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+ u32 iscr;
+
+ iscr = readl(data->base + REG_ISCR);
+ iscr &= ~clr;
+ iscr |= set;
+ writel(iscr, data->base + REG_ISCR);
+}
+EXPORT_SYMBOL(sun4i_usb_phy_update_iscr);
+
I would generally consider this a bad design. What is the purpose of
calling sun4i_usb_phy_update_iscr()
There are 2 different use cases for this one is to enable the dataline
pull-ups at driver init and disable them at driver exit, this could /
should probably be moved to the phy_init / phy_exit code for the usb0 phy
removing the need to do this from within the sunxi musb glue.
The second use-case is more tricky, for some reasons Allwinner has decided
to not use the dedicated id-detect and vusb-sense pins of the phy they are
using (these pins are not routed to the outside).
Instead id-detect and vusb-sense are done through any $random gpio pins
(including non irq capable pins on some designs requiring polling).
But the musb-core still needs to know the status of the id and vbus pins,
and gets this from the usb0-phy iscr register, which reflects the status of
the not connected dedicated pins of the phy. The reason this can still
work at all is because the iscr register allows the user to override
whatever the not connected phy pins are seeing and forcing a value to
report to the musb core as id and vbus status.
This is done by these 2 functions in the musb sunxi glue:
static void sunxi_musb_force_id(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_ID_HIGH;
else
val = SUNXI_ISCR_FORCE_ID_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_ID_MASK, val);
}
static void sunxi_musb_force_vbus(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_VBUS_HIGH;
else
val = SUNXI_ISCR_FORCE_VBUS_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_VBUS_MASK, val);
}
I will happily admit that these 2 functions are a better API between the sunxi
musb
glue and the sunxi usb phy driver. I started with the minimal
sun4i_usb_phy_update_iscr
approach as I wanted to keep the API as small as possible, but having 2
functions like
the one above, which actually reflect what is happening would indeed be better.
Note that the polling of the pins cannot (easily) be moved into the phy driver
for various
reasons:
1) It depends on dr_mode, the otg may be used in host only mode in which case
there are no
pins at all.
2) the musb set_vbus callback needs access to the pins
3) When id changes some musb core state changes are necessary.
I'll respin the patch set to do things this way as soon as we've agreement on
your second point.
> and why can't there be a high-level
PHY API for this?
The current generic phy API seems to not have any bus specific methods, I know
that
in the long run people want to get rid of struct usb_phy, so maybe we should
consider
adding bus specific methods to the generic phy API for things like otg.
If we decide to add bus specific methods, then the question becomes if having
int phy_usb_set_id_detect(struct phy *phy, bool val);
int phy_usb_set_vbus_detect(struct phy *phy, bool val);
Functions in the generic phy API is a good idea, or if this is too sunxi
specific,
I'm fine with doing this either way. If we want to go the generic phy route
I'll split this in 2 patches, one adding these 2 generic functions & phy-ops,
and
1 doing the sunxi implementation.
If people believe this is too sunxi specific (I believe it is, but as said I'm
fine with doing this either way). I'll respin this patch to remove the too
generic sun4i_usb_phy_update_iscr function, and instead add these 2:
void sun4i_usb_phy_set_id_detect(struct phy *phy, bool val);
void sun4i_usb_phy_set_vbus_detect(struct phy *phy, bool val);
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/15] musb: Add support for the Allwinner sunxi musb controller
On Tuesday 10 March 2015 09:46:24 Chen-Yu Tsai wrote: > I believe you are talking about "mmio-sram"? Yes. > The syscon here represents a switch, to toggle whether a block of SRAM is > mapped into the CPU memory space, or to a specific devices private address > space. It is not the actual SRAM. > > The SRAM DT binding is orthogonal to this, if not irrelevant when the block > is mapped privately, as it is no longer visible from the DT's PoV. Ok, got it. I missed this part when reading the introduction. syscon seems fine to me then. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] musb: Do not use musb_read[b|w] / _write[b|w] wrappers in generic fifo functions
On Tuesday 10 March 2015 08:43:22 Hans de Goede wrote: > On 09-03-15 22:50, Arnd Bergmann wrote: > > On Monday 09 March 2015 21:40:18 Hans de Goede wrote: > >> The generic fifo functions already use non wrapped accesses in various > >> cases through the iowrite#_rep functions, and all platforms which override > >> the default musb_read[b|w] / _write[b|w] functions also provide their own > >> fifo access functions, so we can safely drop the unnecessary indirection > >> from the fifo access functions. > >> > >> Signed-off-by: Hans de Goede > >> > > > > The patch looks reasonably, but the description seem misleading. > > I believe the real reason why it's ok to use __raw_writew for the > > FIFO is that a FIFO by definition is using CPU endian access for > > copying byte streams from memory, which is unlike any other MMIO > > register that requires fixed-endian accessors. > > I'm not sure that that is the case here, this fifo allows reading > 4 bytes at a time using 32 bit word access, so endianness may come > into play. This patch is safe however since all existing users of > the generic fifo_read / write helpers which this patch touches, are > also using the generic musb_read[b|w] / _write[b|w] functions which > are just __raw_foo wrappers, so there is no functional change, which > is what the commit message tries to say. This probably means that the generic musb helpers are not safe to use on big-endian ARM systems (but may work on MIPS). The only one currently not using the generic helpers is blackfin, which is fixed to little endian. > Note that sunxi needs this function because of the register address > translation the sunxi_musb_readb / writeb wrappers are doing which > does not know how to deal with fifo data, and besides that it should > make the generic read / write fifo helpers somewhat faster by removing > an indirect function call. Your sunxi_musb_readw/writew functions however also are endian-safe and seem to get this part right, unlike all other platforms that use the generic __raw_*() accessors. With this patch applied, it should actually work fine, and it would work on other platforms as well if we change all __raw_*() calls outside of musb_default_write_fifo() and musb_default_read_fifo() to use *_relaxed() instead. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] musb: Do not use musb_read[b|w] / _write[b|w] wrappers in generic fifo functions
Hi, On 10-03-15 09:50, Arnd Bergmann wrote: On Tuesday 10 March 2015 08:43:22 Hans de Goede wrote: On 09-03-15 22:50, Arnd Bergmann wrote: On Monday 09 March 2015 21:40:18 Hans de Goede wrote: The generic fifo functions already use non wrapped accesses in various cases through the iowrite#_rep functions, and all platforms which override the default musb_read[b|w] / _write[b|w] functions also provide their own fifo access functions, so we can safely drop the unnecessary indirection from the fifo access functions. Signed-off-by: Hans de Goede The patch looks reasonably, but the description seem misleading. I believe the real reason why it's ok to use __raw_writew for the FIFO is that a FIFO by definition is using CPU endian access for copying byte streams from memory, which is unlike any other MMIO register that requires fixed-endian accessors. I'm not sure that that is the case here, this fifo allows reading 4 bytes at a time using 32 bit word access, so endianness may come into play. This patch is safe however since all existing users of the generic fifo_read / write helpers which this patch touches, are also using the generic musb_read[b|w] / _write[b|w] functions which are just __raw_foo wrappers, so there is no functional change, which is what the commit message tries to say. This probably means that the generic musb helpers are not safe to use on big-endian ARM systems (but may work on MIPS). The only one currently not using the generic helpers is blackfin, which is fixed to little endian. Note that sunxi needs this function because of the register address translation the sunxi_musb_readb / writeb wrappers are doing which does not know how to deal with fifo data, and besides that it should make the generic read / write fifo helpers somewhat faster by removing an indirect function call. Your sunxi_musb_readw/writew functions however also are endian-safe and seem to get this part right, unlike all other platforms that use the generic __raw_*() accessors. With this patch applied, it should actually work fine, and it would work on other platforms as well if we change all __raw_*() calls outside of musb_default_write_fifo() and musb_default_read_fifo() to use *_relaxed() instead. I think that that change falls outside of the scope of this patchset. I agree that it would be probably a good idea to get rid of the __raw_foo usage in musb, which is why I've used the non __raw versions in the sunxi glue, but as said I believe this falls outside of the scope of this patchset. All my preparation patches for adding sunxi support carefully do not make any functional changes, as I do not want to cause regressions on hardware which I cannot test. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/15] phy-sun4i-usb: Add a helper function to update the iscr register
On Tuesday 10 March 2015 09:04:43 Hans de Goede wrote:
> Hi,
>
> On 09-03-15 22:47, Arnd Bergmann wrote:
> > On Monday 09 March 2015 21:40:15 Hans de Goede wrote:
> >> +void sun4i_usb_phy_update_iscr(struct phy *_phy, u32 clr, u32 set)
> >> +{
> >> + struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
> >> + struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
> >> + u32 iscr;
> >> +
> >> + iscr = readl(data->base + REG_ISCR);
> >> + iscr &= ~clr;
> >> + iscr |= set;
> >> + writel(iscr, data->base + REG_ISCR);
> >> +}
> >> +EXPORT_SYMBOL(sun4i_usb_phy_update_iscr);
> >> +
> >>
> >
> > I would generally consider this a bad design. What is the purpose of
> > calling sun4i_usb_phy_update_iscr()
>
> There are 2 different use cases for this one is to enable the dataline
> pull-ups at driver init and disable them at driver exit, this could /
> should probably be moved to the phy_init / phy_exit code for the usb0 phy
> removing the need to do this from within the sunxi musb glue.
>
> The second use-case is more tricky, for some reasons Allwinner has decided
> to not use the dedicated id-detect and vusb-sense pins of the phy they are
> using (these pins are not routed to the outside).
>
> Instead id-detect and vusb-sense are done through any $random gpio pins
> (including non irq capable pins on some designs requiring polling).
>
> But the musb-core still needs to know the status of the id and vbus pins,
> and gets this from the usb0-phy iscr register, which reflects the status of
> the not connected dedicated pins of the phy. The reason this can still
> work at all is because the iscr register allows the user to override
> whatever the not connected phy pins are seeing and forcing a value to
> report to the musb core as id and vbus status.
>
> This is done by these 2 functions in the musb sunxi glue:
>
> static void sunxi_musb_force_id(struct musb *musb, u32 val)
> {
> struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
>
> if (val)
> val = SUNXI_ISCR_FORCE_ID_HIGH;
> else
> val = SUNXI_ISCR_FORCE_ID_LOW;
>
> sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_ID_MASK, val);
> }
>
> static void sunxi_musb_force_vbus(struct musb *musb, u32 val)
> {
> struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
>
> if (val)
> val = SUNXI_ISCR_FORCE_VBUS_HIGH;
> else
> val = SUNXI_ISCR_FORCE_VBUS_LOW;
>
> sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_VBUS_MASK,
> val);
> }
>
> I will happily admit that these 2 functions are a better API between the
> sunxi musb
> glue and the sunxi usb phy driver. I started with the minimal
> sun4i_usb_phy_update_iscr
> approach as I wanted to keep the API as small as possible, but having 2
> functions like
> the one above, which actually reflect what is happening would indeed be
> better.
Ok, that would definitely improve things.
> Note that the polling of the pins cannot (easily) be moved into the phy
> driver for various
> reasons:
>
> 1) It depends on dr_mode, the otg may be used in host only mode in which case
> there are no
> pins at all.
> 2) the musb set_vbus callback needs access to the pins
> 3) When id changes some musb core state changes are necessary.
>
> I'll respin the patch set to do things this way as soon as we've agreement on
> your second point.
>
> > and why can't there be a high-level
> > PHY API for this?
>
> The current generic phy API seems to not have any bus specific methods, I
> know that
> in the long run people want to get rid of struct usb_phy, so maybe we should
> consider
> adding bus specific methods to the generic phy API for things like otg.
>
> If we decide to add bus specific methods, then the question becomes if having
>
> int phy_usb_set_id_detect(struct phy *phy, bool val);
> int phy_usb_set_vbus_detect(struct phy *phy, bool val);
>
> Functions in the generic phy API is a good idea, or if this is too sunxi
> specific,
> I'm fine with doing this either way. If we want to go the generic phy route
> I'll split this in 2 patches, one adding these 2 generic functions & phy-ops,
> and
> 1 doing the sunxi implementation.
>
> If people believe this is too sunxi specific (I believe it is, but as said I'm
> fine with doing this either way). I'll respin this patch to remove the too
> generic sun4i_usb_phy_update_iscr function, and instead add these 2:
>
> void sun4i_usb_phy_set_id_detect(struct phy *phy, bool val);
> void sun4i_usb_phy_set_vbus_detect(struct phy *phy, bool val);
Thanks for your detailed explanations. I think this is something for
Kishon to decide, based on where he wants to take the phy API in the
long run. I'm fine with it either way, and it seems easy enough to
change between those two interfaces if we make up our minds about it
later.
Arnd
--
To unsubscribe from this
Re: [linux-sunxi] [PATCH 09/15] ARM: dts: sun4i: Add USB Dual Role Controller
Hi, On 10-03-15 00:31, Julian Calaby wrote: Hi Hans, On Tue, Mar 10, 2015 at 7:40 AM, Hans de Goede wrote: Add a node for the otg/drc usb controller to sun4i-a10.dtsi. Signed-off-by: Hans de Goede --- arch/arm/boot/dts/sun4i-a10.dtsi | 12 drivers/usb/musb/Kconfig | 1 + 2 files changed, 13 insertions(+) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 8180a3a..fab8e9a 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -65,6 +65,7 @@ comment "Platform Glue Layer" config USB_MUSB_SUNXI tristate "Allwinner (sunxi)" depends on ARCH_SUNXI + depends on NOP_USB_XCEIV Shouldn't this be in a different patch? Yes, this was a later fixup commit which got squashed into the wrong patch, thanks for catching this. This has been fixed in my personal tree, and will be in v2 of this set: https://github.com/jwrdegoede/linux-sunxi/commits/sunxi-wip Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
If I plug in my USB DVB-T stick I get the following in dmesg: dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state. dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer. DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver) usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver)... input: IR-receiver inside an USB DVB receiver as /devices/pci:00/:00:14.0/usb1/1-1/input/input17 dvb-usb: schedule remote query interval to 50 msecs. xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 1 comp_code 1 xhci_hcd :00:14.0: Looking for event-dma 000207540400 trb-start 000207540420 trb-end 000207540420 seg-start 0002075404 xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 1 comp_code 1 xhci_hcd :00:14.0: Looking for event-dma 000207540410 trb-start 000207540420 trb-end 000207540420 seg-start 0002075404 dvb-usb: bulk message failed: -110 (2/0) and DVB-T is not functional. The problem came in with: 1163d50 Merge tag 'usb-4.0-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb I never had this xhci_hcd error before so this is a regression. Thanks, Jörg -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8][RESEND]usb:fsl:otg: Remove host drv upon otg bring-up
Change have_hcd variable to remove/suspend host driver on
completion of otg initialization for otg auto detect
Signed-off-by: Ramneek Mehresh
Reviewed-by: Li Yang-R58472
Reviewed-by: Fleming Andrew-AFLEMING
Tested-by: Fleming Andrew-AFLEMING
---
drivers/usb/host/ehci-fsl.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 8f329a0..95c2feb 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -177,6 +177,7 @@ static int usb_hcd_fsl_probe(const struct hc_driver *driver,
#if defined(CONFIG_FSL_USB2_OTG) || defined(CONFIG_FSL_USB2_OTG_MODULE)
if (pdata->operating_mode == FSL_USB2_DR_OTG) {
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB2);
@@ -197,6 +198,11 @@ static int usb_hcd_fsl_probe(const struct hc_driver
*driver,
retval = -ENODEV;
goto err2;
}
+
+ ehci_fsl->have_hcd = 1;
+ } else {
+ dev_err(&pdev->dev, "wrong operating mode\n");
+ return -ENODEV;
}
#endif
return retval;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Control message failures kill entire XHCI stack
Hi Devin, Do you mind to post output of "lspci -xv" on the machine where you saw this problem? We are facing the same problems in other cases. I could provide a patch for it later. Thanks, Baolu On 01/19/2015 04:55 AM, Devin Heitmueller wrote: Hello, I'm debugging some issues on a couple of different USB TV tuners which boil down to the following error: xhci_hcd :00:14.0: xHCI host not responding to stop endpoint command. This is followed by the XHCI driver disconnecting *all* USB devices from the controller. I've done a bit of debugging, and the root of the issue appears to be an intermittent control message timing out, and then the call to usb_kill_urb() that occurs inside of usb_control_msg() when the timeout expires is what causes the disconnect. Specifically, it would appear that xhci_urb_dequeue tries to stop the endpoint using xhci_queue_stop_endpoint(), the command gets queued but the IRQ never fires to perform the TRB_STOP_RING completion code. The function xhci_stop_endpoint_command_watchdog() fires after five seconds, which tears down the entire driver. Below is the dmesg output with the xhci_hcd debugging enabled. The dump_stack() call is something I added (i.e. it's not an OOPS) so I could see which code path was making the usb_kill_urb() call that was failing. Note that the caller is using usb_control_msg() with 1000ms timeout, and we can see from the timestamps that the timer expires which is what causes the call to usb_kill_urb(). I would imagine that explicitly killing URBs is a pretty uncommon task for control endpoint messages (compared to ISOC or BULK endpoints where it's done regularly). Is it possible a exception case has been missed? Independent of the usb_kill_urb() killing the entire stack, I still don't really understand yet why the control message failed in the first place. This is a well-exercised code path in the au0828 driver (related to I2C transfers) and I've never seen this when using the EHCI driver. My assumption is that either the HCD is getting sick which is causing both the control message to fail as well as putting it into an inconsistent state such that we never get the TRB_STOP_RING IRQ, or we've got two separate bugs - the control message failing for some "legitimate" reason (i.e. I screwed something up in my au0828 driver), followed by the usb_kill_urb() error simply not handling killing of URBs on a control endpoint (which causes the entire stack to go down). Thoughts/suggestions/recommendations are welcome. Thanks in advance, Devin Jan 18 14:04:05 devin-MacBookPro kernel: [ 9119.647249] au0828: au0828_writereg(0x0100, 0x00) Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.645091] xhci_hcd :00:14.0: Cancel URB 8802543c36c0, dev 1, ep 0x0, starting at offset 0x25c358940 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.645365] djh dequeue pending=0 ep_index=0 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.645632] CPU: 1 PID: 2782 Comm: tvtime Tainted: P OE 3.18.0-rc4djh+ #33 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.645921] Hardware name: Apple Inc. MacBookPro11,1/Mac-189A3D4F975D5FFC, BIOS MBP111.88Z.0138.B11.1408291433 08/29/2014 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.646236] 88025b9b 88023ea2f9d8 817445c1 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.646570] 8802543c36c0 88023ea2fa58 a0080b2e 00025c358940 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.646909] 88023ea2fa48 3ea2fa18 88023e8a22a0 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.647256] Call Trace: Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.647605] [] dump_stack+0x46/0x58 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.647981] [] xhci_urb_dequeue+0x28e/0x420 [xhci_hcd] Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.648357] [] unlink1+0x2d/0x130 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.648743] [] ? internal_add_timer+0xb0/0xb0 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.649133] [] ? get_device+0x17/0x30 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.649526] [] usb_hcd_unlink_urb+0x5d/0xf0 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.649928] [] usb_kill_urb+0x3a/0xa0 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.650334] [] ? wake_up_state+0x20/0x20 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.650746] [] usb_start_wait_urb+0xc8/0x150 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.651166] [] ? __kmalloc+0x55/0x190 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.651586] [] usb_control_msg+0xc5/0x110 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.652011] [] au0828_writereg+0x79/0xf0 [au0828] Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.652447] [] ? try_to_del_timer_sync+0x4f/0x70 Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.652894] [] au0828_analog_stream_disable+0x29/0x50 [au0828] Jan 18 14:04:06 devin-MacBookPro kernel: [ 9120.653354] [] vidioc_streamoff+0xd9/0x1c0 [au0828] Jan 18 14:04:06 devin-MacBookPro kernel: [
[PATCH 2/8][RESEND]usb:fsl:otg: Add support to add/remove usb host driver
Add workqueue to add/remove host driver (outside interrupt context)
upon each id change
Signed-off-by: Li Yang
Signed-off-by: Ramneek Mehresh
Reviewed-by: Fleming Andrew-AFLEMING
Tested-by: Fleming Andrew-AFLEMING
---
drivers/usb/host/ehci-fsl.c | 102 ++
drivers/usb/host/ehci.h | 4 +-
drivers/usb/phy/phy-fsl-usb.c | 7 ++-
include/linux/usb.h | 1 +
4 files changed, 93 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index ab4eee3..8f329a0 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -33,6 +33,54 @@
#include "ehci-fsl.h"
+struct ehci_fsl {
+ struct ehci_hcd ehci;
+
+#ifdef CONFIG_PM
+ /* Saved USB PHY settings, need to restore after deep sleep. */
+ u32 usb_ctrl;
+#endif
+
+ /* store current hcd state for otg;
+* have_hcd is true when host drv al already part of otg framework,
+* otherwise false;
+* hcd_add is true when otg framework wants to add host
+* drv as part of otg;flase when it wants to remove it
+*/
+ unsigned have_hcd:1;
+ unsigned hcd_add:1;
+};
+
+#if defined(CONFIG_FSL_USB2_OTG) || defined(CONFIG_FSL_USB2_OTG_MODULE)
+static struct ehci_fsl *hcd_to_ehci_fsl(struct usb_hcd *hcd)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+
+ return container_of(ehci, struct ehci_fsl, ehci);
+}
+
+static void do_change_hcd(struct work_struct *work)
+{
+ struct ehci_hcd *ehci = container_of(work, struct ehci_hcd,
+ change_hcd_work);
+ struct usb_hcd *hcd = ehci_to_hcd(ehci);
+ struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
+ void __iomem *non_ehci = hcd->regs;
+ int retval;
+
+ if (ehci_fsl->hcd_add && !ehci_fsl->have_hcd) {
+ writel(USBMODE_CM_HOST, non_ehci + FSL_SOC_USB_USBMODE);
+ /* host, gadget and otg share same int line */
+ retval = usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
+ if (retval == 0)
+ ehci_fsl->have_hcd = 1;
+ } else if (!ehci_fsl->hcd_add && ehci_fsl->have_hcd) {
+ usb_remove_hcd(hcd);
+ ehci_fsl->have_hcd = 0;
+ }
+}
+#endif
+
/* configure so an HC device and id are always provided */
/* always called with process context; sleeping is OK */
@@ -126,11 +174,14 @@ static int usb_hcd_fsl_probe(const struct hc_driver
*driver,
goto err2;
device_wakeup_enable(hcd->self.controller);
-#ifdef CONFIG_USB_OTG
+#if defined(CONFIG_FSL_USB2_OTG) || defined(CONFIG_FSL_USB2_OTG_MODULE)
if (pdata->operating_mode == FSL_USB2_DR_OTG) {
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB2);
+
+ INIT_WORK(&ehci->change_hcd_work, do_change_hcd);
+
dev_dbg(&pdev->dev, "hcd=0x%p ehci=0x%p, phy=0x%p\n",
hcd, ehci, hcd->usb_phy);
@@ -376,15 +427,6 @@ static int ehci_fsl_setup(struct usb_hcd *hcd)
return retval;
}
-struct ehci_fsl {
- struct ehci_hcd ehci;
-
-#ifdef CONFIG_PM
- /* Saved USB PHY settings, need to restore after deep sleep. */
- u32 usb_ctrl;
-#endif
-};
-
#ifdef CONFIG_PM
#ifdef CONFIG_PPC_MPC512x
@@ -532,24 +574,32 @@ static inline int ehci_fsl_mpc512x_drv_resume(struct
device *dev)
}
#endif /* CONFIG_PPC_MPC512x */
-static struct ehci_fsl *hcd_to_ehci_fsl(struct usb_hcd *hcd)
-{
- struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-
- return container_of(ehci, struct ehci_fsl, ehci);
-}
-
static int ehci_fsl_drv_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
- struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
void __iomem *non_ehci = hcd->regs;
+#if defined(CONFIG_FSL_USB2_OTG) || defined(CONFIG_FSL_USB2_OTG_MODULE)
+ struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
+ struct usb_bus host = hcd->self;
+#endif
if (of_device_is_compatible(dev->parent->of_node,
"fsl,mpc5121-usb2-dr")) {
return ehci_fsl_mpc512x_drv_suspend(dev);
}
+#if defined(CONFIG_FSL_USB2_OTG) || defined(CONFIG_FSL_USB2_OTG_MODULE)
+ if (host.is_otg) {
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+
+ /* remove hcd */
+ ehci_fsl->hcd_add = 0;
+ schedule_work(&ehci->change_hcd_work);
+ host.is_otg = 0;
+ return 0;
+ }
+#endif
+
ehci_prepare_ports_for_controller_suspend(hcd_to_ehci(hcd),
device_may_wakeup(dev));
if (!fsl_deep_sleep())
@@ -562,15 +612,29 @@ static int ehci_fsl_drv_suspend(struct device *dev)
static int ehci_fsl_drv_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
- struct ehc
[PATCH 4/8][RESEND]usb:fsl:otg: Combine host/gadget start/resume for ID change
Make call to fsl_otg_event for each id change even
Signed-off-by: Ramneek Mehresh
Reviewed-by: Fleming Andrew-AFLEMING
Tested-by: Fleming Andrew-AFLEMING
---
drivers/usb/phy/phy-fsl-usb.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 2eb54ef..ce40d5c 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -733,6 +733,7 @@ irqreturn_t fsl_otg_isr(int irq, void *dev_id)
{
struct otg_fsm *fsm = &((struct fsl_otg *)dev_id)->fsm;
struct usb_otg *otg = ((struct fsl_otg *)dev_id)->phy.otg;
+ struct fsl_otg *otg_dev = dev_id;
u32 otg_int_src, otg_sc;
otg_sc = fsl_readl(&usb_dr_regs->otgsc);
@@ -762,18 +763,8 @@ irqreturn_t fsl_otg_isr(int irq, void *dev_id)
otg->gadget->is_a_peripheral = !fsm->id;
VDBG("ID int (ID is %d)\n", fsm->id);
- if (fsm->id) { /* switch to gadget */
- schedule_delayed_work(
- &((struct fsl_otg *)dev_id)->otg_event,
- 100);
- } else {/* switch to host */
- cancel_delayed_work(&
- ((struct fsl_otg *)dev_id)->
- otg_event);
- fsl_otg_start_gadget(fsm, 0);
- otg_drv_vbus(fsm, 1);
- fsl_otg_start_host(fsm, 1);
- }
+ schedule_delayed_work(&otg_dev->otg_event, 100);
+
return IRQ_HANDLED;
}
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8][RESEND]usb:fsl:otg: Modify otg_event to start host drv
Add mechanism to start host driver from inside fsl_otg_even
upon each id change interrupt
Signed-off-by: Ramneek Mehresh
Reviewed-by: Fleming Andrew-AFLEMING
Tested-by: Fleming Andrew-AFLEMING
---
drivers/usb/phy/phy-fsl-usb.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 26168da..2eb54ef 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -677,6 +677,10 @@ static void fsl_otg_event(struct work_struct *work)
fsl_otg_start_host(fsm, 0);
otg_drv_vbus(fsm, 0);
fsl_otg_start_gadget(fsm, 1);
+ } else {
+ fsl_otg_start_gadget(fsm, 0);
+ otg_drv_vbus(fsm, 1);
+ fsl_otg_start_host(fsm, 1);
}
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8][RESEND]usb:fsl:otg: Add controller version based ULPI and UTMI phy
Add controller version based ULPI and UTMI phy initialization for
otg driver
Signed-off-by: Shengzhou Liu
Signed-off-by: Ramneek Mehresh
Reviewed-by: Fleming Andrew-AFLEMING
Tested-by: Fleming Andrew-AFLEMING
---
drivers/usb/phy/phy-fsl-usb.c | 20
drivers/usb/phy/phy-fsl-usb.h | 7 +++
2 files changed, 27 insertions(+)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..f90093a 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -923,12 +923,32 @@ int usb_otg_start(struct platform_device *pdev)
temp &= ~(PORTSC_PHY_TYPE_SEL | PORTSC_PTW);
switch (pdata->phy_mode) {
case FSL_USB2_PHY_ULPI:
+ if (pdata->controller_ver) {
+ /* controller version 1.6 or above */
+ setbits32(&p_otg->dr_mem_map->control,
+ USB_CTRL_ULPI_PHY_CLK_SEL);
+ /*
+* Due to controller issue of PHY_CLK_VALID in ULPI
+* mode, we set USB_CTRL_USB_EN before checking
+* PHY_CLK_VALID, otherwise PHY_CLK_VALID doesn't work.
+*/
+ clrsetbits_be32(&p_otg->dr_mem_map->control,
+USB_CTRL_UTMI_PHY_EN, USB_CTRL_IOENB);
+ }
temp |= PORTSC_PTS_ULPI;
break;
case FSL_USB2_PHY_UTMI_WIDE:
temp |= PORTSC_PTW_16BIT;
/* fall through */
case FSL_USB2_PHY_UTMI:
+ if (pdata->controller_ver) {
+ /* controller version 1.6 or above */
+ setbits32(&p_otg->dr_mem_map->control,
+USB_CTRL_UTMI_PHY_EN);
+ /* Delay for UTMI PHY CLK to become stable - 10ms */
+ mdelay(FSL_UTMI_PHY_DLY);
+ }
+ setbits32(&p_otg->dr_mem_map->control, USB_CTRL_UTMI_PHY_EN);
temp |= PORTSC_PTS_UTMI;
/* fall through */
default:
diff --git a/drivers/usb/phy/phy-fsl-usb.h b/drivers/usb/phy/phy-fsl-usb.h
index 2314995..4a78fb3 100644
--- a/drivers/usb/phy/phy-fsl-usb.h
+++ b/drivers/usb/phy/phy-fsl-usb.h
@@ -199,6 +199,13 @@
/* control Register Bit Masks */
#define USB_CTRL_IOENB(0x1<<2)
#define USB_CTRL_ULPI_INT0EN (0x1<<0)
+#define USB_CTRL_WU_INT_EN(0x1<<1)
+#define USB_CTRL_LINE_STATE_FILTER__EN(0x1<<3)
+#define USB_CTRL_KEEP_OTG_ON (0x1<<4)
+#define USB_CTRL_OTG_PORT (0x1<<5)
+#define USB_CTRL_PLL_RESET(0x1<<8)
+#define USB_CTRL_UTMI_PHY_EN (0x1<<9)
+#define USB_CTRL_ULPI_PHY_CLK_SEL (0x1<<10)
/* BCSR5 */
#define BCSR5_INT_USB (0x02)
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8][RESEND]usb:fsl:otg: Add host-gadget drv sync delay
Resolve synchronization issue between host and gadget drivers
upon role-reversal
Signed-off-by: Ramneek Mehresh
Reviewed-by: Li Yang-R58472
Reviewed-by: Fleming Andrew-AFLEMING
Tested-by: Fleming Andrew-AFLEMING
---
drivers/usb/phy/phy-fsl-usb.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index ce40d5c..a3a578d 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -544,8 +544,17 @@ int fsl_otg_start_gadget(struct otg_fsm *fsm, int on)
dev = otg->gadget->dev.parent;
if (on) {
- if (dev->driver->resume)
+ /* Delay gadget resume to synchronize between host and gadget
+* drivers. Upon role-reversal host drv is shutdown by kernel
+* worker thread. By the time host drv shuts down, controller
+* gets programmed for gadget role. Shutting host drv after
+* this results in controller getting reset, and it stops
+* responding to otg events
+*/
+ if (dev->driver->resume) {
+ msleep(1000);
dev->driver->resume(dev);
+ }
} else {
if (dev->driver->suspend)
dev->driver->suspend(dev, otg_suspend_state);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8][RESEND]usb:fsl:otg: Resolve OTG crash issue with another host
Resolves kernel crash issue when a USB flash drive is inserted
into USB1 port with USB2 port configured as otg. Removing
"else" block so that the controller coming up in "non-otg" mode
doesn't return -ENODEV. Returning "ENODEV" results in platform
framework unbinding platform-drv from controller resulting in
kernel crash later in hub driver
Signed-off-by: Ramneek Mehresh
---
drivers/usb/host/ehci-fsl.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 95c2feb..0d5677c 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -200,9 +200,6 @@ static int usb_hcd_fsl_probe(const struct hc_driver *driver,
}
ehci_fsl->have_hcd = 1;
- } else {
- dev_err(&pdev->dev, "wrong operating mode\n");
- return -ENODEV;
}
#endif
return retval;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/15] phy-sun4i-usb: Add a helper function to update the iscr register
Hi,
On 10-03-15 09:57, Arnd Bergmann wrote:
On Tuesday 10 March 2015 09:04:43 Hans de Goede wrote:
Hi,
On 09-03-15 22:47, Arnd Bergmann wrote:
On Monday 09 March 2015 21:40:15 Hans de Goede wrote:
+void sun4i_usb_phy_update_iscr(struct phy *_phy, u32 clr, u32 set)
+{
+ struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+ struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+ u32 iscr;
+
+ iscr = readl(data->base + REG_ISCR);
+ iscr &= ~clr;
+ iscr |= set;
+ writel(iscr, data->base + REG_ISCR);
+}
+EXPORT_SYMBOL(sun4i_usb_phy_update_iscr);
+
I would generally consider this a bad design. What is the purpose of
calling sun4i_usb_phy_update_iscr()
There are 2 different use cases for this one is to enable the dataline
pull-ups at driver init and disable them at driver exit, this could /
should probably be moved to the phy_init / phy_exit code for the usb0 phy
removing the need to do this from within the sunxi musb glue.
The second use-case is more tricky, for some reasons Allwinner has decided
to not use the dedicated id-detect and vusb-sense pins of the phy they are
using (these pins are not routed to the outside).
Instead id-detect and vusb-sense are done through any $random gpio pins
(including non irq capable pins on some designs requiring polling).
But the musb-core still needs to know the status of the id and vbus pins,
and gets this from the usb0-phy iscr register, which reflects the status of
the not connected dedicated pins of the phy. The reason this can still
work at all is because the iscr register allows the user to override
whatever the not connected phy pins are seeing and forcing a value to
report to the musb core as id and vbus status.
This is done by these 2 functions in the musb sunxi glue:
static void sunxi_musb_force_id(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_ID_HIGH;
else
val = SUNXI_ISCR_FORCE_ID_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_ID_MASK, val);
}
static void sunxi_musb_force_vbus(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_VBUS_HIGH;
else
val = SUNXI_ISCR_FORCE_VBUS_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_VBUS_MASK, val);
}
I will happily admit that these 2 functions are a better API between the sunxi
musb
glue and the sunxi usb phy driver. I started with the minimal
sun4i_usb_phy_update_iscr
approach as I wanted to keep the API as small as possible, but having 2
functions like
the one above, which actually reflect what is happening would indeed be better.
Ok, that would definitely improve things.
Note that the polling of the pins cannot (easily) be moved into the phy driver
for various
reasons:
1) It depends on dr_mode, the otg may be used in host only mode in which case
there are no
pins at all.
2) the musb set_vbus callback needs access to the pins
3) When id changes some musb core state changes are necessary.
I'll respin the patch set to do things this way as soon as we've agreement on
your second point.
> and why can't there be a high-level
PHY API for this?
The current generic phy API seems to not have any bus specific methods, I know
that
in the long run people want to get rid of struct usb_phy, so maybe we should
consider
adding bus specific methods to the generic phy API for things like otg.
If we decide to add bus specific methods, then the question becomes if having
int phy_usb_set_id_detect(struct phy *phy, bool val);
int phy_usb_set_vbus_detect(struct phy *phy, bool val);
Functions in the generic phy API is a good idea, or if this is too sunxi
specific,
I'm fine with doing this either way. If we want to go the generic phy route
I'll split this in 2 patches, one adding these 2 generic functions & phy-ops,
and
1 doing the sunxi implementation.
If people believe this is too sunxi specific (I believe it is, but as said I'm
fine with doing this either way). I'll respin this patch to remove the too
generic sun4i_usb_phy_update_iscr function, and instead add these 2:
void sun4i_usb_phy_set_id_detect(struct phy *phy, bool val);
void sun4i_usb_phy_set_vbus_detect(struct phy *phy, bool val);
Thanks for your detailed explanations. I think this is something for
Kishon to decide, based on where he wants to take the phy API in the
long run. I'm fine with it either way, and it seems easy enough to
change between those two interfaces if we make up our minds about it
later.
Ok, so I've fixed things in my personal tree to use 2 sun4i private functions
for
now:
void sun4i_usb_phy_set_id_detect(struct phy *phy, bool val);
void sun4i_usb_phy_set_vbus_detect(struct phy *phy, bool val);
You can find this change here:
https://g
[PATCH 8/8] usb:fsl:otg: Make fsl otg driver as tristate
Provide option to load fsl otg driver as loadable module Signed-off-by: Ramneek Mehresh --- drivers/usb/phy/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 52d3d58..a1637c0 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -19,7 +19,7 @@ config AB8500_USB in host mode, low speed. config FSL_USB2_OTG - bool "Freescale USB OTG Transceiver Driver" + tristate "Freescale USB OTG Transceiver Driver" depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM select USB_OTG select USB_PHY -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cp210x GPIO support
Hi Preston, Are you still maintaining GPIO support for the cp210x driver? I'm looking at the last set of patches from 2012 that attempted to mainline it, but it looks like it didn't get anywhere. I'd like to try again. Have you looked at CONFIG_GPIO support in the kernel? I think the biggest complaint the last time had been the creation of a new ioctl method. Using the GPIO library should probably get around that. The GPIO userspace abi isn't fantastic, but it is already accepted, so there shouldn't be any complaints there. It also makes it possible to use the GPIO lines. g. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/15] phy-sun4i-usb: Add a helper function to update the iscr register
Hi,
On Tuesday 10 March 2015 03:43 PM, Hans de Goede wrote:
Hi,
On 10-03-15 09:57, Arnd Bergmann wrote:
On Tuesday 10 March 2015 09:04:43 Hans de Goede wrote:
Hi,
On 09-03-15 22:47, Arnd Bergmann wrote:
On Monday 09 March 2015 21:40:15 Hans de Goede wrote:
+void sun4i_usb_phy_update_iscr(struct phy *_phy, u32 clr, u32 set)
+{
+ struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+ struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+ u32 iscr;
+
+ iscr = readl(data->base + REG_ISCR);
+ iscr &= ~clr;
+ iscr |= set;
+ writel(iscr, data->base + REG_ISCR);
+}
+EXPORT_SYMBOL(sun4i_usb_phy_update_iscr);
+
I would generally consider this a bad design. What is the purpose of
calling sun4i_usb_phy_update_iscr()
right. That would bind the PHY driver and the controller driver and would
start creating problems when a different PHY is connected with the
controller.
There are 2 different use cases for this one is to enable the dataline
pull-ups at driver init and disable them at driver exit, this could /
should probably be moved to the phy_init / phy_exit code for the usb0 phy
removing the need to do this from within the sunxi musb glue.
The second use-case is more tricky, for some reasons Allwinner has decided
to not use the dedicated id-detect and vusb-sense pins of the phy they are
using (these pins are not routed to the outside).
Instead id-detect and vusb-sense are done through any $random gpio pins
(including non irq capable pins on some designs requiring polling).
The polling can still be done in PHY driver and can use the extcon framework
to report the status to controller driver no?
But the musb-core still needs to know the status of the id and vbus pins,
musb-core or the musb-glue (musb/sunxi.c in this case)?
and gets this from the usb0-phy iscr register, which reflects the status of
the not connected dedicated pins of the phy. The reason this can still
work at all is because the iscr register allows the user to override
whatever the not connected phy pins are seeing and forcing a value to
report to the musb core as id and vbus status.
This is done by these 2 functions in the musb sunxi glue:
static void sunxi_musb_force_id(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_ID_HIGH;
else
val = SUNXI_ISCR_FORCE_ID_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_ID_MASK, val);
What will writing to this register lead to? call to sunxi_musb_id_vbus_det_irq
interrupt handler?
}
static void sunxi_musb_force_vbus(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_VBUS_HIGH;
else
val = SUNXI_ISCR_FORCE_VBUS_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_VBUS_MASK,
val);
}
I will happily admit that these 2 functions are a better API between the
sunxi musb
glue and the sunxi usb phy driver. I started with the minimal
sun4i_usb_phy_update_iscr
approach as I wanted to keep the API as small as possible, but having 2
functions like
the one above, which actually reflect what is happening would indeed be better.
Ok, that would definitely improve things.
Note that the polling of the pins cannot (easily) be moved into the phy
driver for various
reasons:
1) It depends on dr_mode, the otg may be used in host only mode in which
case there are no
pins at all.
2) the musb set_vbus callback needs access to the pins
3) When id changes some musb core state changes are necessary.
I'll respin the patch set to do things this way as soon as we've agreement on
your second point.
> and why can't there be a high-level
PHY API for this?
The current generic phy API seems to not have any bus specific methods, I
know that
in the long run people want to get rid of struct usb_phy, so maybe we should
consider
adding bus specific methods to the generic phy API for things like otg.
If we decide to add bus specific methods, then the question becomes if having
int phy_usb_set_id_detect(struct phy *phy, bool val);
int phy_usb_set_vbus_detect(struct phy *phy, bool val);
Functions in the generic phy API is a good idea, or if this is too sunxi
specific,
I'm fine with doing this either way. If we want to go the generic phy route
I'll split this in 2 patches, one adding these 2 generic functions &
phy-ops, and
1 doing the sunxi implementation.
Please don't do that. We don't want to be bloating phy framework with platform
specific hooks.
Cheers
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/15] phy-sun4i-usb: Add a helper function to update the iscr register
Hi,
On 10-03-15 11:53, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 10 March 2015 03:43 PM, Hans de Goede wrote:
Hi,
On 10-03-15 09:57, Arnd Bergmann wrote:
On Tuesday 10 March 2015 09:04:43 Hans de Goede wrote:
Hi,
On 09-03-15 22:47, Arnd Bergmann wrote:
On Monday 09 March 2015 21:40:15 Hans de Goede wrote:
+void sun4i_usb_phy_update_iscr(struct phy *_phy, u32 clr, u32 set)
+{
+ struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+ struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+ u32 iscr;
+
+ iscr = readl(data->base + REG_ISCR);
+ iscr &= ~clr;
+ iscr |= set;
+ writel(iscr, data->base + REG_ISCR);
+}
+EXPORT_SYMBOL(sun4i_usb_phy_update_iscr);
+
I would generally consider this a bad design. What is the purpose of
calling sun4i_usb_phy_update_iscr()
right. That would bind the PHY driver and the controller driver and would
start creating problems when a different PHY is connected with the
controller.
There are 2 different use cases for this one is to enable the dataline
pull-ups at driver init and disable them at driver exit, this could /
should probably be moved to the phy_init / phy_exit code for the usb0 phy
removing the need to do this from within the sunxi musb glue.
The second use-case is more tricky, for some reasons Allwinner has decided
to not use the dedicated id-detect and vusb-sense pins of the phy they are
using (these pins are not routed to the outside).
Instead id-detect and vusb-sense are done through any $random gpio pins
(including non irq capable pins on some designs requiring polling).
The polling can still be done in PHY driver and can use the extcon framework
to report the status to controller driver no?
Technically the polling can be moved to the phy driver yes, but it is not easy,
e.g. we only need to poll when we're in otg mode rather then host-only
or peripheral-only mode, and the mode gets set by the musb driver not phy
the phy driver. The sunxi musb implementation uses an integrated phy, so it
is just much easier and more logical to have all control / polling happening
from a single module rather then from 2 different modules.
But the musb-core still needs to know the status of the id and vbus pins,
musb-core or the musb-glue (musb/sunxi.c in this case)?
and gets this from the usb0-phy iscr register, which reflects the status of
the not connected dedicated pins of the phy. The reason this can still
work at all is because the iscr register allows the user to override
whatever the not connected phy pins are seeing and forcing a value to
report to the musb core as id and vbus status.
This is done by these 2 functions in the musb sunxi glue:
static void sunxi_musb_force_id(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_ID_HIGH;
else
val = SUNXI_ISCR_FORCE_ID_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_ID_MASK, val);
What will writing to this register lead to? call to sunxi_musb_id_vbus_det_irq
interrupt handler?
No this changes the vbus and id status as seen by the musb core, and the musb
responds to this, by e.g. starting a session, or when vbus does not get high
after a session request by reporting a vbus-error interrupt.
The sunxi_musb_id_vbus_det_irq handler gets triggered by edges on the gpio
pins which are used to monitor the id and vbus status.
}
static void sunxi_musb_force_vbus(struct musb *musb, u32 val)
{
struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent);
if (val)
val = SUNXI_ISCR_FORCE_VBUS_HIGH;
else
val = SUNXI_ISCR_FORCE_VBUS_LOW;
sun4i_usb_phy_update_iscr(glue->phy, SUNXI_ISCR_FORCE_VBUS_MASK,
val);
}
I will happily admit that these 2 functions are a better API between the
sunxi musb
glue and the sunxi usb phy driver. I started with the minimal
sun4i_usb_phy_update_iscr
approach as I wanted to keep the API as small as possible, but having 2
functions like
the one above, which actually reflect what is happening would indeed be better.
Ok, that would definitely improve things.
Note that the polling of the pins cannot (easily) be moved into the phy
driver for various
reasons:
1) It depends on dr_mode, the otg may be used in host only mode in which
case there are no
pins at all.
2) the musb set_vbus callback needs access to the pins
3) When id changes some musb core state changes are necessary.
I'll respin the patch set to do things this way as soon as we've agreement on
your second point.
> and why can't there be a high-level
PHY API for this?
The current generic phy API seems to not have any bus specific methods, I
know that
in the long run people want to get rid of struct usb_phy, so maybe we should
consider
adding bus specific methods to the generic phy API for things like otg.
If we decide
[PATCH RESEND] usb: dwc2: rework initialization of host and gadget in dual-role mode
If device is configured to work only in HOST or DEVICE mode, there is
no point in initializing both subdrivers. This patch also fixes
resource leakage if host subdriver fails to initialize.
Signed-off-by: Marek Szyprowski
---
drivers/usb/dwc2/core.h | 2 ++
drivers/usb/dwc2/platform.c | 29 +
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 7a70a1349334..f93b06daef97 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -570,6 +570,8 @@ struct dwc2_hsotg {
struct dwc2_core_params *core_params;
enum usb_otg_state op_state;
enum usb_dr_mode dr_mode;
+ unsigned int hcd_enabled:1;
+ unsigned int gadget_enabled:1;
struct phy *phy;
struct usb_phy *uphy;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 6a795aa2ff05..ee0b0b06d0fc 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -121,8 +121,10 @@ static int dwc2_driver_remove(struct platform_device *dev)
{
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
- dwc2_hcd_remove(hsotg);
- s3c_hsotg_remove(hsotg);
+ if (hsotg->hcd_enabled)
+ dwc2_hcd_remove(hsotg);
+ if (hsotg->gadget_enabled)
+ s3c_hsotg_remove(hsotg);
return 0;
}
@@ -214,12 +216,23 @@ static int dwc2_driver_probe(struct platform_device *dev)
spin_lock_init(&hsotg->lock);
mutex_init(&hsotg->init_mutex);
- retval = dwc2_gadget_init(hsotg, irq);
- if (retval)
- return retval;
- retval = dwc2_hcd_init(hsotg, irq, params);
- if (retval)
- return retval;
+
+ if (hsotg->dr_mode != USB_DR_MODE_HOST) {
+ retval = dwc2_gadget_init(hsotg, irq);
+ if (retval)
+ return retval;
+ hsotg->gadget_enabled = 1;
+ }
+
+ if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
+ retval = dwc2_hcd_init(hsotg, irq, params);
+ if (retval) {
+ if (hsotg->gadget_enabled)
+ s3c_hsotg_remove(hsotg);
+ return retval;
+ }
+ hsotg->hcd_enabled = 1;
+ }
platform_set_drvdata(dev, hsotg);
--
1.9.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
On 10.03.2015 11:40, Jörg Otte wrote: > If I plug in my USB DVB-T stick I get the following in dmesg: > > dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm > state. > dvb-usb: will pass the complete MPEG2 transport stream to the software > demuxer. > DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver) > usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0 > Highspeed DVB-T Receiver)... > input: IR-receiver inside an USB DVB receiver as > /devices/pci:00/:00:14.0/usb1/1-1/input/input17 > dvb-usb: schedule remote query interval to 50 msecs. > xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of > current TD ep_index 1 comp_code 1 > xhci_hcd :00:14.0: Looking for event-dma 000207540400 > trb-start 000207540420 trb-end 000207540420 seg-start > 0002075404 > xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of > current TD ep_index 1 comp_code 1 > xhci_hcd :00:14.0: Looking for event-dma 000207540410 > trb-start 000207540420 trb-end 000207540420 seg-start > 0002075404 > dvb-usb: bulk message failed: -110 (2/0) > > and DVB-T is not functional. The problem came in with: > > 1163d50 Merge tag 'usb-4.0-rc3' of > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb > > I never had this xhci_hcd error before so this is a regression. > > > Thanks, Jörg Oh, thanks. Looks like we get an event for a TRB we just moved past. Any chance you could take a log with xhci debugging enabled before attaching the DVB-T stick? echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control I'd suspect one of these two patches: commit 45ba2154d12fc43b70312198ec47085f10be801a xhci: fix reporting of 0-sized URBs in control endpoint commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa xhci: Clear the host side toggle manually when endpoint is 'soft reset' -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] musb: Do not use musb_read[b|w] / _write[b|w] wrappers in generic fifo functions
On Tuesday 10 March 2015 09:56:52 Hans de Goede wrote: > On 10-03-15 09:50, Arnd Bergmann wrote: > > On Tuesday 10 March 2015 08:43:22 Hans de Goede wrote: > >> On 09-03-15 22:50, Arnd Bergmann wrote: > >>> On Monday 09 March 2015 21:40:18 Hans de Goede wrote: > The generic fifo functions already use non wrapped accesses in various > cases through the iowrite#_rep functions, and all platforms which > override > the default musb_read[b|w] / _write[b|w] functions also provide their own > fifo access functions, so we can safely drop the unnecessary indirection > from the fifo access functions. > > Signed-off-by: Hans de Goede > > >>> > >>> The patch looks reasonably, but the description seem misleading. > >>> I believe the real reason why it's ok to use __raw_writew for the > >>> FIFO is that a FIFO by definition is using CPU endian access for > >>> copying byte streams from memory, which is unlike any other MMIO > >>> register that requires fixed-endian accessors. > >> > >> I'm not sure that that is the case here, this fifo allows reading > >> 4 bytes at a time using 32 bit word access, so endianness may come > >> into play. This patch is safe however since all existing users of > >> the generic fifo_read / write helpers which this patch touches, are > >> also using the generic musb_read[b|w] / _write[b|w] functions which > >> are just __raw_foo wrappers, so there is no functional change, which > >> is what the commit message tries to say. > > > > This probably means that the generic musb helpers are not safe to use > > on big-endian ARM systems (but may work on MIPS). The only one currently > > not using the generic helpers is blackfin, which is fixed to little > > endian. > > > >> Note that sunxi needs this function because of the register address > >> translation the sunxi_musb_readb / writeb wrappers are doing which > >> does not know how to deal with fifo data, and besides that it should > >> make the generic read / write fifo helpers somewhat faster by removing > >> an indirect function call. > > > > Your sunxi_musb_readw/writew functions however also are endian-safe > > and seem to get this part right, unlike all other platforms that use > > the generic __raw_*() accessors. With this patch applied, it should > > actually work fine, and it would work on other platforms as well > > if we change all __raw_*() calls outside of musb_default_write_fifo() > > and musb_default_read_fifo() to use *_relaxed() instead. > > I think that that change falls outside of the scope of this patchset. Yes, sorry for not being clear about that. > I agree that it would be probably a good idea to get rid of the > __raw_foo usage in musb, which is why I've used the non __raw > versions in the sunxi glue, but as said I believe this falls outside > of the scope of this patchset. All my preparation patches for adding > sunxi support carefully do not make any functional changes, as I > do not want to cause regressions on hardware which I cannot test. Right. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8][RESEND]usb:fsl:otg: Add support to add/remove usb host driver
On Tue, 10 Mar 2015, Ramneek Mehresh wrote:
> Add workqueue to add/remove host driver (outside interrupt context)
> upon each id change
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -177,7 +177,9 @@ struct ehci_hcd { /* one per controller */
> unsignedperiodic_count; /* periodic activity count */
> unsigneduframe_periodic_max; /* max periodic time per
> uframe */
>
> -
> +#if defined(CONFIG_FSL_USB2_OTG) || defined(CONFIG_FSL_USB2_OTG_MODULE)
> + struct work_struct change_hcd_work;
> +#endif
This belongs in the ehci_fsl structure, not ehci_hcd. Or maybe in a
generic OTG structure.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
2015-03-10 14:06 GMT+01:00 Mathias Nyman : > On 10.03.2015 11:40, Jörg Otte wrote: >> If I plug in my USB DVB-T stick I get the following in dmesg: >> >> dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm >> state. >> dvb-usb: will pass the complete MPEG2 transport stream to the software >> demuxer. >> DVB: registering new adapter (TerraTec/qanu USB2.0 Highspeed DVB-T Receiver) >> usb 1-1: DVB: registering adapter 0 frontend 0 (TerraTec/qanu USB2.0 >> Highspeed DVB-T Receiver)... >> input: IR-receiver inside an USB DVB receiver as >> /devices/pci:00/:00:14.0/usb1/1-1/input/input17 >> dvb-usb: schedule remote query interval to 50 msecs. >> xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of >> current TD ep_index 1 comp_code 1 >> xhci_hcd :00:14.0: Looking for event-dma 000207540400 >> trb-start 000207540420 trb-end 000207540420 seg-start >> 0002075404 >> xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of >> current TD ep_index 1 comp_code 1 >> xhci_hcd :00:14.0: Looking for event-dma 000207540410 >> trb-start 000207540420 trb-end 000207540420 seg-start >> 0002075404 >> dvb-usb: bulk message failed: -110 (2/0) >> >> and DVB-T is not functional. The problem came in with: >> >> 1163d50 Merge tag 'usb-4.0-rc3' of >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb >> >> I never had this xhci_hcd error before so this is a regression. >> >> >> Thanks, Jörg > > Oh, thanks. > > Looks like we get an event for a TRB we just moved past. > > Any chance you could take a log with xhci debugging enabled before attaching > the DVB-T > stick? > > echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control > > here it comes attached. > I'd suspect one of these two patches: > > commit 45ba2154d12fc43b70312198ec47085f10be801a > xhci: fix reporting of 0-sized URBs in control endpoint > > commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa > xhci: Clear the host side toggle manually when endpoint is 'soft reset' > > -Mathias > Thanks, Jörg xhci-debug.gz Description: GNU Zip compressed data
Re: [PATCH] phy: Add a driver for dm816x USB PHY
On Mon, Mar 9, 2015 at 4:41 PM, Tony Lindgren wrote: > * Bin Liu [150309 14:35]: >> On Mon, Mar 9, 2015 at 4:26 PM, Tony Lindgren wrote: >> > * Felipe Balbi [150309 14:21]: >> >> On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote: >> >> > On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi wrote: >> >> > > On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote: >> >> > >> Hi, >> >> > >> >> >> > >> On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren >> >> > >> wrote: >> >> > >> > Add a minimal driver for dm816x USB. Otherwise we can just use >> >> > >> > the existing musb_am335x and musb_dsps on dm816x. >> >> > >> >> >> > >> dm816x has the almost identical usbss as that in am335x, we should be >> >> > >> able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too? >> >> > > >> >> > > Tony's using the same musb glue layers, this is just a phy driver, >> >> > > right ? >> >> > >> >> > Can the current am335x phy driver be adopted too? I remember it is >> >> > under drivers/usb/phy/. >> >> >> >> Tony will be best to answer, but according to our IRC discussions, it's >> >> too different. Tony ? >> > >> > Well we should check again between dm814x and am335x documentation >> > against the $subject driver as the dm816x docs were buggy and I may >> > have gotten a wrong idea initially. >> >> Will it make our life a little easier by comparing the usb drivers for >> dm81xx and am335x in previous TI kernel releases? I cam dig out the >> source code if that helps. > > Yeah that probably helps if you've dealt with dm814x earlier :) Well, don't expect much from me about dm814x ;), I did not read its trm nor errata, but only booted the board a few times trying to compare the MUSB behaviours while debugging issues on am335x. Ok, the previous TI released 3.2 kernel for am335x is at [1], and 2.6.37 kernel dm81xx is at [2]. Regards, -Bin. [1] http://arago-project.org/git/projects/?p=linux-am33x.git;a=shortlog;h=refs/heads/v3.2-staging [2] http://arago-project.org/git/projects/?p=linux-omap3.git;a=shortlog;h=refs/heads/TI81XXPSP_04.04.00.01 > Just don't look at the dm816x trm, only the errata pdf works for > dm816x phy. > > Note that we still are missing basic support for dm814x in mainline, > I'm planning to tackle that at some point but I don't know when I'm > going to get to it.. > > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Control message failures kill entire XHCI stack
Hello Baolu, I'm doing most of my testing on a 2014 Retina Macbook Pro. lsusb output below. I've also seen the issue on an Intel NUC D34010WYKH, which I don't have in front of me or I would provide the lsusb output for that as well. Apologies to Mathias for not getting back to this sooner. I've been buried in another project but plan to get back to it this week. Thanks for investigating this issue. Cheers, Devin Bus 002 Device 002: ID 05ac:8406 Apple, Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x05ac Apple, Inc. idProduct 0x8406 bcdDevice8.20 iManufacturer 3 Apple iProduct4 Card Reader iSerial 5 0820 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 44 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 224mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 4 Binary Object Store Descriptor: bLength 5 bDescriptorType15 wTotalLength 22 bNumDeviceCaps 2 USB 2.0 Extension Device Capability: bLength 7 bDescriptorType16 bDevCapabilityType 2 bmAttributes 0x0002 Link Power Management (LPM) Supported SuperSpeed USB Device Capability: bLength10 bDescriptorType16 bDevCapabilityType 3 bmAttributes 0x02 Latency Tolerance Messages (LTM) Supported wSpeedsSupported 0x000e Device can operate at Full Speed (12Mbps) Device can operate at High Speed (480Mbps) Device can operate at SuperSpeed (5Gbps) bFunctionalitySupport 1 Lowest fully-functional device speed is Full Speed (12Mbps) bU1DevExitLat 10 micro seconds bU2DevExitLat2047 micro seconds Device Status: 0x0010 (Bus Powered) Latency Tolerance Messaging (LTM) Enabled Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 3 bMaxPacketSize0 9 idVendor 0x1d6b Linux Foundation idProduct 0x0003 3.0 root hub bcdDevice3.19 iManufacturer 3 Linux 3.19.0-rc5djh+ xhci-hcd iProduct2 xHCI Host Controller iSerial 1 :00:14.0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 31 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0004 1x 4 byt
Re: [PATCH 12/15] ARM: dts: sun4i: Enable USB DRC on Chuwi V7 CW0825
Hi Hans,
On Mon, Mar 09, 2015 at 09:40:25PM +0100, Hans de Goede wrote:
> Enable the otg/drc usb controller on the Chuwi V7 CW0825 tablet.
>
> Signed-off-by: Hans de Goede
> ---
> arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts | 31
> +
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> b/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> index 97fca89..034396c 100644
> --- a/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> @@ -111,6 +111,26 @@
> status = "okay";
> };
>
> +&pio {
> + usb0_id_detect_pin: usb0_id_detect_pin@0 {
> + allwinner,pins = "PH4";
> + allwinner,function = "gpio_in";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + usb0_vbus_detect_pin: usb0_vbus_detect_pin@0 {
> + allwinner,pins = "PH5";
> + allwinner,function = "gpio_in";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +};
> +
> +®_usb0_vbus {
> + status = "okay";
> +};
> +
> ®_usb2_vbus {
> status = "okay";
> };
> @@ -121,7 +141,18 @@
> status = "okay";
> };
>
> +&usb_otg {
> + pinctrl-names = "default";
> + pinctrl-0 = <&usb0_id_detect_pin
> + &usb0_vbus_detect_pin>;
> + id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
> + vbus_det-gpio = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
> + dr_mode = "otg";
Is there a reason to not put that in the DTSI?
All the users seem to be wiring it as OTG.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 12/15] ARM: dts: sun4i: Enable USB DRC on Chuwi V7 CW0825
Hi,
On 03/10/2015 04:07 PM, Maxime Ripard wrote:
Hi Hans,
On Mon, Mar 09, 2015 at 09:40:25PM +0100, Hans de Goede wrote:
Enable the otg/drc usb controller on the Chuwi V7 CW0825 tablet.
Signed-off-by: Hans de Goede
---
arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts | 31 +
1 file changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
b/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
index 97fca89..034396c 100644
--- a/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
+++ b/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
@@ -111,6 +111,26 @@
status = "okay";
};
+&pio {
+ usb0_id_detect_pin: usb0_id_detect_pin@0 {
+ allwinner,pins = "PH4";
+ allwinner,function = "gpio_in";
+ allwinner,drive = ;
+ allwinner,pull = ;
+ };
+
+ usb0_vbus_detect_pin: usb0_vbus_detect_pin@0 {
+ allwinner,pins = "PH5";
+ allwinner,function = "gpio_in";
+ allwinner,drive = ;
+ allwinner,pull = ;
+ };
+};
+
+®_usb0_vbus {
+ status = "okay";
+};
+
®_usb2_vbus {
status = "okay";
};
@@ -121,7 +141,18 @@
status = "okay";
};
+&usb_otg {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb0_id_detect_pin
+&usb0_vbus_detect_pin>;
+ id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
+ vbus_det-gpio = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
+ dr_mode = "otg";
Is there a reason to not put that in the DTSI?
This is not in the dtsi because it is board specific.
All the users seem to be wiring it as OTG.
Sofar I've only enabled the otg on a handful of boards,
the otg on the Mele-M9 for example is connected to a usb->sata
bridge and as such should be brought up in host only mode.
And on the wobo-i5 (which I still need to create a dts file for)
the otg is connected to an usb wifi module.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbhid: add quirk for a Logitech mouse
From: Oliver Neukum
This device disconnects every 60s without X
Signed-off-by: Oliver Neukum
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/usbhid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 204312b..8a7c347 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -586,6 +586,7 @@
#define USB_VENDOR_ID_LOGITECH 0x046d
#define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
#define USB_DEVICE_ID_LOGITECH_T6510xb00c
+#define USB_DEVICE_ID_LOGITECH_C0770xc007
#define USB_DEVICE_ID_LOGITECH_RECEIVER0xc101
#define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110
#define USB_DEVICE_ID_LOGITECH_HARMONY_LAST 0xc14f
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 9be99a67..a821277 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -78,6 +78,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_ELO, USB_DEVICE_ID_ELO_TS2700, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
HID_QUIRK_NOGET },
+ { USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_C077,
HID_QUIRK_ALWAYS_POLL },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3_JP,
HID_QUIRK_NO_INIT_REPORTS },
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb: dsps: don't fake of_node to musb core
If we pass our own of_node to musb_core, at least pinctrl settings will be duplicated, meaning that pinctrl framework will try to select default pin state for musb_core when they were already requested by musb-dsps. A Warning will be printed however things will still work. Reported-by: Tony Lindgren Signed-off-by: Felipe Balbi --- drivers/usb/musb/musb_dsps.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index baa757ba1353..850da05be696 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -697,7 +697,6 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, musb->dev.parent= dev; musb->dev.dma_mask = &musb_dmamask; musb->dev.coherent_dma_mask = musb_dmamask; - musb->dev.of_node = of_node_get(dn); glue->musb = musb; -- 2.3.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
On 10.03.2015 17:36, Jörg Otte wrote: >>> Any chance you could take a log with xhci debugging enabled before >>> attaching the DVB-T >>> stick? >>> >>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control >>> >>> >> >> here it comes attached. >> >> >>> I'd suspect one of these two patches: >>> >>> commit 45ba2154d12fc43b70312198ec47085f10be801a >>> xhci: fix reporting of 0-sized URBs in control endpoint >>> >>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa >>> xhci: Clear the host side toggle manually when endpoint is 'soft reset' >>> > > Revert the commits. > The second one "xhci: Clear the host side..." is it ! > Yes, thank you Seems that It wasn't mature enough, I'll revert it. >From your logs I can see what went wrong, If you still have some time, could you try out a patch (attached) and see if it solves the issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own USB DVB-T device -Mathias >From a895eb69a63dfef1943f0593da29167bea12100c Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Tue, 10 Mar 2015 18:50:45 +0200 Subject: [PATCH] xhci: set correct dequeue status in endpoint soft reset The endpoint might already processesed some TRBs on the endpiont ring before we soft reset the endpoint. Make sure we set the dequeue pointer to where we were befere soft reset Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index b06d1a5..64527a4 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2972,6 +2972,8 @@ void xhci_endpoint_reset(struct usb_hcd *hcd, unsigned int ep_index, ep_state; unsigned long flags; u32 ep_flag; + struct xhci_ep_ctx *ep_ctx; + dma_addr_t addr; xhci = hcd_to_xhci(hcd); udev = (struct usb_device *) ep->hcpriv; @@ -3046,6 +3048,9 @@ void xhci_endpoint_reset(struct usb_hcd *hcd, virt_dev->out_ctx, ctrl_ctx, ep_flag, ep_flag); xhci_endpoint_copy(xhci, command->in_ctx, virt_dev->out_ctx, ep_index); + ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index); + addr = xhci_trb_virt_to_dma(virt_ep->ring->deq_seg, virt_ep->ring->dequeue); + ep_ctx->deq = cpu_to_le64(addr | virt_ep->ring->cycle_state); xhci_queue_configure_endpoint(xhci, command, command->in_ctx->dma, udev->slot_id, false); -- 1.8.3.2
Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
On Tue, 10 Mar 2015, Mathias Nyman wrote: > Yes, thank you > > Seems that It wasn't mature enough, I'll revert it. > > From your logs I can see what went wrong, > > If you still have some time, could you try out a patch (attached) and see if > it solves the > issue for you. (on top of clean 4.0-rc3). I can't reproduce it with my own > USB DVB-T device Mathias: Your patch description says this: > The endpoint might already processesed some TRBs on the endpiont ring > before we soft reset the endpoint. > Make sure we set the dequeue pointer to where we were befere soft reset However, if a driver tries to issue an endpoint reset while there are still some URBs queued, it is a bug. Host controller drivers shouldn't have to worry about this -- xhci_endpoint_reset() should simply return an error if the endpoint ring isn't empty. I suppose we should check for this in the USB core. I'll write a patch and CC: you. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/15] musb: Add support for the Allwinner sunxi musb controller
On Mon, Mar 09, 2015 at 09:40:13PM +0100, Hans de Goede wrote: > Hi All, > > This patch set has been a while in the making, so I'm very happy to present > the end result here, and I hope everyone likes it. > > Before talking about merging this there are 2 things which I would like to > point out: > > a) The musb controller in the sunxi SoCs uses some SRAM which needs to be > mapped to the musb controller by poking some bits in the SRAM controller, > just like the EMAC patches which were send a while back I've chosen to use > syscon for this, actually 2 of the patches in this set come directly from the > SRAM mapping patchset for the EMAC. > > I know that Maxime is not 100% in favor of using syscon: > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/320221.html > > But I disagree with his arguments for writing a special driver for the SRAM > controller: And I disagree with your disagreement :) > 1) syscon was specifically designed for global system control registers like > this and is fine to use as long as there are no conflicts where 1 bit is of > interest to multiple drivers, and there is no such conflict here No. Syscon has been designed for being lazy. This is a huge abstraction non-sense. You have to put all the logic of dealing with some external register layout in clients drivers, including dealing with the different revisions/SoC that are different from that aspect, and duplicating that code across all client drivers. > 2) Maxime's other arguments seem to boil down to it would be nice / prettier > to have a specific driver for this, without really proving a hard need for > such a driver. But writing such a driver is going to be a lot of work, and > we've a ton of other work to do, and as said there is no real need for a > separate driver, syscon works fine for this. Actually, I already wrote some prototype for this. I'll clean this up and send it tonight/tomorrow. > 3) I actually believe that having a specific driver for this is a bad idea, > because that means inventing a whole new cross driver API for this, and > getting those right is, hard, a lot of work, and even then one is still likely > to get it wrong. We can avoid all this by going with the proven syscon > solution. Except that modifying an in-kernel API, especially when we have two users, is easy. Moving back from syscon is not. > Maxime, can we please have your ack for moving forward with this using syscon? > (see above for my arguments why) If you can address my objections above, sure. Thanks for your awesome work on this, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
[RFC] Warn about resets of active endpoints
On Tue, 10 Mar 2015, Alan Stern wrote:
> However, if a driver tries to issue an endpoint reset while there are
> still some URBs queued, it is a bug. Host controller drivers shouldn't
> have to worry about this -- xhci_endpoint_reset() should simply return
> an error if the endpoint ring isn't empty.
>
> I suppose we should check for this in the USB core. I'll write a patch
> and CC: you.
Here it is. Does this mean your new patch isn't needed?
Jörg, what do you get if you test with this patch applied?
Alan Stern
Index: usb-3.19/drivers/usb/core/hcd.c
===
--- usb-3.19.orig/drivers/usb/core/hcd.c
+++ usb-3.19/drivers/usb/core/hcd.c
@@ -2018,6 +2018,14 @@ void usb_hcd_reset_endpoint(struct usb_d
{
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+ /* Resetting an active endpoint is a bug! */
+ if (!list_empty(&ep->urb_list)) {
+ WARN(1, "%s ep %02x: attempt to reset active endpoint\n",
+ dev_name(&udev->dev),
+ ep->desc.bEndpointAddress);
+ return;
+ }
+
if (hcd->driver->endpoint_reset)
hcd->driver->endpoint_reset(hcd, ep);
else {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1] xhci fixes for usb-linus, revert patch that went to 4.0-rc3
Hi Greg One xhci patch that went to 4.0-rc3 needs to be reverted Turns out the fix for xhci connected scanners cause regression on some DVB-T devices. The bug in the original patch is found, but I need to make sure everything works properly first, so it's better to revert this than try to fix it on the fly. Mathias Nyman (1): Revert "xhci: Clear the host side toggle manually when endpoint is 'soft reset'" drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.c | 100 --- drivers/usb/host/xhci.h | 2 - 3 files changed, 10 insertions(+), 94 deletions(-) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] Revert "xhci: Clear the host side toggle manually when endpoint is 'soft reset'"
This reverts commit 27082e2654dc ("xhci: Clear the host side toggle manually")
Turns out this fix to enable soft resetting endpoints wasn't mature enough.
It caused regression with some usb DVB-T devices and needs some more tuning
to get the endpiont ring pointers set correctly.
The original commit was tagged for stable 3.18, and should be reverted
from there as well.
Cc: stable # v3.18
Signed-off-by: Mathias Nyman
---
drivers/usb/host/xhci-ring.c | 2 +-
drivers/usb/host/xhci.c | 100 ---
drivers/usb/host/xhci.h | 2 -
3 files changed, 10 insertions(+), 94 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5fb66db..73485fa 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1729,7 +1729,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd
*xhci,
if (!command)
return;
- ep->ep_state |= EP_HALTED | EP_RECENTLY_HALTED;
+ ep->ep_state |= EP_HALTED;
ep->stopped_stream = stream_id;
xhci_queue_reset_ep(xhci, command, slot_id, ep_index);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b06d1a5..ec8ac16 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1338,12 +1338,6 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb
*urb, gfp_t mem_flags)
goto exit;
}
- /* Reject urb if endpoint is in soft reset, queue must stay empty */
- if (xhci->devs[slot_id]->eps[ep_index].ep_state & EP_CONFIG_PENDING) {
- xhci_warn(xhci, "Can't enqueue URB while ep is in soft
reset\n");
- ret = -EINVAL;
- }
-
if (usb_endpoint_xfer_isoc(&urb->ep->desc))
size = urb->number_of_packets;
else
@@ -2954,36 +2948,23 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci,
}
}
-/* Called after clearing a halted device. USB core should have sent the control
+/* Called when clearing halted device. The core should have sent the control
* message to clear the device halt condition. The host side of the halt should
- * already be cleared with a reset endpoint command issued immediately when the
- * STALL tx event was received.
+ * already be cleared with a reset endpoint command issued when the STALL tx
+ * event was received.
+ *
+ * Context: in_interrupt
*/
void xhci_endpoint_reset(struct usb_hcd *hcd,
struct usb_host_endpoint *ep)
{
struct xhci_hcd *xhci;
- struct usb_device *udev;
- struct xhci_virt_device *virt_dev;
- struct xhci_virt_ep *virt_ep;
- struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_command *command;
- unsigned int ep_index, ep_state;
- unsigned long flags;
- u32 ep_flag;
xhci = hcd_to_xhci(hcd);
- udev = (struct usb_device *) ep->hcpriv;
- if (!ep->hcpriv)
- return;
- virt_dev = xhci->devs[udev->slot_id];
- ep_index = xhci_get_endpoint_index(&ep->desc);
- virt_ep = &virt_dev->eps[ep_index];
- ep_state = virt_ep->ep_state;
/*
-* Implement the config ep command in xhci 4.6.8 additional note:
+* We might need to implement the config ep cmd in xhci 4.8.1 note:
* The Reset Endpoint Command may only be issued to endpoints in the
* Halted state. If software wishes reset the Data Toggle or Sequence
* Number of an endpoint that isn't in the Halted state, then software
@@ -2991,72 +2972,9 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
* for the target endpoint. that is in the Stopped state.
*/
- if (ep_state & SET_DEQ_PENDING || ep_state & EP_RECENTLY_HALTED) {
- virt_ep->ep_state &= ~EP_RECENTLY_HALTED;
- xhci_dbg(xhci, "ep recently halted, no toggle reset needed\n");
- return;
- }
-
- /* Only interrupt and bulk ep's use Data toggle, USB2 spec 5.5.4-> */
- if (usb_endpoint_xfer_control(&ep->desc) ||
- usb_endpoint_xfer_isoc(&ep->desc))
- return;
-
- ep_flag = xhci_get_endpoint_flag(&ep->desc);
-
- if (ep_flag == SLOT_FLAG || ep_flag == EP0_FLAG)
- return;
-
- command = xhci_alloc_command(xhci, true, true, GFP_NOWAIT);
- if (!command) {
- xhci_err(xhci, "Could not allocate xHCI command structure.\n");
- return;
- }
-
- spin_lock_irqsave(&xhci->lock, flags);
-
- /* block ringing ep doorbell */
- virt_ep->ep_state |= EP_CONFIG_PENDING;
-
- /*
-* Make sure endpoint ring is empty before resetting the toggle/seq.
-* Driver is required to synchronously cancel all transfer request.
-*
-* xhci 4.6.6 says we can issue a configure endpoint command on a
-* running endpoint ring as long as it's idle (queue empty)
-*/
-
- if (!list_empty(&virt_ep->ri
RE: [PATCH 2/8][RESEND]usb:fsl:otg: Add support to add/remove usb host driver
> -Original Message-
> From: Alan Stern [mailto:st...@rowland.harvard.edu]
> Sent: Tuesday, March 10, 2015 7:21 PM
> To: Mehresh Ramneek-B31383
> Cc: linux-usb@vger.kernel.org; ba...@ti.com; gre...@linuxfoundation.org; Li
> Yang-Leo-R58472
> Subject: Re: [PATCH 2/8][RESEND]usb:fsl:otg: Add support to add/remove
> usb host driver
>
> On Tue, 10 Mar 2015, Ramneek Mehresh wrote:
>
> > Add workqueue to add/remove host driver (outside interrupt context)
> > upon each id change
>
> > --- a/drivers/usb/host/ehci.h
> > +++ b/drivers/usb/host/ehci.h
> > @@ -177,7 +177,9 @@ struct ehci_hcd { /* one per
> controller */
> > unsignedperiodic_count; /* periodic activity
> count */
> > unsigneduframe_periodic_max; /* max periodic time
> per uframe */
> >
> > -
> > +#if defined(CONFIG_FSL_USB2_OTG) ||
> defined(CONFIG_FSL_USB2_OTG_MODULE)
> > + struct work_struct change_hcd_work;
> > +#endif
>
> This belongs in the ehci_fsl structure, not ehci_hcd. Or maybe in a generic
> OTG structure.
>
Understood...let me try moving this into ehci_fsl
Thanks.
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/15] ARM: dts: sun4i: Enable USB DRC on Chuwi V7 CW0825
On Tue, Mar 10, 2015 at 04:23:09PM +0100, Hans de Goede wrote:
> Hi,
>
> On 03/10/2015 04:07 PM, Maxime Ripard wrote:
> >Hi Hans,
> >
> >On Mon, Mar 09, 2015 at 09:40:25PM +0100, Hans de Goede wrote:
> >>Enable the otg/drc usb controller on the Chuwi V7 CW0825 tablet.
> >>
> >>Signed-off-by: Hans de Goede
> >>---
> >> arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts | 31
> >> +
> >> 1 file changed, 31 insertions(+)
> >>
> >>diff --git a/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> >>b/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> >>index 97fca89..034396c 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> >>+++ b/arch/arm/boot/dts/sun4i-a10-chuwi-v7-cw0825.dts
> >>@@ -111,6 +111,26 @@
> >>status = "okay";
> >> };
> >>
> >>+&pio {
> >>+ usb0_id_detect_pin: usb0_id_detect_pin@0 {
> >>+ allwinner,pins = "PH4";
> >>+ allwinner,function = "gpio_in";
> >>+ allwinner,drive = ;
> >>+ allwinner,pull = ;
> >>+ };
> >>+
> >>+ usb0_vbus_detect_pin: usb0_vbus_detect_pin@0 {
> >>+ allwinner,pins = "PH5";
> >>+ allwinner,function = "gpio_in";
> >>+ allwinner,drive = ;
> >>+ allwinner,pull = ;
> >>+ };
> >>+};
> >>+
> >>+®_usb0_vbus {
> >>+ status = "okay";
> >>+};
> >>+
> >> ®_usb2_vbus {
> >>status = "okay";
> >> };
> >>@@ -121,7 +141,18 @@
> >>status = "okay";
> >> };
> >>
> >>+&usb_otg {
> >>+ pinctrl-names = "default";
> >>+ pinctrl-0 = <&usb0_id_detect_pin
> >>+&usb0_vbus_detect_pin>;
> >>+ id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
> >>+ vbus_det-gpio = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */
> >>+ dr_mode = "otg";
> >
> >Is there a reason to not put that in the DTSI?
>
> This is not in the dtsi because it is board specific.
>
> >All the users seem to be wiring it as OTG.
>
> Sofar I've only enabled the otg on a handful of boards,
> the otg on the Mele-M9 for example is connected to a usb->sata
> bridge and as such should be brought up in host only mode.
>
> And on the wobo-i5 (which I still need to create a dts file for)
> the otg is connected to an usb wifi module.
Ok, that seems like a good reason :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [V4.0.0-rc3] Xhci Regression: ERROR Transfer event TRB DMA ptr not part of current TD
On Tue, 10 Mar 2015, Mathias Nyman wrote: > > Mathias: > > > > Your patch description says this: > > > >> The endpoint might already processesed some TRBs on the endpiont ring > >> before we soft reset the endpoint. > >> Make sure we set the dequeue pointer to where we were befere soft reset > > > > However, if a driver tries to issue an endpoint reset while there are > > still some URBs queued, it is a bug. Host controller drivers shouldn't > > have to worry about this -- xhci_endpoint_reset() should simply return > > an error if the endpoint ring isn't empty. > > > > I suppose we should check for this in the USB core. I'll write a patch > > and CC: you. > > > > Alan Stern > > > > It's possible that there's something in usb core as well, > but I think the following was what happened: > > 1. First a normal configure endpoint command is issued, it sets endpoint > dequeue pointer >to xxx400 = start of ring segment > 2. two urbs get queued -> two TDs put on endpoint ring. > 3. xhci executes those, ring is in running (idle) state. sw dequeue at > xxx430, No TDs queued. >Endpoint dequeue pointer is not written to the endpoint output context as > the ring is still >in running state (even if idle, not advancing with no TDs queued) it still > shows xxx400 > 4. -> something happends, xhci_endpoint_reset() is called, we do a new > configure endpoint >to 'soft reset' the endpiont, but we copy the dequeue pointer from the old > endpoint >output context to the configure endpoint input context, which > re-initializes the old >dequeue xxx400 pointer to xhci hardware, and it starts executing the old > TDs from the ring. Obviously that's bad. But don't you have to stop the endpoint ring in order to configure it? When you stop the ring, doesn't the controller store the correct current value of the dequeue pointer somewhere? > 5. xhci driver notices that we get events for old TRBs that do not belong to > the TD the driver >thinks we should be handling Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] usb: chipidea: otg_fsm: add HNP polling support
On Mon, Mar 09, 2015 at 10:09:21AM +0800, Li Jun wrote: > From: Li Jun > > Add a dedicataed normal timer for HNP polling since it's cyclical, while > in peripheral mode, change a/b_bus_req to be 1 will make it response > to host request flag with 1, then role switch will be started. > > Signed-off-by: Li Jun I don't get why you have so much code hidden away in a single controller. You should really add this timer much more generically so that there's no change needed on chipidea/musb/dwc2/dwc3/etc. -- balbi signature.asc Description: Digital signature
Re: [PATCH] drivers: usb: gadget: udc: Fix NULL dereference
On Tue, Mar 10, 2015 at 03:00:49AM +, Peter Chen wrote:
>
> > On Tue, Mar 10, 2015 at 02:02:44AM +, Peter Chen wrote:
> > >
> > > > > --- a/drivers/usb/gadget/udc/lpc32xx_udc.c
> > > > > +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
> > > > > @@ -1803,7 +1803,7 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep,
> > > > > req = container_of(_req, struct lpc32xx_request, req);
> > > > > ep = container_of(_ep, struct lpc32xx_ep, ep);
> > > > >
> > > > > - if (!_req || !_req->complete || !_req->buf ||
> > > > > + if (!_ep || !_req || !_req->complete || !_req->buf ||
> > > > > !list_empty(&req->queue))
> > > > > return -EINVAL;
> > > > >
> > > > > @@ -1815,8 +1815,7 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep,
> > > > > }
> > > > >
> > > > >
> > > > > - if ((!udc) || (!udc->driver) ||
> > > > > - (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
> > > > > + if ((!udc->driver) || (udc->gadget.speed == USB_SPEED_UNKNOWN))
> > > > {
> > > > > dev_dbg(udc->dev, "invalid device\n");
> > > > > return -EINVAL;
> > > > > }
> > > >
> > > > what's going to happen here ?
> > > >
> > >
> > > I just changed the current code, in fact, udc->driver is impossible to
> > > NULL which is cleared at .udc_stop.
> > >
> > > The speed is possible for unknown if the reset has occurred at that time.
> >
> > oh, alright.
>
> Do you need me or Tapasweni send patch for this?
if there's anything to be fixed, sure.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] udc: gadget: atmel_usba_udc: depend on COMMON_CLK_AT91
On Tue, Mar 03, 2015 at 10:41:38AM +0100, Alexandre Belloni wrote: > On 03/03/2015 at 09:26:20 +0100, Boris Brezillon wrote : > > > config USB_ATMEL_USBA > > > tristate "Atmel USBA" > > > - depends on AVR32 || ARCH_AT91 > > > + depends on AVR32 || ARCH_AT91 && COMMON_CLK_AT91 > > > > I guess you should add parenthesis to make it clearer ? > > > > depends on AVR32 || (ARCH_AT91 && COMMON_CLK_AT91) > > > > And I wonder why you need that. I though this option was selected by all > > at91 platforms ? > > > > That is currently the case but maybe, one day, one of the AT91 platform > will not use the same clock driver. then, maybe, one day, you send this patch. -- balbi signature.asc Description: Digital signature
Re: gadgetfs broken since 7f7f25e8
On Sun, Mar 08, 2015 at 02:35:25PM -0400, Alan Stern wrote: > On Sun, 8 Mar 2015, Al Viro wrote: > > > On Sat, Mar 07, 2015 at 04:08:49PM -0500, Alan Stern wrote: > > > On Sat, 7 Mar 2015, Alexander Holler wrote: > > > > > > > Am 07.03.2015 um 12:23 schrieb Alexander Holler: > > > > > Am 04.03.2015 um 16:31 schrieb Alan Stern: > > > > > > > > > >> check to see what those values actually were. It's easy enough to > > > > >> fix > > > > >> up, though; revised patch below. > > > > > > > > > > Thanks, in contrast to the patch from Al Viro that one applies. > > > > > > > > And as I've just tested it, it isn't complete. ep_config_operations will > > > > be switched to ep_io_operations and suffers under the same problem of > > > > not having initially (aio_)read/(aio_)write since commit 7f7f25e8 > > > > (3.16). > > > > > > Of course. I stated in the email accompanying the original version of > > > this patch that it contained some corrections that Al's patch left out. > > > You have to use the two of them together to fix all the issues. > > > > FWIW, I've pushed those two fixes in vfs.git#gadget; could I have your > > s-o-b on the ep0 part? See 2b13438 in vfs.git... > > Certainly. > > Signed-off-by: Alan Stern Just to make sure people know I'm okay with you taking the patch: Acked-by: Felipe Balbi -- balbi signature.asc Description: Digital signature
Re: [PATCHv7 0/4] USB: gadget: atmel_usba_udc: PM driver improvements
On Wed, Mar 04, 2015 at 02:12:51PM +0100, Nicolas Ferre wrote: > Le 12/02/2015 18:54, Sylvain Rochet a écrit : > > Start clocks on rising edge of the Vbus signal, stop clocks on falling > > edge of the Vbus signal. > > > > Add suspend/resume with wakeup support. > > Hi Felipe, > > I think this series by Sylvain is ready for inclusion. Are you able to > take it for the next cycle? taking it now. Thanks -- balbi signature.asc Description: Digital signature
Re: [PATCHv7 2/4] USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable
Hi, (dropping patch, my only context is subject line) "USB: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ instead of an auto enabled IRQ request followed by IRQ disable" Holy crap, that's a pretty long patch *short* description. I'm trimming it to: "usb: gadget: atmel_usba_udc: Request an auto disabled Vbus signal IRQ" cheers -- balbi signature.asc Description: Digital signature
Re: [PATCHv7 3/4] USB: gadget: atmel_usba_udc: Start clocks on rising edge of the Vbus signal, stop clocks on falling edge of the Vbus signal
Hi, this one became: "usb: gadget: atmel_usba_udc: condition clocks to vbus state" cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/1] usb: common: otg-fsm: only signal connect after switching to peripheral
On Sun, Feb 15, 2015 at 03:39:48PM +0800, Peter Chen wrote: > We should signal connect (pull up dp) after we have already > at peripheral mode, otherwise, the dp may be toggled due to > we reset controller or do disconnect during the initialization > for peripheral, then, the host may be confused during the > enumeration, eg, it finds the reset can't succeed, but the > device is still there, see below error message. > > hub 1-0:1.0: USB hub found > hub 1-0:1.0: 1 port detected > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: cannot reset port 1 (err = -32) > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > hub 1-0:1.0: unable to enumerate USB device on port 1 > > Signed-off-by: Peter Chen when was this introduced ? Do you want to Cc stable ? (hint: you do) -- balbi signature.asc Description: Digital signature
Re: [PATCH] udc: gadget: atmel_usba_udc: depend on COMMON_CLK_AT91
On 10/03/2015 at 15:53:12 -0500, Felipe Balbi wrote : > On Tue, Mar 03, 2015 at 10:41:38AM +0100, Alexandre Belloni wrote: > > On 03/03/2015 at 09:26:20 +0100, Boris Brezillon wrote : > > > > config USB_ATMEL_USBA > > > > tristate "Atmel USBA" > > > > - depends on AVR32 || ARCH_AT91 > > > > + depends on AVR32 || ARCH_AT91 && COMMON_CLK_AT91 > > > > > > I guess you should add parenthesis to make it clearer ? > > > > > > depends on AVR32 || (ARCH_AT91 && COMMON_CLK_AT91) > > > > > > And I wonder why you need that. I though this option was selected by all > > > at91 platforms ? > > > > > > > That is currently the case but maybe, one day, one of the AT91 platform > > will not use the same clock driver. > > then, maybe, one day, you send this patch. Yeah, let's drop it for now but I have the feeling that this will break (I actually broke it when switching at91 to multiplatform). -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: phy: phy-generic: No need to call gpiod_direction_output() twice
On Tue, Feb 03, 2015 at 07:18:17PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam
>
> Commit 9eb0797722895f4309b4 ("usb: phy: generic: fix the gpios to be
> optional")
> calls gpiod_direction_output() in the probe function, so there is no need to
> call it again, as we can simply call gpiod_set_value() directly.
>
> Also, in usb_gen_phy_shutdown() we can simply put the GPIO directly in its
> active level state and this allows us to simplify the nop_reset function to
> treat only the reset case.
>
> Signed-off-by: Fabio Estevam
Is this for v4.1 or v4.0-rc ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3] usb: phy: phy-generic: No need to call gpiod_direction_output() twice
On Tue, Mar 10, 2015 at 6:20 PM, Felipe Balbi wrote:
> On Tue, Feb 03, 2015 at 07:18:17PM -0200, Fabio Estevam wrote:
>> From: Fabio Estevam
>>
>> Commit 9eb0797722895f4309b4 ("usb: phy: generic: fix the gpios to be
>> optional")
>> calls gpiod_direction_output() in the probe function, so there is no need to
>> call it again, as we can simply call gpiod_set_value() directly.
>>
>> Also, in usb_gen_phy_shutdown() we can simply put the GPIO directly in its
>> active level state and this allows us to simplify the nop_reset function to
>> treat only the reset case.
>>
>> Signed-off-by: Fabio Estevam
>
> Is this for v4.1 or v4.0-rc ?
For 4.1, thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: commit ef11982dd7a657512c362242508bb4021e0d67b6 breaks musb
On Thu, Mar 05, 2015 at 12:25:48PM +0530, Amit Virdi wrote: > +cc Sebastian, Alan Stern > > Hello Felipe, > > On 2/28/2015 3:18 AM, Felipe Balbi wrote: > >Hi Amit, > > > >commit ef11982dd7a657512c362242508bb4021e0d67b6 (Add support for > >interrupt EP) actually broke testusb for MUSB when MUSB is the gadget. > > > >The reason is that we're requesting an endpoint with a 64-byte FIFO, but > >later deciding to use the same endpoint with wMaxPacketSize set to 1024 > >and MUSB errors out because the endpoint was selected for 64-byte only. > > > >This only happens when trying to set alternate setting 2 and it's pretty > >easy to trigger. > > > > This issue was brought up earlier by Sebastian. He even submitted a patch > for the same and there were discussions over it. > http://www.spinics.net/lists/linux-usb/msg117962.html > > However, things were never concluded. Using module parameters should get it > working but that isn't a clean solution. > > What do you suggest? allocate endpoints based on largest wMaxPacketSize ? If we have more FIFO space than needed, that's not a problem. The problem is when we allocate a FIFO which is not large enough. -- balbi signature.asc Description: Digital signature
Re: [PATCH] udc: gadget: atmel_usba_udc: depend on COMMON_CLK_AT91
On Tue, Mar 10, 2015 at 10:21:22PM +0100, Alexandre Belloni wrote: > On 10/03/2015 at 15:53:12 -0500, Felipe Balbi wrote : > > On Tue, Mar 03, 2015 at 10:41:38AM +0100, Alexandre Belloni wrote: > > > On 03/03/2015 at 09:26:20 +0100, Boris Brezillon wrote : > > > > > config USB_ATMEL_USBA > > > > > tristate "Atmel USBA" > > > > > - depends on AVR32 || ARCH_AT91 > > > > > + depends on AVR32 || ARCH_AT91 && COMMON_CLK_AT91 > > > > > > > > I guess you should add parenthesis to make it clearer ? > > > > > > > > depends on AVR32 || (ARCH_AT91 && COMMON_CLK_AT91) > > > > > > > > And I wonder why you need that. I though this option was selected by all > > > > at91 platforms ? > > > > > > > > > > That is currently the case but maybe, one day, one of the AT91 platform > > > will not use the same clock driver. > > > > then, maybe, one day, you send this patch. > > Yeah, let's drop it for now but I have the feeling that this will > break (I actually broke it when switching at91 to multiplatform). aha, that changes it. So you already have something which makes this break ? Are you planning on sending that upstream any time soon ? We could very well use that same series to merge this patch. Only when it's needed ;-) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 2/3] usb: udc: add usb_udc_vbus_handler
hi,
On Fri, Mar 06, 2015 at 10:36:03AM +0800, Peter Chen wrote:
> @@ -145,6 +148,34 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
>
> /* -
> */
>
> +static void usb_udc_connect_control(struct usb_udc *udc)
> +{
> + if (udc->vbus)
> + usb_gadget_connect(udc->gadget);
> + else
> + usb_gadget_disconnect(udc->gadget);
> +}
> +
> +/**
> + * usb_udc_vbus_handler - updates the udc core vbus status, and try to
> + * connect or disconnect gadget
> + * @gadget: The gadget which vbus change occurs
> + * @status: The vbus status
> + *
> + * The udc driver calls it when it wants to connect or disconnect gadget
> + * according to vbus status.
> + */
> +void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
I have a feeling we need more bits for status here. If we ever want to
treat different VBUS levels (vbus valid, session valid, session end,
etc). But maybe we can just change this prototype when that's really
needed ?
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 3/3] usb: chipidea: udc: apply new usb_udc_vbus_handler interface
hi,
On Fri, Mar 06, 2015 at 10:36:04AM +0800, Peter Chen wrote:
> @@ -1574,13 +1574,12 @@ static int ci_udc_pullup(struct usb_gadget *_gadget,
> int is_on)
> {
> struct ci_hdrc *ci = container_of(_gadget, struct ci_hdrc, gadget);
>
> - if (!ci->vbus_active)
> - return -EOPNOTSUPP;
> -
> + pm_runtime_get_sync(&ci->gadget.dev);
> if (is_on)
> hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> else
> hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> + pm_runtime_put_sync(&ci->gadget.dev);
pm_runtime_* here look like they belong to a separate patch ?
--
balbi
signature.asc
Description: Digital signature
[PATCH] usb: phy: am335x-control: check return value of bus_find_device
From: David Dueck
This fixes a potential null pointer dereference.
Cc: # v3.16+
Fixes: d4332013919a ("driver core: dev_get_drvdata: Don't check for NULL dev")
Acked-by: Sebastian Andrzej Siewior
Signed-off-by: David Dueck
Signed-off-by: Felipe Balbi
---
drivers/usb/phy/phy-am335x-control.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/phy/phy-am335x-control.c
b/drivers/usb/phy/phy-am335x-control.c
index 403fab772724..7b3035ff9434 100644
--- a/drivers/usb/phy/phy-am335x-control.c
+++ b/drivers/usb/phy/phy-am335x-control.c
@@ -126,6 +126,9 @@ struct phy_control *am335x_get_phy_control(struct device
*dev)
return NULL;
dev = bus_find_device(&platform_bus_type, NULL, node, match);
+ if (!dev)
+ return NULL;
+
ctrl_usb = dev_get_drvdata(dev);
if (!ctrl_usb)
return NULL;
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/5] Add support for Fujitsu USB host controller
On Sun, Feb 22, 2015 at 12:25:35PM +0800, Sneeker Yeh wrote: > These patches add support for XHCI compliant Host controller found > on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/ > The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 > driver > and last four patch is about quirk implementation of errata in Synopsis > DesignWare USB3 IP. > > Patch 1 introduces a quirk with device disconnection management necessary > Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the > quirk, that host controller will die after a usb device is disconnected from > port of root hub. > > Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci > platform > data. > > Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3 > IP core driver. > > Patch 4 introduces using a quirk based on a errata of Synopsis > DesignWare USB3 IP which is versions < 3.00a and has hardware configuration > DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a > result this quirk has to be enabled via platform data or device tree. > > Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP > driver. > > Successfully tested on Fujitsu mb86s7x board. > > Changes since v4 (RFC): > [https://lkml.org/lkml/2015/2/17/13] > - based on Felipe's comment, rename dwc3-mb86s70.c to dwc3-fujitsu.c which is >more generic. > - based on Mathias's comment, remove unhelpful comment, and change the >function name of xhci_try_to_clear_csc() to xhci_late_csc_clear_quirk() >which is more appropriate. > > Changes since v3 (RFC): > [https://lkml.org/lkml/2015/1/25/8] > - based on Mathias's comment, fix bug and using xhci_port_state_to_neutral() >helper function to mask out some RW1C bits, prevent writing back all the >bits read from the PORTSC register. > > Changes since v2 (RFC): > [https://lkml.org/lkml/2015/1/19/55] > - based on Felipe's comment, re-order patches to avoid breaking > bisectability, > - based on Felipe's comment, add comment to structure's member, and sort it >alphabetically, > - based on Felipe's comment, add another v2.90 revision number in dwc3 IP. > > Changes since v1 (RFC): > [https://lkml.org/lkml/2014/12/15/929] > - based on Arnd's comment, remove entire unnecessary Fujitsu EHCI/OHCI glue, > - based on Felipe's comment, fix mis-using of runtime-pm API and setting dma >mask, remove unnecessary comment, and refactor suspend/resume handler in >Fujitsu Specific Glue layer in dwc3, > - based on Felipe's comment, add more commit log and comments in Synopsis >quirk implementation, and separate it into four patches. > > Sneeker Yeh (5): > xhci: add a quirk for device disconnection errata for Synopsis > Designware USB3 core > xhci: Platform: Set Synopsis device disconnection quirk based on > platform data > usb: dwc3: add revision number DWC3_REVISION_290A and > DWC3_REVISION_300A > usb: dwc3: Add quirk for Synopsis device disconnection errata > usb: dwc3: add Fujitsu Specific Glue layer Mathias, if you want me to take this series I need your Acked-by, otherwise I can give you mine, just let me know. -- balbi signature.asc Description: Digital signature
Re: [PATCH v3] usb: phy: phy-generic: No need to call gpiod_direction_output() twice
On Tue, Mar 10, 2015 at 06:22:17PM -0300, Fabio Estevam wrote:
> On Tue, Mar 10, 2015 at 6:20 PM, Felipe Balbi wrote:
> > On Tue, Feb 03, 2015 at 07:18:17PM -0200, Fabio Estevam wrote:
> >> From: Fabio Estevam
> >>
> >> Commit 9eb0797722895f4309b4 ("usb: phy: generic: fix the gpios to be
> >> optional")
> >> calls gpiod_direction_output() in the probe function, so there is no need
> >> to
> >> call it again, as we can simply call gpiod_set_value() directly.
> >>
> >> Also, in usb_gen_phy_shutdown() we can simply put the GPIO directly in its
> >> active level state and this allows us to simplify the nop_reset function to
> >> treat only the reset case.
> >>
> >> Signed-off-by: Fabio Estevam
> >
> > Is this for v4.1 or v4.0-rc ?
>
> For 4.1, thanks
in my testing/next now, thanks
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] udc: gadget: atmel_usba_udc: depend on COMMON_CLK_AT91
On Tue, Mar 10, 2015 at 10:41:09PM +0100, Alexandre Belloni wrote: > On 10/03/2015 at 16:23:53 -0500, Felipe Balbi wrote : > > > Yeah, let's drop it for now but I have the feeling that this will > > > break (I actually broke it when switching at91 to multiplatform). > > > > aha, that changes it. So you already have something which makes this > > break ? Are you planning on sending that upstream any time soon ? > > > > It has been sent but not merge and I need to send another version. > > > We could very well use that same series to merge this patch. Only when > > it's needed ;-) > > > > Like said, this is not an issue for now, I fixed it by simply having all > the AT91 platform selecting COMMON_CLK_AT91. I just have the feeling > that this is not quite future proof. > So this is not urgent at all and I'll try to remember to resend when > needed. fair enough :-) cheers -- balbi signature.asc Description: Digital signature
Re: USB ID detection interrupt - non OTG PHY
Hi Anjana, On Tue, Mar 03, 2015 at 09:27:09AM +0800, Peter Chen wrote: > On Mon, Mar 02, 2015 at 11:47:31AM +0530, Anjana V Kumar wrote: > > Hi Peter, > > > > I will need to configure both the phy and the controller. > > > > Thanks > > Anjana > > > > On Mon, Mar 2, 2015 at 11:24 AM, Peter Chen > > wrote: > > > > > > > > >> This is regarding the registering and handling of ID interrupt to swich > > >> between > > >> host and device mode. > > >> > > >> My aim is to have OTG functionality for a not-OTG phy. > > >> > > >> We wanted to register an ID interrupt which fires of change in ID value. > > >> Based > > >> on this ID value, we need to configure the phy to host or device mode. Do you really have a single phy for both modes or do you have a single port connected to different host and device phys? BR, David > > > > > > Do you really need to configure USB PHY (not usb controller) according to > > > host or device mode? > > > If it is, you need to handle your id switch routine at phy's driver, > > > since you need > > > to set PHY's register according to id value. > > > > > > Peter > > > > > It may be no easy to handle it if you can't handle all things > at controller driver, like handle id and vbus. Have a look > for below thread, some framework are not ready, like DRD library, > George, any updates for that? > > https://lkml.org/lkml/2014/12/23/442 > > -- > > Best Regards, > Peter Chen > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/15] musb: Add support for the Allwinner sunxi musb controller
Hi, On 03/10/2015 06:41 PM, Maxime Ripard wrote: On Mon, Mar 09, 2015 at 09:40:13PM +0100, Hans de Goede wrote: Hi All, This patch set has been a while in the making, so I'm very happy to present the end result here, and I hope everyone likes it. Before talking about merging this there are 2 things which I would like to point out: a) The musb controller in the sunxi SoCs uses some SRAM which needs to be mapped to the musb controller by poking some bits in the SRAM controller, just like the EMAC patches which were send a while back I've chosen to use syscon for this, actually 2 of the patches in this set come directly from the SRAM mapping patchset for the EMAC. I know that Maxime is not 100% in favor of using syscon: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/320221.html But I disagree with his arguments for writing a special driver for the SRAM controller: And I disagree with your disagreement :) 1) syscon was specifically designed for global system control registers like this and is fine to use as long as there are no conflicts where 1 bit is of interest to multiple drivers, and there is no such conflict here No. Syscon has been designed for being lazy. This is a huge abstraction non-sense. You have to put all the logic of dealing with some external register layout in clients drivers, including dealing with the different revisions/SoC that are different from that aspect, and duplicating that code across all client drivers. 2) Maxime's other arguments seem to boil down to it would be nice / prettier to have a specific driver for this, without really proving a hard need for such a driver. But writing such a driver is going to be a lot of work, and we've a ton of other work to do, and as said there is no real need for a separate driver, syscon works fine for this. Actually, I already wrote some prototype for this. I'll clean this up and send it tonight/tomorrow. Ok, lets see your prototype and then we'll go from there. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc2: Use platform endianness when accessing registers
Hello, On Tue, 10 Mar 2015 16:05:36 -0500, Felipe Balbi wrote: > I don't get it, why is it so that only mips needs this ? What's special > about mips' writel/readl implementation that it can't be used here ? > > This works for everybody else. Would it be a design oddity of this SoC that device registers do match CPU endianness, instead of matching implementations used in other SoCs ? This is the first time I ever get to debug some endianness issue in a driver, let alone on a SoC. I have extremely little knowledge of how this is expected to happen in general. Please pardon my naivety. AFAIK the Raspberry Pi is another SoC where this device is present, and the RPi is little endian. As this driver works there, it suggests the registers are little-endian too. I think proposed change will not break this, as discussed with John Youn on previous submission. I couldn't get a reasonable RPi kernel build environment (because of the lack of working kernel .config for a recent kernel) to verify this though. > Is it so that dwc2 and cpu endianness don't match here ? They do match actually. It's readl/writel internal byte swapping happening on big-endian archs which breaks this. Once straightened as per Antti's patch, the driver detects the HCD and USB devices work fine. Well, actually there is one out-of-driver (platform-specific) register to set so DMA transfers match CPU endianness (I have no idea why it doesn't default to the working setting). > weren't readl() and writel() already supposed to handle endianness for > us? If asm-generic implementation is representative of the intent, then the intent is clearly to read little-endian values from memory and convert to CPU endianness. http://lxr.free-electrons.com/source/include/asm-generic/io.h#L127 http://lxr.free-electrons.com/source/include/asm-generic/io.h#L161 Which brings me back to my original questions: Is SoC hardware expected to expose registers in cpu-consistent endianness (or maybe even "default-cpu-endianness") ? > wrong comment style. Expected style is to prefix lines with * and to end block below these, right ? Nice catch, thanks. Regards, -- Vincent Pelletier -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 5/5] usb: gadget: udc-core: independent registration of gadgets and gadget drivers
Hi Alan,
On Tue, Feb 17, 2015 at 11:51 PM, Alan Stern wrote:
> On Tue, 17 Feb 2015, Ruslan Bilovol wrote:
>
>> Change behavior during registration of gadgets and
>> gadget drivers in udc-core. Instead of previous
>> approach when for successful probe of usb gadget driver
>> at least one usb gadget should be already registered
>> use another one where gadget drivers and gadgets
>> can be registered in udc-core independently.
>>
>> Independent registration of gadgets and gadget drivers
>> is useful for built-in into kernel gadget and gadget
>> driver case - because it's possible that gadget is
>> really probed only on late_init stage (due to deferred
>> probe) whereas gadget driver's probe is silently failed
>> on module_init stage due to no any UDC added.
>>
>> Also it is useful for modules case - now there is no
>> difference what module to insert first: gadget module
>> or gadget driver one.
>>
>> Signed-off-by: Ruslan Bilovol
>> ---
>> drivers/usb/gadget/udc/udc-core.c | 51
>> ++-
>> include/linux/usb/gadget.h| 2 ++
>> 2 files changed, 42 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/udc-core.c
>> b/drivers/usb/gadget/udc/udc-core.c
>> index a960f3f..9e82497 100644
>> --- a/drivers/usb/gadget/udc/udc-core.c
>> +++ b/drivers/usb/gadget/udc/udc-core.c
>> @@ -48,8 +48,11 @@ struct usb_udc {
>>
>> static struct class *udc_class;
>> static LIST_HEAD(udc_list);
>> +static LIST_HEAD(gadget_driver_pending_list);
>> static DEFINE_MUTEX(udc_lock);
>>
>> +static int udc_bind_to_driver(struct usb_udc *udc,
>> + struct usb_gadget_driver *driver);
>
> Strange indentation, not at all like the other continuation lines in
> this source file.
>
> Also, there should be a blank line here, as in the original file.
Will fix it
>
>> /*
>> - */
>>
>> #ifdef CONFIG_HAS_DMA
>> @@ -243,6 +246,7 @@ int usb_add_gadget_udc_release(struct device *parent,
>> struct usb_gadget *gadget,
>> void (*release)(struct device *dev))
>> {
>> struct usb_udc *udc;
>> + struct usb_gadget_driver *pgadget;
>
> Don't call it "pgadget". None of the other pointer variables in this
> file have an extra "p" added to the beginnings of their names.
It seems this name is confusing (it is intended to have "pending
gadget" meaning). Will change it in next patch version
>
>> int ret = -ENOMEM;
>>
>> udc = kzalloc(sizeof(*udc), GFP_KERNEL);
>> @@ -288,6 +292,18 @@ int usb_add_gadget_udc_release(struct device *parent,
>> struct usb_gadget *gadget,
>>
>> usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
>>
>> + /* pick up one of pending gadget drivers */
>> + list_for_each_entry(pgadget, &gadget_driver_pending_list, pending) {
>> + if (!pgadget->udc_name || strcmp(pgadget->udc_name,
>> + dev_name(&udc->dev)) == 0) {
>> + ret = udc_bind_to_driver(udc, pgadget);
>
> Are you sure it's safe to call this routine while holding the udc_lock
> mutex?
Yes, it's safe here, in the only place where it was used before
this routine is also called while holding the udc_lock mutex
(see usb_gadget_probe_driver)
>
>> + if (ret)
>> + goto err4;
>> + list_del(&pgadget->pending);
>
> Use list_del_init().
>
>> + break;
>> + }
>> + }
>> +
>> mutex_unlock(&udc_lock);
>>
>> return 0;
>> @@ -364,10 +380,11 @@ found:
>> dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
>>
>> list_del(&udc->list);
>> - mutex_unlock(&udc_lock);
>> -
>> - if (udc->driver)
>> + if (udc->driver) {
>> + list_add(&udc->driver->pending, &gadget_driver_pending_list);
>> usb_gadget_remove_driver(udc);
>
> Are you sure it's safe to call this routine while holding the udc_lock
> mutex?
Should not be any issue here but it's possible to prevent this, will do it
in next patch version
>
>> + }
>> + mutex_unlock(&udc_lock);
>>
>> kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
>> flush_work(&gadget->work);
>> @@ -426,24 +443,26 @@ int usb_gadget_probe_driver(struct usb_gadget_driver
>> *driver)
>> if (!ret)
>> break;
>> }
>> - if (ret)
>> - ret = -ENODEV;
>> - else if (udc->driver)
>> - ret = -EBUSY;
>> - else
>> + if (!ret && udc->driver) {
>> + mutex_unlock(&udc_lock);
>> + return -EBUSY;
>
> This is a judgement call. It might be better to return 0 and add the
> gadget driver to the pending list.
Yes, that makes sense, will change it
>
>> + } else if (!ret) {
>>
Re: [PATCHv3 1/5] usb: gadget: bind UDC by name passed via usb_gadget_driver structure
Hi Sergei,
On Wed, Feb 18, 2015 at 2:05 PM, Sergei Shtylyov
wrote:
> Hello.
>
> On 2/18/2015 12:17 AM, Ruslan Bilovol wrote:
>
>> Introduce new 'udc_name' member to usb_gadget_driver structure.
>> The 'udc_name' is a name of UDC that usb_gadget_driver should
>> be bound to. If udc_name is NULL, it will be bound to any
>> available UDC.
>
>
>> Signed-off-by: Ruslan Bilovol
>> ---
>> drivers/usb/gadget/udc/udc-core.c | 25 -
>> include/linux/usb/gadget.h| 4
>> 2 files changed, 24 insertions(+), 5 deletions(-)
>
>
>> diff --git a/drivers/usb/gadget/udc/udc-core.c
>> b/drivers/usb/gadget/udc/udc-core.c
>> index 5a81cb0..e1079e08 100644
>> --- a/drivers/usb/gadget/udc/udc-core.c
>> +++ b/drivers/usb/gadget/udc/udc-core.c
>> @@ -440,21 +440,36 @@ EXPORT_SYMBOL_GPL(usb_udc_attach_driver);
>> int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
>> {
>> struct usb_udc *udc = NULL;
>> - int ret;
>> + int ret = -ENODEV;
>>
>> if (!driver || !driver->bind || !driver->setup)
>> return -EINVAL;
>>
>> mutex_lock(&udc_lock);
>> - list_for_each_entry(udc, &udc_list, list) {
>> - /* For now we take the first one */
>> - if (!udc->driver)
>> + if (driver->udc_name) {
>> + list_for_each_entry(udc, &udc_list, list) {
>> + ret = strcmp(driver->udc_name,
>> dev_name(&udc->dev));
>> + if (!ret)
>> + break;
>> + }
>> + if (ret)
>> + ret = -ENODEV;
>> + else if (udc->driver)
>> + ret = -EBUSY;
>> + else
>> goto found;
>> + } else {
>> + list_for_each_entry(udc, &udc_list, list) {
>> + /* For now we take the first one */
>> + if (!udc->driver)
>> + goto found;
>> + }
>> + ret = -ENODEV;
>
>
>Already assigned the same value by the initializer.
Thanks for pointing to this, will be fixed in next patch version
>
>> }
>>
>> pr_debug("couldn't find an available UDC\n");
>> mutex_unlock(&udc_lock);
>> - return -ENODEV;
>> + return ret;
>> found:
>> ret = udc_bind_to_driver(udc, driver);
>> mutex_unlock(&udc_lock);
>
> [...]
>
> WBR, Sergei
>
Best regards,
Ruslan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Control message failures kill entire XHCI stack
On 03/10/2015 11:10 PM, Devin Heitmueller wrote: Hello Baolu, I'm doing most of my testing on a 2014 Retina Macbook Pro. lsusb output below. I've also seen the issue on an Intel NUC D34010WYKH, which I don't have in front of me or I would provide the lsusb output for that as well. Devin, I'd like you to provide output of "lspci -xv". I want to know which xHC devices (PCI vendor and device ID) you are working with. Apologies to Mathias for not getting back to this sooner. I've been buried in another project but plan to get back to it this week. Thanks for investigating this issue. Cheers, Devin Bus 002 Device 002: ID 05ac:8406 Apple, Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x05ac Apple, Inc. idProduct 0x8406 bcdDevice8.20 iManufacturer 3 Apple iProduct4 Card Reader iSerial 5 0820 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 44 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 224mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 4 Binary Object Store Descriptor: bLength 5 bDescriptorType15 wTotalLength 22 bNumDeviceCaps 2 USB 2.0 Extension Device Capability: bLength 7 bDescriptorType16 bDevCapabilityType 2 bmAttributes 0x0002 Link Power Management (LPM) Supported SuperSpeed USB Device Capability: bLength10 bDescriptorType16 bDevCapabilityType 3 bmAttributes 0x02 Latency Tolerance Messages (LTM) Supported wSpeedsSupported 0x000e Device can operate at Full Speed (12Mbps) Device can operate at High Speed (480Mbps) Device can operate at SuperSpeed (5Gbps) bFunctionalitySupport 1 Lowest fully-functional device speed is Full Speed (12Mbps) bU1DevExitLat 10 micro seconds bU2DevExitLat2047 micro seconds Device Status: 0x0010 (Bus Powered) Latency Tolerance Messaging (LTM) Enabled Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass9 Hub bDeviceSubClass 0 Unused bDeviceProtocol 3 bMaxPacketSize0 9 idVendor 0x1d6b Linux Foundation idProduct 0x0003 3.0 root hub bcdDevice3.19 iManufacturer 3 Linux 3.19.0-rc5djh+ xhci-hcd iProduct2 xHCI Host Controller iSerial 1 :00:14.0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 31 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 9 Hub bInterfaceSubClass 0 Unused bInterfaceProtocol 0 Full speed (or root) hub iInterface 0 Endpoint Descriptor:
Re: [PATCHv3 5/5] usb: gadget: udc-core: independent registration of gadgets and gadget drivers
On Wed, 11 Mar 2015, Ruslan Bilovol wrote: > Hi Alan, Hello. > > If you add the list_init and list_del_init above, this loop won't be > > needed. You can just call list_del. > > I disagree with this. This function is externally visible and we can't > guarantee that some buggy code will not call it with uninitialized > 'pending' list_head. For example, if it never called usb_gadget_probe_driver() > but calls usb_gadget_unregister_driver(). > As per my opinion it's better to check it and return -ENODEV rather than > fail on deleting of uninitialized list_head. In this case adding the list_init > and list_del_init above is not needed. No, that is not the approach used in the rest of the kernel. We _want_ to know about bugs, so we can fix them. If you silently return -ENODEV then nobody will realize anything is wrong, but a big fat WARN or OOPS caused by an uninitialized list_head will draw people's attention very quickly. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: omap-usb2: Fix missing clk_prepare call when using old dt name
Current code does not call clk_prepare(phy->optclk) when using the old
usb_otg_ss_refclk960m name. Fix it.
Signed-off-by: Axel Lin
---
drivers/phy/phy-omap-usb2.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 6f4aef3..dcdd850 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -296,10 +296,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
dev_warn(&pdev->dev,
"found usb_otg_ss_refclk960m, please fix
DTS\n");
}
- } else {
- clk_prepare(phy->optclk);
}
+ if (!IS_ERR(phy->optclk))
+ clk_prepare(phy->optclk);
+
usb_add_phy_dev(&phy->phy);
return 0;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 5/5] usb: gadget: udc-core: independent registration of gadgets and gadget drivers
Hi,
On Wed, Mar 11, 2015 at 02:21:38AM +0200, Ruslan Bilovol wrote:
> >> @@ -469,6 +488,16 @@ int usb_gadget_unregister_driver(struct
> >> usb_gadget_driver *driver)
> >> break;
> >> }
> >>
> >> + if (ret) {
> >> + struct usb_gadget_driver *tmp;
> >> +
> >> + list_for_each_entry(tmp, &gadget_driver_pending_list,
> >> pending)
> >> + if (tmp == driver) {
> >> + list_del(&driver->pending);
> >> + ret = 0;
> >> + break;
> >> + }
> >> + }
> >
> > If you add the list_init and list_del_init above, this loop won't be
> > needed. You can just call list_del.
>
> I disagree with this. This function is externally visible and we can't
> guarantee that some buggy code will not call it with uninitialized
> 'pending' list_head. For example, if it never called usb_gadget_probe_driver()
> but calls usb_gadget_unregister_driver().
those cases deserve to suffer a really painfull and horrible death by
means of a kernel oops ;-) Sure, defensive programming and all, but at
some point we need to consider certain cases ridiculous and just not do
anything about them, so people know that they need more coffee and
attention while writing code.
--
balbi
signature.asc
Description: Digital signature
[PATCH v2 1/1] usb: gadget: lpc32xxx_udc: Fix NULL dereference
udc is then checked for NULL, if NULL, it is then dereferenced as
udc->dev, it is found using Coccinelle.
We simplify the code to fix this problem, and we delete some conditions
at if {} which will never be met.
Reported-by: Tapasweni Pathak
Reported-by : Julia Lawall
Signed-off-by: Peter Chen
---
drivers/usb/gadget/udc/lpc32xx_udc.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c
b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 27fd413..3b6a785 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1803,23 +1803,14 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep,
req = container_of(_req, struct lpc32xx_request, req);
ep = container_of(_ep, struct lpc32xx_ep, ep);
- if (!_req || !_req->complete || !_req->buf ||
+ if (!_ep || !_req || !_req->complete || !_req->buf ||
!list_empty(&req->queue))
return -EINVAL;
udc = ep->udc;
- if (!_ep) {
- dev_dbg(udc->dev, "invalid ep\n");
- return -EINVAL;
- }
-
-
- if ((!udc) || (!udc->driver) ||
- (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
- dev_dbg(udc->dev, "invalid device\n");
- return -EINVAL;
- }
+ if (udc->gadget.speed == USB_SPEED_UNKNOWN)
+ return -EPIPE;
if (ep->lep) {
struct lpc32xx_usbd_dd_gad *dd;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: pci: Add device mode to the dwc2-pci driver
The pci driver now registers a platform driver, like in dwc3, and lets
its probe function do all the initialization. This allows it to
account for changes to the platform driver that were not added to the
pci driver. Also future changes to the probe function don't have to be
duplicated. This also has the effect of adding device and DRD mode to
the pci driver. Tested on the Synopsys HAPS PCIe platform.
Signed-off-by: John Youn
---
drivers/usb/dwc2/Kconfig | 7 ++-
drivers/usb/dwc2/pci.c | 159 +--
2 files changed, 76 insertions(+), 90 deletions(-)
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index 76b9ba4..3db204f 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -59,11 +59,12 @@ config USB_DWC2_PLATFORM
config USB_DWC2_PCI
tristate "DWC2 PCI"
- depends on USB_DWC2_HOST && PCI
- default USB_DWC2_HOST
+ depends on PCI
+ default n
+ select USB_DWC2_PLATFORM
help
The Designware USB2.0 PCI interface module for controllers
- connected to a PCI bus. This is only used for host mode.
+ connected to a PCI bus.
config USB_DWC2_DEBUG
bool "Enable Debugging Messages"
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index 6646adb..ae41961 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -50,112 +50,97 @@
#include
#include
-
-#include "core.h"
-#include "hcd.h"
+#include
+#include
#define PCI_PRODUCT_ID_HAPS_HSOTG 0xabc0
-static const char dwc2_driver_name[] = "dwc2";
-
-static const struct dwc2_core_params dwc2_module_params = {
- .otg_cap= -1,
- .otg_ver= -1,
- .dma_enable = -1,
- .dma_desc_enable= 0,
- .speed = -1,
- .enable_dynamic_fifo= -1,
- .en_multiple_tx_fifo= -1,
- .host_rx_fifo_size = 1024,
- .host_nperio_tx_fifo_size = 256,
- .host_perio_tx_fifo_size= 1024,
- .max_transfer_size = 65535,
- .max_packet_count = 511,
- .host_channels = -1,
- .phy_type = -1,
- .phy_utmi_width = -1,
- .phy_ulpi_ddr = -1,
- .phy_ulpi_ext_vbus = -1,
- .i2c_enable = -1,
- .ulpi_fs_ls = -1,
- .host_support_fs_ls_low_power = -1,
- .host_ls_low_power_phy_clk = -1,
- .ts_dline = -1,
- .reload_ctl = -1,
- .ahbcfg = -1,
- .uframe_sched = -1,
+static const char dwc2_driver_name[] = "dwc2-pci";
+
+struct dwc2_pci_glue {
+ struct platform_device *dwc2;
+ struct platform_device *phy;
};
-/**
- * dwc2_driver_remove() - Called when the DWC_otg core is unregistered with the
- * DWC_otg driver
- *
- * @dev: Bus device
- *
- * This routine is called, for example, when the rmmod command is executed. The
- * device may or may not be electrically present. If it is present, the driver
- * stops device processing. Any resources used on behalf of this device are
- * freed.
- */
-static void dwc2_driver_remove(struct pci_dev *dev)
+static void dwc2_pci_remove(struct pci_dev *pci)
{
- struct dwc2_hsotg *hsotg = pci_get_drvdata(dev);
+ struct dwc2_pci_glue *glue = pci_get_drvdata(pci);
- dwc2_hcd_remove(hsotg);
- pci_disable_device(dev);
+ platform_device_unregister(glue->dwc2);
+ usb_phy_generic_unregister(glue->phy);
+ kfree(glue);
+ pci_set_drvdata(pci, NULL);
}
-/**
- * dwc2_driver_probe() - Called when the DWC_otg core is bound to the DWC_otg
- * driver
- *
- * @dev: Bus device
- *
- * This routine creates the driver components required to control the device
- * (core, HCD, and PCD) and initializes the device. The driver components are
- * stored in a dwc2_hsotg structure. A reference to the dwc2_hsotg is saved
- * in the device private data. This allows the driver to access the dwc2_hsotg
- * structure on subsequent calls to driver methods for this device.
- */
-static int dwc2_driver_probe(struct pci_dev *dev,
-const struct pci_device_id *id)
+static int dwc2_pci_probe(struct pci_dev *pci,
+ const struct pci_device_id *id)
{
- struct dwc2_hsotg *hsotg;
- int retval;
+ struct resource res[2];
+ struct platform_device *dwc2;
+ struct platform_device *phy;
+ int ret;
+ struct device *dev = &pci->dev;
+ struct dwc2_pci_glue*glue;
+
+ ret = pcim_enable_device(pci);
+ if (ret) {
+ dev_err(dev, "failed to enable pci device\n");
+ return -ENODEV;
+
Re: [PATCH 5/7] usb: chipidea: otg_fsm: add HNP polling support
On Mon, Mar 09, 2015 at 03:52:35PM +0800, Peter Chen wrote:
> On Mon, Mar 09, 2015 at 10:09:21AM +0800, Li Jun wrote:
> > From: Li Jun
> >
> > Add a dedicataed normal timer for HNP polling since it's cyclical, while
> > in peripheral mode, change a/b_bus_req to be 1 will make it response
> > to host request flag with 1, then role switch will be started.
>
> use a_bus_req/b_bus_req
>
Okay.
> Why hnp polling timer can't use chipidea fsm hrtimer list?
>
It can use hrtimer, I thought using hrtimer for HNP polling is some over-design,
I will change.
Li Jun
> >
> > Signed-off-by: Li Jun
> > ---
> > drivers/usb/chipidea/ci.h |4
> > drivers/usb/chipidea/otg_fsm.c | 49
> > +---
> > drivers/usb/chipidea/otg_fsm.h |2 ++
> > 3 files changed, 52 insertions(+), 3 deletions(-)
> >
> > + if (otg_hnp_polling(&ci->fsm) != HOST_REQUEST_FLAG) {
> > + pm_runtime_put_sync(ci->dev);
> > + return 0;
> > + }
> > + }
> > +
>
> When you do the polling device, it seems you disable the interrupt.
>
I will move it out of otg fsm queue work.
Li Jun
> > if (otg_statemachine(&ci->fsm)) {
> > if (ci->fsm.otg->state == OTG_STATE_A_IDLE) {
> > /*
> > @@ -817,4 +859,5 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
> > void ci_hdrc_otg_fsm_remove(struct ci_hdrc *ci)
> > {
> > sysfs_remove_group(&ci->dev->kobj, &inputs_attr_group);
> > + del_timer_sync(&ci->hnp_polling_timer);
> > }
> > diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
> > index 2689375..fbd6dc7 100644
> > --- a/drivers/usb/chipidea/otg_fsm.h
> > +++ b/drivers/usb/chipidea/otg_fsm.h
> > @@ -62,6 +62,8 @@
> > /* SSEND time before SRP */
> > #define TB_SSEND_SRP (1500)/* minimum 1.5 sec,
> > section:5.1.2 */
> >
> > +#define T_HOST_REQ_POLL (1500)/* HNP polling interval 1s~2s */
> > +
> > #ifdef CONFIG_USB_OTG_FSM
> >
> > int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
> > --
>
> --
>
> Best Regards,
> Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] usb: chipidea: otg_fsm: add HNP polling support
On Tue, Mar 10, 2015 at 03:46:31PM -0500, Felipe Balbi wrote: > On Mon, Mar 09, 2015 at 10:09:21AM +0800, Li Jun wrote: > > From: Li Jun > > > > Add a dedicataed normal timer for HNP polling since it's cyclical, while > > in peripheral mode, change a/b_bus_req to be 1 will make it response > > to host request flag with 1, then role switch will be started. > > > > Signed-off-by: Li Jun > > I don't get why you have so much code hidden away in a single > controller. You should really add this timer much more generically so > that there's no change needed on chipidea/musb/dwc2/dwc3/etc. > Accept, I will move those stuff out of controller driver and put it in common/usb-otg-fsm.c, then it can be used for different controllers w/o change needed. Li Jun > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: phy: Find the right match in devm_usb_phy_match
The res parameter passed to devm_usb_phy_match() is the location where the
pointer to the usb_phy is stored, hence it needs to be dereferenced before
comparing to the match data in order to find the correct match.
Signed-off-by: Axel Lin
---
drivers/usb/phy/phy.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 2f9735b..d1cd6b5 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -81,7 +81,9 @@ static void devm_usb_phy_release(struct device *dev, void
*res)
static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
{
- return res == match_data;
+ struct usb_phy **phy = res;
+
+ return *phy == match_data;
}
/**
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Samsung NP530U3C-A02CL] usb 1-2: device descriptor read/64, error
Hello Developers of kernel USB, My report: https://bugzilla.kernel.org/show_bug.cgi?id=94691 "Mount Samsung Galaxy Mini S4 + Cyagenmod 4.4.2" "dmesg: [ 92.854737] usb 1-2: device descriptor read/64, error -71 [ 93.126517] usb 1-2: device descriptor read/64, error -71 [ 93.342337] usb 1-2: new low-speed USB device number 8 using xhci_hcd [ 93.510224] usb 1-2: device descriptor read/64, error -71 [ 93.782012] usb 1-2: device descriptor read/64, error -71 [ 93.997737] usb 1-2: new low-speed USB device number 9 using xhci_hcd [ 93.998530] usb 1-2: Device not responding to setup address. [ 94.201669] usb 1-2: Device not responding to setup address. [ 94.405466] usb 1-2: device not accepting address 9, error -71" Thanks for you work, -- Cristian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: common: otg-fsm: only signal connect after switching to peripheral
On Tue, Mar 10, 2015 at 04:18:47PM -0500, Felipe Balbi wrote: > On Sun, Feb 15, 2015 at 03:39:48PM +0800, Peter Chen wrote: > > We should signal connect (pull up dp) after we have already > > at peripheral mode, otherwise, the dp may be toggled due to > > we reset controller or do disconnect during the initialization > > for peripheral, then, the host may be confused during the > > enumeration, eg, it finds the reset can't succeed, but the > > device is still there, see below error message. > > > > hub 1-0:1.0: USB hub found > > hub 1-0:1.0: 1 port detected > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: cannot reset port 1 (err = -32) > > hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? > > hub 1-0:1.0: unable to enumerate USB device on port 1 > > > > Signed-off-by: Peter Chen > > when was this introduced ? Do you want to Cc stable ? > When this fsm code was added at the first time, thanks, I will cc to stable at next version. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] usb: common: otg-fsm: only signal connect after switching to peripheral
We should signal connect (pull up dp) after we have already at peripheral mode, otherwise, the dp may be toggled due to we reset controller or do disconnect during the initialization for peripheral, then, the host may be confused during the enumeration, eg, it finds the reset can't succeed, but the device is still there, see below error message. hub 1-0:1.0: USB hub found hub 1-0:1.0: 1 port detected hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: cannot reset port 1 (err = -32) hub 1-0:1.0: Cannot enable port 1. Maybe the USB cable is bad? hub 1-0:1.0: unable to enumerate USB device on port 1 Cc: sta...@vger.kernel.org Signed-off-by: Peter Chen --- Changes for v2: - Add Cc tag for sta...@vger.kernel.org drivers/usb/common/usb-otg-fsm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c index c6b35b7..61d538a 100644 --- a/drivers/usb/common/usb-otg-fsm.c +++ b/drivers/usb/common/usb-otg-fsm.c @@ -150,9 +150,9 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) break; case OTG_STATE_B_PERIPHERAL: otg_chrg_vbus(fsm, 0); - otg_loc_conn(fsm, 1); otg_loc_sof(fsm, 0); otg_set_protocol(fsm, PROTO_GADGET); + otg_loc_conn(fsm, 1); break; case OTG_STATE_B_WAIT_ACON: otg_chrg_vbus(fsm, 0); @@ -213,10 +213,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state) break; case OTG_STATE_A_PERIPHERAL: - otg_loc_conn(fsm, 1); otg_loc_sof(fsm, 0); otg_set_protocol(fsm, PROTO_GADGET); otg_drv_vbus(fsm, 1); + otg_loc_conn(fsm, 1); otg_add_timer(fsm, A_BIDL_ADIS); break; case OTG_STATE_A_WAIT_VFALL: -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] usb: udc: add usb_udc_vbus_handler
On Tue, Mar 10, 2015 at 04:26:58PM -0500, Felipe Balbi wrote:
> hi,
>
> On Fri, Mar 06, 2015 at 10:36:03AM +0800, Peter Chen wrote:
> > @@ -145,6 +148,34 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
> >
> > /*
> > - */
> >
> > +static void usb_udc_connect_control(struct usb_udc *udc)
> > +{
> > + if (udc->vbus)
> > + usb_gadget_connect(udc->gadget);
> > + else
> > + usb_gadget_disconnect(udc->gadget);
> > +}
> > +
> > +/**
> > + * usb_udc_vbus_handler - updates the udc core vbus status, and try to
> > + * connect or disconnect gadget
> > + * @gadget: The gadget which vbus change occurs
> > + * @status: The vbus status
> > + *
> > + * The udc driver calls it when it wants to connect or disconnect gadget
> > + * according to vbus status.
> > + */
> > +void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
>
> I have a feeling we need more bits for status here. If we ever want to
> treat different VBUS levels (vbus valid, session valid, session end,
> etc). But maybe we can just change this prototype when that's really
> needed ?
>
Controller glue layer should decide it, isn't it? And it should be
decided at the driver initialization, and only one time. The host
only cares about CONNECT, and the CONNECT should be later than session
valid.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
