Re: USB gadgets with configfs hang reboot

2016-04-18 Thread Ivaylo Dimitrov



On  8.04.2016 23:13, Ivaylo Dimitrov wrote:

Hi,

On 16.01.2016 12:40, Ivaylo Dimitrov wrote:

Hi,

On 16.01.2016 00:48, Tony Lindgren wrote:

Hi all,

Looks like there's some issue with the USB gadgets and configfs.


...


(copied from "Re: [PATCH] usb: f_mass_storage: test whether thread is
running before starting another" thread)

Yet another problem with USB gadget, this time with f_acm - if there is
an open /dev/ttyGSn device, it is impossible to reboot/power down the
device.

My investigation shown that there is a process(pnatd) that opens
/dev/ttyGSn devices, so gserial_free_port() hangs on
wait_event(port->close_wait, gs_closed(port)); if I do "cd
/sys/bus/platform/drivers/musb-hdrc && echo musb-hdrc.0.auto > unbind".

Unfortunately I don't have serial debug port connector on my N900, so I
can't capture logs after the reboot command, however, I suspect it hangs
on the same place as with unbind.

That looks weird, as one would expect that close() is called when the
kernel kills user processes on reboot/powerdown.

Ivo



Anyone?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core

2016-04-18 Thread Peter Chen
On Tue, Apr 05, 2016 at 05:05:14PM +0300, Roger Quadros wrote:
> The OTG state machine needs a mechanism to start and
> stop the gadget controller. Add usb_gadget_start()
> and usb_gadget_stop().
> 
> Introduce usb_otg_add_gadget_udc() to allow controller drivers
> to register a gadget controller that is part of an OTG instance.
> 
> Register with OTG core when gadget function driver
> is available and unregister when function driver is unbound.
> 
> We need to unlock the usb_lock mutexbefore calling

...mutex before...

> usb_otg_register_gadget() in udc_bind_to_driver() and
> usb_gadget_remove_driver() else it will cause a circular
> locking dependency.
> 
> Ignore softconnect sysfs control when we're in OTG
> mode as OTG FSM takes care of gadget softconnect using
> the b_bus_req mechanism.
> 
> Signed-off-by: Roger Quadros 
> ---
>  drivers/usb/gadget/udc/udc-core.c | 166 
> +++---
>  include/linux/usb/gadget.h|   4 +
>  2 files changed, 161 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/udc-core.c 
> b/drivers/usb/gadget/udc/udc-core.c
> index 4151597..9b9702f 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -28,6 +28,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +#include 
> +#include 
>  
>  /**
>   * struct usb_udc - describes one usb device controller
> @@ -304,6 +308,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
>   */
>  static inline int usb_gadget_udc_start(struct usb_udc *udc)
>  {
> + dev_dbg(&udc->dev, "%s\n", __func__);
>   return udc->gadget->ops->udc_start(udc->gadget, udc->driver);
>  }

You may delete the debug message next time.

>  
> @@ -321,10 +326,81 @@ static inline int usb_gadget_udc_start(struct usb_udc 
> *udc)
>   */
>  static inline void usb_gadget_udc_stop(struct usb_udc *udc)
>  {
> + dev_dbg(&udc->dev, "%s\n", __func__);
>   udc->gadget->ops->udc_stop(udc->gadget);
>  }

The same.

>  
>  /**
> + * usb_gadget_start - start the usb gadget controller and connect to bus
> + * @gadget: the gadget device to start
> + *
> + * This is external API for use by OTG core.
> + *
> + * Start the usb device controller and connect to bus (enable pull).
> + */
> +static int usb_gadget_start(struct usb_gadget *gadget)
> +{
> + int ret;
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(&gadget->dev, "%s\n", __func__);

The same

> + mutex_lock(&udc_lock);
> + list_for_each_entry(udc, &udc_list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(&udc_lock);
> + return -EINVAL;
> +
> +found:
> + ret = usb_gadget_udc_start(udc);
> + if (ret)
> + dev_err(&udc->dev, "USB Device Controller didn't start: %d\n",
> + ret);
> + else
> + usb_udc_connect_control(udc);
> +
> + mutex_unlock(&udc_lock);
> +
> + return ret;
> +}
> +
> +/**
> + * usb_gadget_stop - disconnect from bus and stop the usb gadget
> + * @gadget: The gadget device we want to stop
> + *
> + * This is external API for use by OTG core.
> + *
> + * Disconnect from the bus (disable pull) and stop the
> + * gadget controller.
> + */
> +static int usb_gadget_stop(struct usb_gadget *gadget)
> +{
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(&gadget->dev, "%s\n", __func__);

The same

> + mutex_lock(&udc_lock);
> + list_for_each_entry(udc, &udc_list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(&udc_lock);
> + return -EINVAL;

The above finding gadget code is the same with usb_gadget_start, can we
use one common API to instead of it?

> +
> +found:
> + usb_gadget_disconnect(udc->gadget);
> + udc->driver->disconnect(udc->gadget);
> + usb_gadget_udc_stop(udc);
> + mutex_unlock(&udc_lock);
> +
> + return 0;
> +}
> +
> +/**
>   * usb_udc_release - release the usb_udc struct
>   * @dev: the dev member within usb_udc
>   *
> @@ -486,6 +562,48 @@ int usb_add_gadget_udc(struct device *parent, struct 
> usb_gadget *gadget)
>  }
>  EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
>  
> +/**
> + * usb_otg_add_gadget_udc - adds a new gadget to the udc class driver list
> + * @parent: the parent device to this udc. Usually the controller
> + * driver's device.
> + * @gadget: the gadget to be added to the list
> + * @otg_dev: the OTG controller device
> + *
> + * If otg_dev is NULL then device tree node is checked
> + * for OTG controller via the otg-controller property.
> + * Returns zero on success, negative errno otherwise.
> + */
> +int usb_otg_add_gadget_udc(struct device *parent, struct usb_gadget *gadget,
> +struct device *otg_dev)
> +{
> + if (!

Re: dwc3 initiated xhci probe problem in arm64 4.4 kernel due to DMA setup

2016-04-18 Thread Felipe Balbi

Hi,

Grygorii Strashko  writes:
> Felipe, Seems the problem might be deeper than on first look :(
>
> Lets see what grep says below. Even if we descope all SoC's drivers,
> there still will be few USB core components which manipulate with DMA 
> parameters:
>
> ---
> ./drivers/usb/core/hcd.c: hcd->self.uses_dma = (dev->dma_mask != NULL);
> usb_create_shared_hcd()
>   hcd->self.uses_dma = (dev->dma_mask != NULL);
>
> ---
> ./drivers/usb/core/usb.c: dev->dev.dma_mask = bus->controller->dma_mask;
> usb_alloc_dev()
>   dev->dev.dma_mask = bus->controller->dma_mask;
>
> ---
> ./drivers/usb/core/message.c: intf->dev.dma_mask = dev->dev.dma_mask;
> usb_set_configuration()
>   intf->dev.dma_mask = dev->dev.dma_mask;
>
> ^ why it is here is total mystery :(

interfaces are devices in their own right. A USB device might be
composite, meaning that it has several "devices" (functions or classes
might be a better wording) bundled into a single device/package.

> ---
> ./drivers/usb/dwc3/core.c:dev->dma_mask = dev->parent->dma_mask;
>
> ---
> ./drivers/usb/dwc2/hcd.c: /* Check if the bus driver or platform code has 
> setup a dma_mask */
> dwc2_hcd_init()
>   dma_set_mask(hsotg->dev, DMA_BIT_MASK(32)) < 0)
>
> ---
> ./drivers/usb/host/xhci.c:
> xhci_gen_setup()
>   !dma_set_mask(dev, DMA_BIT_MASK(64)
> ./drivers/usb/host/xhci-plat.c:   if (WARN_ON(!pdev->dev.dma_mask))
>
> ---
> ./drivers/usb/musb/musb_core.c:   if (use_dma && dev->dma_mask) {
>
> ---
> ./drivers/usb/storage/scsiglue.c: if 
> (!us->pusb_dev->bus->controller->dma_mask)
>
> It is big secret as for me (I'm USB noob:) which device is used for
> DMA transfers and when :(

heh, seems like we need to audit the usb subsystem (peripheral and host) :-)

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v6 11/12] usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable

2016-04-18 Thread Peter Chen
On Tue, Apr 05, 2016 at 05:05:16PM +0300, Roger Quadros wrote:
> This is the a_set_b_hnp_enable flag in the OTG state machine
> diagram and must be set when the A-Host has successfully set
> the b_hnp_enable feature of the OTG-B-Peripheral attached to it.
> 
> When this bit changes we kick our OTG FSM to make note of the
> change and act accordingly.

Since we have still not added fsm.a_set_b_hnp_en in OTG FSM, and this
patch set is mainly for DRD, would you please move out this patch from
this set?

Peter

> 
> Signed-off-by: Roger Quadros 
> ---
>  drivers/usb/core/hub.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 38cc4ba..27e3b4c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2273,6 +2273,7 @@ static int usb_enumerate_device_otg(struct usb_device 
> *udev)
>   && udev->parent == udev->bus->root_hub) {
>   struct usb_otg_descriptor   *desc = NULL;
>   struct usb_bus  *bus = udev->bus;
> + struct usb_hcd  *hcd = bus_to_hcd(bus);
>   unsignedport1 = udev->portnum;
>  
>   /* descriptor may appear anywhere in config */
> @@ -2302,6 +2303,9 @@ static int usb_enumerate_device_otg(struct usb_device 
> *udev)
>   dev_err(&udev->dev, "can't set HNP mode: %d\n",
>   err);
>   bus->b_hnp_enable = 0;
> + } else {
> + /* notify OTG fsm about a_set_b_hnp_enable */
> + usb_otg_kick_fsm(hcd->otg_dev);
>   }
>   } else if (desc->bLength == sizeof
>   (struct usb_otg_descriptor)) {
> @@ -2312,10 +2316,14 @@ static int usb_enumerate_device_otg(struct usb_device 
> *udev)
>   USB_DEVICE_A_ALT_HNP_SUPPORT,
>   0, NULL, 0,
>   USB_CTRL_SET_TIMEOUT);
> - if (err < 0)
> + if (err < 0) {
>   dev_err(&udev->dev,
>   "set a_alt_hnp_support failed: %d\n",
>   err);
> + } else {
> + /* notify OTG fsm about a_set_b_hnp_enable */
> + usb_otg_kick_fsm(hcd->otg_dev);
> + }
>   }
>   }
>  #endif
> @@ -4355,8 +4363,13 @@ hub_port_init(struct usb_hub *hub, struct usb_device 
> *udev, int port1,
>*/
>   if (!hdev->parent) {
>   delay = HUB_ROOT_RESET_TIME;
> - if (port1 == hdev->bus->otg_port)
> + if (port1 == hdev->bus->otg_port) {
>   hdev->bus->b_hnp_enable = 0;
> +#ifdef CONFIG_USB_OTG
> + /* notify OTG fsm about a_set_b_hnp_enable change */
> + usb_otg_kick_fsm(hcd->otg_dev);
> +#endif
> + }
>   }
>  
>   /* Some low speed devices have problems with the quick delay, so */
> -- 
> 2.5.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

-- 

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 v6 02/12] usb: hcd.h: Add OTG to HCD interface

2016-04-18 Thread Peter Chen
On Tue, Apr 05, 2016 at 05:05:07PM +0300, Roger Quadros wrote:
> The OTG core will use struct otg_hcd_ops to interface
> with the HCD controller.
> 
> The main purpose of this interface is to avoid directly
> calling HCD APIs from the OTG core as they
> wouldn't be defined in the built-in symbol table if
> CONFIG_USB is m.
> 
> Signed-off-by: Roger Quadros 
> ---
>  include/linux/usb/hcd.h | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index b98f831..861ccaa 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -399,6 +399,30 @@ struct hc_driver {
>  
>  };
>  
> +/**
> + * struct otg_hcd_ops - Interface between OTG core and HCD
> + *
> + * Provided by the HCD core to allow the OTG core to interface with the HCD
> + *
> + * @add: function to add the HCD
> + * @remove: function to remove the HCD
> + * @usb_bus_start_enum: function to immediately start bus enumeration
> + * @usb_control_msg: function to build and send of a control urb
> + * @usb_hub_find_child: function to get pointer to the child device
> + */
> +struct otg_hcd_ops {
> + int (*add)(struct usb_hcd *hcd,
> +unsigned int irqnum, unsigned long irqflags);
> + void (*remove)(struct usb_hcd *hcd);
> + int (*usb_bus_start_enum)(struct usb_bus *bus, unsigned int port_num);
> + int (*usb_control_msg)(struct usb_device *dev, unsigned int pipe,
> +__u8 request, __u8 requesttype, __u16 value,
> +__u16 index, void *data, __u16 size,
> +int timeout);
> + struct usb_device * (*usb_hub_find_child)(struct usb_device *hdev,
> +   int port1);
> +};
> +
>  static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd)
>  {
>   return hcd->driver->flags & HCD_BH;
> -- 
> 2.5.0
> 

Acked-by: Peter Chen 

-- 

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] cp210x: Add ID for Link ECU

2016-04-18 Thread Johan Hovold
On Mon, Apr 18, 2016 at 01:41:38PM +1000, Mike Manning wrote:
> Please find attached patch file for addition of Link G4 and Link G4+ Ecu's.

Thanks for the patch.

Try running your patch through scripts/checkpath.pl before submitting.
It would have let you know that the patch has some whitespace issues
(leading spaces instead of tabs).

Also make sure to send the patch inline rather than as an attachment.

git format-patch and git send-email are very convenient to get this
right.

> From a705ab11807ee600b9252f15681f64a346377629 Mon Sep 17 00:00:00 2001
> Subject: [PATCH] cp210x: Add ID for Link ECU
> From: Mike Manning 
> 
> The Link ECU is an aftermarket ECU computer for vehicles that provides full
>  tuning abilities as well as datalogging and displaying capabilities via
>  the USB to Serial adapter built into the device.
> Date: Mon, 18 Apr 2016 02:21:01 +

There shouldn't be a date field here.

> Signed-off-by: Michael Manning 

And this address looks wrong?

> ---
>  drivers/usb/serial/cp210x.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index bdc0f2f..7f45d00 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -140,6 +140,8 @@ static const struct usb_device_id id_table[] = {
> { USB_DEVICE(0x10C4, 0xF004) }, /* Elan Digital Systems USBcount50 */
> { USB_DEVICE(0x10C5, 0xEA61) }, /* Silicon Labs MobiData GPRS USB 
> Modem */
> { USB_DEVICE(0x10CE, 0xEA6A) }, /* Silicon Labs MobiData GPRS USB 
> Modem 100EU */
> +   { USB_DEVICE(0x12B8, 0xEC60) }, /* Link G4 ECU */
> +   { USB_DEVICE(0x12B8, 0xEC62) }, /* Link G4+ ECU */
> { USB_DEVICE(0x13AD, 0x) }, /* Baltech card reader */
> { USB_DEVICE(0x1555, 0x0004) }, /* Owen AC4 USB-RS485 Converter */
> { USB_DEVICE(0x166A, 0x0201) }, /* Clipsal 5500PACA C-Bus Pascal 
> Automation Controller */

Care to fix that up and resend?

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 03/12] usb: otg-fsm: use usb_otg wherever possible

2016-04-18 Thread Peter Chen
On Tue, Apr 05, 2016 at 05:05:08PM +0300, Roger Quadros wrote:
> Move otg_fsm into usb_otg and use usb_otg wherever possible
> in the usb_otg APIs.
> 
> Signed-off-by: Roger Quadros 

Acked-by: Peter Chen 
> ---
>  drivers/usb/chipidea/ci.h|   1 -
>  drivers/usb/chipidea/core.c  |  12 +--
>  drivers/usb/chipidea/debug.c |   2 +-
>  drivers/usb/chipidea/otg_fsm.c   | 171 ++---
>  drivers/usb/chipidea/udc.c   |  17 ++--
>  drivers/usb/common/usb-otg-fsm.c | 180 
> ---
>  drivers/usb/phy/phy-fsl-usb.c| 143 +++
>  drivers/usb/phy/phy-fsl-usb.h|   3 +-
>  include/linux/usb/otg-fsm.h  | 132 +++-
>  include/linux/usb/otg.h  | 107 +++
>  10 files changed, 384 insertions(+), 384 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index cd41455..c523975 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -209,7 +209,6 @@ struct ci_hdrc {
>   enum ci_rolerole;
>   boolis_otg;
>   struct usb_otg  otg;
> - struct otg_fsm  fsm;
>   struct hrtimer  otg_fsm_hrtimer;
>   ktime_t hr_timeouts[NUM_OTG_FSM_TIMERS];
>   unsignedenabled_otg_timer_bits;
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e6..a5570a9 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -1085,7 +1085,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
>  /* Prepare wakeup by SRP before suspend */
>  static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc *ci)
>  {
> - if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
> + if ((ci->otg.state == OTG_STATE_A_IDLE) &&
>   !hw_read_otgsc(ci, OTGSC_ID)) {
>   hw_write(ci, OP_PORTSC, PORTSC_W1C_BITS | PORTSC_PP,
>   PORTSC_PP);
> @@ -1097,13 +1097,13 @@ static void ci_otg_fsm_suspend_for_srp(struct ci_hdrc 
> *ci)
>  /* Handle SRP when wakeup by data pulse */
>  static void ci_otg_fsm_wakeup_by_srp(struct ci_hdrc *ci)
>  {
> - if ((ci->fsm.otg->state == OTG_STATE_A_IDLE) &&
> - (ci->fsm.a_bus_drop == 1) && (ci->fsm.a_bus_req == 0)) {
> + if ((ci->otg.state == OTG_STATE_A_IDLE) &&
> + (ci->otg.fsm.a_bus_drop == 1) && (ci->otg.fsm.a_bus_req == 0)) {
>   if (!hw_read_otgsc(ci, OTGSC_ID)) {
> - ci->fsm.a_srp_det = 1;
> - ci->fsm.a_bus_drop = 0;
> + ci->otg.fsm.a_srp_det = 1;
> + ci->otg.fsm.a_bus_drop = 0;
>   } else {
> - ci->fsm.id = 1;
> + ci->otg.fsm.id = 1;
>   }
>   ci_otg_queue_work(ci);
>   }
> diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
> index 6d23eed..374cdaa 100644
> --- a/drivers/usb/chipidea/debug.c
> +++ b/drivers/usb/chipidea/debug.c
> @@ -224,7 +224,7 @@ static int ci_otg_show(struct seq_file *s, void *unused)
>   if (!ci || !ci_otg_is_fsm_mode(ci))
>   return 0;
>  
> - fsm = &ci->fsm;
> + fsm = &ci->otg.fsm;
>  
>   /* -- State - */
>   seq_printf(s, "OTG state: %s\n\n",
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 5f169b3..f4e9fb5 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -40,7 +40,7 @@ get_a_bus_req(struct device *dev, struct device_attribute 
> *attr, char *buf)
>  
>   next = buf;
>   size = PAGE_SIZE;
> - t = scnprintf(next, size, "%d\n", ci->fsm.a_bus_req);
> + t = scnprintf(next, size, "%d\n", ci->otg.fsm.a_bus_req);
>   size -= t;
>   next += t;
>  
> @@ -56,25 +56,25 @@ set_a_bus_req(struct device *dev, struct device_attribute 
> *attr,
>   if (count > 2)
>   return -1;
>  
> - mutex_lock(&ci->fsm.lock);
> + mutex_lock(&ci->otg.fsm.lock);
>   if (buf[0] == '0') {
> - ci->fsm.a_bus_req = 0;
> + ci->otg.fsm.a_bus_req = 0;
>   } else if (buf[0] == '1') {
>   /* If a_bus_drop is TRUE, a_bus_req can't be set */
> - if (ci->fsm.a_bus_drop) {
> - mutex_unlock(&ci->fsm.lock);
> + if (ci->otg.fsm.a_bus_drop) {
> + mutex_unlock(&ci->otg.fsm.lock);
>   return count;
>   }
> - ci->fsm.a_bus_req = 1;
> - if (ci->fsm.otg->state == OTG_STATE_A_PERIPHERAL) {
> + ci->otg.fsm.a_bus_req = 1;
> + if (ci->otg.state == OTG_STATE_A_PERIPHERAL) {
>   ci->gadget.host_request_flag = 1;
> - mutex_unlo

[PATCH 0/5] usb: gadget: add new APIs of udc-core and modify renesas_usbhs for IPMMU

2016-04-18 Thread Yoshihiro Shimoda
This patch set is based on the latest Felipe's usb.git / testing/next
branch. (The commit id = 578bfe42fc37d588effe0d9fcd5e35e10d3d2e78)

Changes from RFC:
 - rebase the latest commit of Felipe's usb.git.
 - revise commit log in patch 3 and 4.

About the RFC patch set:
http://thread.gmane.org/gmane.linux.kernel.renesas-soc/2627

Yoshihiro Shimoda (5):
  usb: gadget: udc: core: Fix argument of dev_err() in
usb_gadget_map_request()
  usb: gadget: udc: core: add usb_gadget_{un}map_request_by_dev()
  usb: renesas_usbhs: change function call orfer in
usbhsf_dma_prepare_push()
  usb: renesas_usbhs: change arguments of dma_map_ctrl()
  usb: renesas_usbhs: use usb_gadget_{un}map_request_by_dev() for IPMMU

 drivers/usb/gadget/udc/udc-core.c  | 26 +++---
 drivers/usb/renesas_usbhs/fifo.c   | 16 +---
 drivers/usb/renesas_usbhs/mod_gadget.c |  9 -
 drivers/usb/renesas_usbhs/mod_host.c   |  3 ++-
 drivers/usb/renesas_usbhs/pipe.c   |  3 ++-
 drivers/usb/renesas_usbhs/pipe.h   |  6 --
 include/linux/usb/gadget.h |  4 
 7 files changed, 44 insertions(+), 23 deletions(-)

-- 
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 1/5] usb: gadget: udc: core: Fix argument of dev_err() in usb_gadget_map_request()

2016-04-18 Thread Yoshihiro Shimoda
The argument of dev_err() in usb_gadget_map_request() should be dev
instead of &gadget->dev.

Fixes: 7ace8fc ("usb: gadget: udc: core: Fix argument of dma_map_single for 
IOMMU")
Cc:  # v4.3+
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/udc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index e4e70e1..c6e7646 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -75,7 +75,7 @@ int usb_gadget_map_request(struct usb_gadget *gadget,
mapped = dma_map_sg(dev, req->sg, req->num_sgs,
is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
if (mapped == 0) {
-   dev_err(&gadget->dev, "failed to map SGs\n");
+   dev_err(dev, "failed to map SGs\n");
return -EFAULT;
}
 
-- 
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 2/5] usb: gadget: udc: core: add usb_gadget_{un}map_request_by_dev()

2016-04-18 Thread Yoshihiro Shimoda
If the following environment, the first argument of DMA API should
be set to a DMAC's device structure, not a udc controller's one.
 - A udc controller needs an external DMAC device (like a DMA Engine).
 - The external DMAC enables IOMMU.

So, this patch add usb_gadget_{un}map_request_by_dev() API to set
a DMAC's device structure by a udc controller driver.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/udc-core.c | 24 ++--
 include/linux/usb/gadget.h|  4 
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index c6e7646..6e8300d 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -61,11 +61,9 @@ static int udc_bind_to_driver(struct usb_udc *udc,
 
 #ifdef CONFIG_HAS_DMA
 
-int usb_gadget_map_request(struct usb_gadget *gadget,
+int usb_gadget_map_request_by_dev(struct device *dev,
struct usb_request *req, int is_in)
 {
-   struct device *dev = gadget->dev.parent;
-
if (req->length == 0)
return 0;
 
@@ -92,24 +90,38 @@ int usb_gadget_map_request(struct usb_gadget *gadget,
 
return 0;
 }
+EXPORT_SYMBOL_GPL(usb_gadget_map_request_by_dev);
+
+int usb_gadget_map_request(struct usb_gadget *gadget,
+   struct usb_request *req, int is_in)
+{
+   return usb_gadget_map_request_by_dev(gadget->dev.parent, req, is_in);
+}
 EXPORT_SYMBOL_GPL(usb_gadget_map_request);
 
-void usb_gadget_unmap_request(struct usb_gadget *gadget,
+void usb_gadget_unmap_request_by_dev(struct device *dev,
struct usb_request *req, int is_in)
 {
if (req->length == 0)
return;
 
if (req->num_mapped_sgs) {
-   dma_unmap_sg(gadget->dev.parent, req->sg, req->num_mapped_sgs,
+   dma_unmap_sg(dev, req->sg, req->num_mapped_sgs,
is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
 
req->num_mapped_sgs = 0;
} else {
-   dma_unmap_single(gadget->dev.parent, req->dma, req->length,
+   dma_unmap_single(dev, req->dma, req->length,
is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
}
 }
+EXPORT_SYMBOL_GPL(usb_gadget_unmap_request_by_dev);
+
+void usb_gadget_unmap_request(struct usb_gadget *gadget,
+   struct usb_request *req, int is_in)
+{
+   usb_gadget_unmap_request_by_dev(gadget->dev.parent, req, is_in);
+}
 EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
 
 #endif /* CONFIG_HAS_DMA */
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 5d4e151..457651b 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1223,9 +1223,13 @@ int usb_otg_descriptor_init(struct usb_gadget *gadget,
 
 /* utility to simplify map/unmap of usb_requests to/from DMA */
 
+extern int usb_gadget_map_request_by_dev(struct device *dev,
+   struct usb_request *req, int is_in);
 extern int usb_gadget_map_request(struct usb_gadget *gadget,
struct usb_request *req, int is_in);
 
+extern void usb_gadget_unmap_request_by_dev(struct device *dev,
+   struct usb_request *req, int is_in);
 extern void usb_gadget_unmap_request(struct usb_gadget *gadget,
struct usb_request *req, int is_in);
 
-- 
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 5/5] usb: renesas_usbhs: use usb_gadget_{un}map_request_by_dev() for IPMMU

2016-04-18 Thread Yoshihiro Shimoda
The previous code could use the first USB-DMAC with IPMMU if iommus
property was set into this device node. However, in this case, it
could not control the second USB-DMAC with IPMMU because a parameter
of IPMMU (micro-TLB id) is different with each USB-DMAC.

So, this patch uses the usb_gadget_{un}map_request_by_dev() APIs for
IPMMU. (Then, iommus property should be set into USB-DMAC node(s).)

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/mod_gadget.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c 
b/drivers/usb/renesas_usbhs/mod_gadget.c
index d701ae6..30345c2 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -197,8 +197,6 @@ static int usbhsg_dma_map_ctrl(struct device *dma_dev, 
struct usbhs_pkt *pkt,
struct usbhsg_request *ureq = usbhsg_pkt_to_ureq(pkt);
struct usb_request *req = &ureq->req;
struct usbhs_pipe *pipe = pkt->pipe;
-   struct usbhsg_uep *uep = usbhsg_pipe_to_uep(pipe);
-   struct usbhsg_gpriv *gpriv = usbhsg_uep_to_gpriv(uep);
enum dma_data_direction dir;
int ret = 0;
 
@@ -208,13 +206,13 @@ static int usbhsg_dma_map_ctrl(struct device *dma_dev, 
struct usbhs_pkt *pkt,
/* it can not use scatter/gather */
WARN_ON(req->num_sgs);
 
-   ret = usb_gadget_map_request(&gpriv->gadget, req, dir);
+   ret = usb_gadget_map_request_by_dev(dma_dev, req, dir);
if (ret < 0)
return ret;
 
pkt->dma = req->dma;
} else {
-   usb_gadget_unmap_request(&gpriv->gadget, req, dir);
+   usb_gadget_unmap_request_by_dev(dma_dev, req, dir);
}
 
return ret;
-- 
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 3/5] usb: renesas_usbhs: change function call orfer in usbhsf_dma_prepare_push()

2016-04-18 Thread Yoshihiro Shimoda
Since usbhsf_dma_{un}map() will use the "fifo" data in the near future,
this patch changes function call orfer in usbhsf_dma_prepare_push().

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/fifo.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 000f975..a805b23 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -881,12 +881,12 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
if (!fifo)
goto usbhsf_pio_prepare_push;
 
-   if (usbhsf_dma_map(pkt) < 0)
-   goto usbhsf_pio_prepare_push;
-
ret = usbhsf_fifo_select(pipe, fifo, 0);
if (ret < 0)
-   goto usbhsf_pio_prepare_push_unmap;
+   goto usbhsf_pio_prepare_push;
+
+   if (usbhsf_dma_map(pkt) < 0)
+   goto usbhsf_pio_prepare_push_unselect;
 
pkt->trans = len;
 
@@ -896,8 +896,8 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
 
return 0;
 
-usbhsf_pio_prepare_push_unmap:
-   usbhsf_dma_unmap(pkt);
+usbhsf_pio_prepare_push_unselect:
+   usbhsf_fifo_unselect(pipe, fifo);
 usbhsf_pio_prepare_push:
/*
 * change handler to PIO
-- 
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 4/5] usb: renesas_usbhs: change arguments of dma_map_ctrl()

2016-04-18 Thread Yoshihiro Shimoda
Since usbhsg_dma_map_ctrl() needs DMA device structure in the near future,
this patch changes arguments of dma_map_ctrl() to give such data.
(This patch is only change the argument.)

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/renesas_usbhs/fifo.c   | 4 +++-
 drivers/usb/renesas_usbhs/mod_gadget.c | 3 ++-
 drivers/usb/renesas_usbhs/mod_host.c   | 3 ++-
 drivers/usb/renesas_usbhs/pipe.c   | 3 ++-
 drivers/usb/renesas_usbhs/pipe.h   | 6 --
 5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index a805b23..7be4e7d 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -799,8 +799,10 @@ static int __usbhsf_dma_map_ctrl(struct usbhs_pkt *pkt, 
int map)
struct usbhs_pipe *pipe = pkt->pipe;
struct usbhs_priv *priv = usbhs_pipe_to_priv(pipe);
struct usbhs_pipe_info *info = usbhs_priv_to_pipeinfo(priv);
+   struct usbhs_fifo *fifo = usbhs_pipe_to_fifo(pipe);
+   struct dma_chan *chan = usbhsf_dma_chan_get(fifo, pkt);
 
-   return info->dma_map_ctrl(pkt, map);
+   return info->dma_map_ctrl(chan->device->dev, pkt, map);
 }
 
 static void usbhsf_dma_complete(void *arg);
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c 
b/drivers/usb/renesas_usbhs/mod_gadget.c
index 53d104b..d701ae6 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -191,7 +191,8 @@ static void usbhsg_queue_push(struct usbhsg_uep *uep,
 /*
  * dma map/unmap
  */
-static int usbhsg_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
+static int usbhsg_dma_map_ctrl(struct device *dma_dev, struct usbhs_pkt *pkt,
+  int map)
 {
struct usbhsg_request *ureq = usbhsg_pkt_to_ureq(pkt);
struct usb_request *req = &ureq->req;
diff --git a/drivers/usb/renesas_usbhs/mod_host.c 
b/drivers/usb/renesas_usbhs/mod_host.c
index 1a8e4c4..3bf0b72 100644
--- a/drivers/usb/renesas_usbhs/mod_host.c
+++ b/drivers/usb/renesas_usbhs/mod_host.c
@@ -929,7 +929,8 @@ static int usbhsh_dcp_queue_push(struct usb_hcd *hcd,
 /*
  * dma map functions
  */
-static int usbhsh_dma_map_ctrl(struct usbhs_pkt *pkt, int map)
+static int usbhsh_dma_map_ctrl(struct device *dma_dev, struct usbhs_pkt *pkt,
+  int map)
 {
if (map) {
struct usbhsh_request *ureq = usbhsh_pkt_to_ureq(pkt);
diff --git a/drivers/usb/renesas_usbhs/pipe.c b/drivers/usb/renesas_usbhs/pipe.c
index 78e9dba..77b615c 100644
--- a/drivers/usb/renesas_usbhs/pipe.c
+++ b/drivers/usb/renesas_usbhs/pipe.c
@@ -655,7 +655,8 @@ static void usbhsp_put_pipe(struct usbhs_pipe *pipe)
 }
 
 void usbhs_pipe_init(struct usbhs_priv *priv,
-int (*dma_map_ctrl)(struct usbhs_pkt *pkt, int map))
+int (*dma_map_ctrl)(struct device *dma_dev,
+struct usbhs_pkt *pkt, int map))
 {
struct usbhs_pipe_info *info = usbhs_priv_to_pipeinfo(priv);
struct usbhs_pipe *pipe;
diff --git a/drivers/usb/renesas_usbhs/pipe.h b/drivers/usb/renesas_usbhs/pipe.h
index 7835747..95185fd 100644
--- a/drivers/usb/renesas_usbhs/pipe.h
+++ b/drivers/usb/renesas_usbhs/pipe.h
@@ -47,7 +47,8 @@ struct usbhs_pipe_info {
struct usbhs_pipe *pipe;
int size;   /* array size of "pipe" */
 
-   int (*dma_map_ctrl)(struct usbhs_pkt *pkt, int map);
+   int (*dma_map_ctrl)(struct device *dma_dev, struct usbhs_pkt *pkt,
+   int map);
 };
 
 /*
@@ -84,7 +85,8 @@ int usbhs_pipe_is_running(struct usbhs_pipe *pipe);
 void usbhs_pipe_running(struct usbhs_pipe *pipe, int running);
 
 void usbhs_pipe_init(struct usbhs_priv *priv,
-int (*dma_map_ctrl)(struct usbhs_pkt *pkt, int map));
+int (*dma_map_ctrl)(struct device *dma_dev,
+struct usbhs_pkt *pkt, int map));
 int usbhs_pipe_get_maxpacket(struct usbhs_pipe *pipe);
 void usbhs_pipe_clear(struct usbhs_pipe *pipe);
 int usbhs_pipe_is_accessible(struct usbhs_pipe *pipe);
-- 
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: USB gadgets with configfs hang reboot

2016-04-18 Thread Felipe Balbi

Hi,

Ivaylo Dimitrov  writes:
> Hi,
>
> On 16.01.2016 12:40, Ivaylo Dimitrov wrote:
>> Hi,
>>
>> On 16.01.2016 00:48, Tony Lindgren wrote:
>>> Hi all,
>>>
>>> Looks like there's some issue with the USB gadgets and configfs.
>>>
>>> I'm seeing rmmod of the UDC driver cause a warning and then reboot
>>> hangs the system. This happens at least with v4.4, and I've reproduced
>>> it with dwc3 and musb so it seems to be generic.
>>>
>>
>> Having configfs is not needed, disabling usb gadgets (#
>> CONFIG_USB_MUSB_GADGET is not set) seems to solved at least poweroff
>> hang issue on N900. Also, g_nokia is not a module in the config I use,
>> so I guess the problem is not related whether gadgets are modular or
>> not. Unfortunately I was not able to test reboot, as rootfs became
>> corrupted after the first poweroff :( . So it looks like my theory that
>> onenand corruption on N900 is because poweroff/reboot hangs is wrong.
>
> (copied from "Re: [PATCH] usb: f_mass_storage: test whether thread is 
> running before starting another" thread)
>
> Yet another problem with USB gadget, this time with f_acm - if there is 
> an open /dev/ttyGSn device, it is impossible to reboot/power down the 
> device.
>
> My investigation shown that there is a process(pnatd) that opens 
> /dev/ttyGSn devices, so gserial_free_port() hangs on 
> wait_event(port->close_wait, gs_closed(port)); if I do "cd 
> /sys/bus/platform/drivers/musb-hdrc && echo musb-hdrc.0.auto > unbind".
>
> Unfortunately I don't have serial debug port connector on my N900, so I 
> can't capture logs after the reboot command, however, I suspect it hangs 
> on the same place as with unbind.
>
> That looks weird, as one would expect that close() is called when the 
> kernel kills user processes on reboot/powerdown.

right, care to enable lockup detection and see if you can get more info
out of it ?

-- 
balbi


signature.asc
Description: PGP signature


RE: [PATCH v2 0/2] usb: dwc3: add disable receiver detection in P3 quirk

2016-04-18 Thread Felipe Balbi

Hi,

Rajesh Bhagat  writes:
>>-Original Message-
>>From: Rajesh Bhagat [mailto:rajesh.bha...@nxp.com]
>>Sent: Monday, March 14, 2016 2:41 PM
>>To: ba...@ti.com
>>Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
>>ker...@vger.kernel.org; linux-o...@vger.kernel.org; Sriram Dash
>>; Rajesh Bhagat 
>>Subject: [PATCH v2 0/2] usb: dwc3: add disable receiver detection in P3
>>quirk
>>
>>Adds disable receiver detection in P3 quirk in DWC3 driver.
>>
>>Rajesh Bhagat (2):
>>  usb: dwc3: add disable receiver detection in P3 quirk
>>  Documentation: dt: dwc3: Add snps,dis_rxdet_inp3_quirk property
>>
>> Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
>> drivers/usb/dwc3/core.c| 6 ++
>> drivers/usb/dwc3/core.h| 2 ++
>> drivers/usb/dwc3/platform_data.h   | 1 +
>> 4 files changed, 11 insertions(+)
>>
>
> Hello Felipe,
>
> Any comments on above [v2] patches?
>

https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=ddf332eb6a5379c3f4bcade6760ae41bcd8f2202

https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=bdeb03b4ed887a36595fc8ccdd04f82e718e9ab4

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCHv4 2/3] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

2016-04-18 Thread Felipe Balbi

Hi,

Alan Stern  writes:
> On Fri, 15 Apr 2016, Felipe Balbi wrote:
>
>> Alan Stern  writes:
>> > On Thu, 14 Apr 2016, Felipe Balbi wrote:
>> >
>> >> >> --- a/drivers/usb/storage/scsiglue.c
>> >> >> +++ b/drivers/usb/storage/scsiglue.c
>> >> >> @@ -127,6 +127,11 @@ static int slave_configure(struct scsi_device 
>> >> >> *sdev)
>> >> >>if (queue_max_hw_sectors(sdev->request_queue) > 
>> >> >> max_sectors)
>> >> >>blk_queue_max_hw_sectors(sdev->request_queue,
>> >> >>  max_sectors);
>> >> >> +  } else if (us->pusb_dev->speed >= USB_SPEED_SUPER) {
>> >> >> +  /* USB3 devices will be limited to 2048 sectors. This 
>> >> >> gives us
>> >> >> +   * better throughput on most devices.
>> >> >> +   */
>> >> >> +  blk_queue_max_hw_sectors(sdev->request_queue, 2048);
>> >> >>} else if (sdev->type == TYPE_TAPE) {
>> >> >>/* Tapes need much higher max_sector limits, so just
>> >> >> * raise it to the maximum possible (4 GB / 512) and
>> >> >
>> >> > Argh!  This has the same kind of problem as before.  What will happen
>> >> > when somebody has a USB-3 tape drive?
>> >> 
>> >> I didn't know that was even plausible :-) Anyway, I'll update, but while
>> >> at that, so I use for bcdUSB instead of speed as Oliver suggested ? I
>> >> mean, a USB3 stick running on high-speed can also support 2048 max
>> >> sectors, right ?
>> >> 
>> >> let me know
>> >
>> > To tell the truth, I have no idea.  There probably aren't enough USB-3 
>> > products in existence yet to tell -- not to mention that with the 
>> > existing code, we wouldn't detect any exceptions.
>> >
>> > It sounds reasonable...  But won't a USB-3 device running at high speed
>> > provide a device descriptor that has bcdUSB set to 0x0210?  (See the
>> > second paragraph of section 9.6.1 in the USB-3.1 spec.)
>> 
>> right, but an HS USB 2.1 device is also recent enough that it's likely
>> to work similarly, no ?
>
> Again, I don't know.  But in my experience, relying on USB device
> designers to do things correctly tends not to work out.  :-(
>
> Besides, whether something works right has less to do with how recent 
> it is and more to do with whether or not it was ever tested.  If 
> Windows doesn't transfer more than 240 sectors at a time at high speed 
> then we probably shouldn't either.

fair enough, I'll send v5 shortly.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v6 04/12] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops

2016-04-18 Thread Peter Chen
On Tue, Apr 05, 2016 at 05:05:09PM +0300, Roger Quadros wrote:
> This is to prevent missing symbol build error if OTG is
> enabled (built-in) and HCD core (CONFIG_USB) is module.
> 
> Signed-off-by: Roger Quadros 

Acked-by: Peter Chen 

> ---
>  drivers/usb/chipidea/otg_fsm.c   |  7 +++
>  drivers/usb/common/usb-otg-fsm.c | 15 +++
>  drivers/usb/phy/phy-fsl-usb.c|  7 +++
>  include/linux/usb/otg.h  |  2 ++
>  4 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index f4e9fb5..5fdf8ca 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -582,6 +582,12 @@ static struct otg_fsm_ops ci_otg_ops = {
>   .start_gadget = ci_otg_start_gadget,
>  };
>  
> +static struct otg_hcd_ops ci_hcd_ops = {
> + .usb_bus_start_enum = usb_bus_start_enum,
> + .usb_control_msg = usb_control_msg,
> + .usb_hub_find_child = usb_hub_find_child,
> +};
> +
>  int ci_otg_fsm_work(struct ci_hdrc *ci)
>  {
>   /*
> @@ -805,6 +811,7 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
>   ci->otg.state = OTG_STATE_UNDEFINED;
>   ci->otg.fsm.ops = &ci_otg_ops;
>   ci->otg.fsm.dev = ci->dev;
> + ci->otg.hcd_ops = &ci_hcd_ops;
>   ci->gadget.hnp_polling_support = 1;
>   ci->otg.fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
>   if (!ci->otg.fsm.host_req_flag)
> diff --git a/drivers/usb/common/usb-otg-fsm.c 
> b/drivers/usb/common/usb-otg-fsm.c
> index abc462c..2cb4aed 100644
> --- a/drivers/usb/common/usb-otg-fsm.c
> +++ b/drivers/usb/common/usb-otg-fsm.c
> @@ -135,11 +135,16 @@ static void otg_hnp_polling_work(struct work_struct 
> *work)
>   enum usb_otg_state state = otg->state;
>   u8 flag;
>   int retval;
> + struct otg_hcd_ops *hcd_ops = otg->hcd_ops;
>  
>   if (state != OTG_STATE_A_HOST && state != OTG_STATE_B_HOST)
>   return;
>  
> - udev = usb_hub_find_child(otg->host->root_hub, 1);
> + if (!hcd_ops || !hcd_ops->usb_control_msg ||
> + !hcd_ops->usb_hub_find_child)
> + return;
> +
> + udev = hcd_ops->usb_hub_find_child(otg->host->root_hub, 1);
>   if (!udev) {
>   dev_err(otg->host->controller,
>   "no usb dev connected, can't start HNP polling\n");
> @@ -148,7 +153,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
>  
>   *fsm->host_req_flag = 0;
>   /* Get host request flag from connected USB device */
> - retval = usb_control_msg(udev,
> + retval = hcd_ops->usb_control_msg(udev,
>   usb_rcvctrlpipe(udev, 0),
>   USB_REQ_GET_STATUS,
>   USB_DIR_IN | USB_RECIP_DEVICE,
> @@ -177,7 +182,7 @@ static void otg_hnp_polling_work(struct work_struct *work)
>   if (state == OTG_STATE_A_HOST) {
>   /* Set b_hnp_enable */
>   if (!otg->host->b_hnp_enable) {
> - retval = usb_control_msg(udev,
> + retval = hcd_ops->usb_control_msg(udev,
>   usb_sndctrlpipe(udev, 0),
>   USB_REQ_SET_FEATURE, 0,
>   USB_DEVICE_B_HNP_ENABLE,
> @@ -256,7 +261,9 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
> usb_otg_state new_state)
>   otg_loc_conn(otg, 0);
>   otg_loc_sof(otg, 1);
>   otg_set_protocol(fsm, PROTO_HOST);
> - usb_bus_start_enum(otg->host, otg->host->otg_port);
> + if (otg->hcd_ops && otg->hcd_ops->usb_bus_start_enum)
> + otg->hcd_ops->usb_bus_start_enum(otg->host,
> +  otg->host->otg_port);
>   otg_start_hnp_polling(fsm);
>   break;
>   case OTG_STATE_A_IDLE:
> diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
> index a18a2ee..39605d5 100644
> --- a/drivers/usb/phy/phy-fsl-usb.c
> +++ b/drivers/usb/phy/phy-fsl-usb.c
> @@ -792,6 +792,12 @@ static struct otg_fsm_ops fsl_otg_ops = {
>   .start_gadget = fsl_otg_start_gadget,
>  };
>  
> +static struct otg_hcd_ops fsl_hcd_ops = {
> + .usb_bus_start_enum = usb_bus_start_enum,
> + .usb_control_msg = usb_control_msg,
> + .usb_hub_find_child = usb_hub_find_child,
> +};
> +
>  /* Initialize the global variable fsl_otg_dev and request IRQ for OTG */
>  static int fsl_otg_conf(struct platform_device *pdev)
>  {
> @@ -821,6 +827,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
>   /* Set OTG state machine operations */
>   fsl_otg_tc->otg.fsm.ops = &fsl_otg_ops;
>   fsl_otg_tc->otg.fsm.dev = &pdev->dev;
> + fsl_otg_tc->otg.hcd_ops = &fsl_hcd_ops;
>  
>   /* initialize the otg structure */
>   fsl_otg_tc->phy.label = DRIVER_DESC;
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.

Re: [PATCH v6 06/12] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG

2016-04-18 Thread Peter Chen
On Tue, Apr 05, 2016 at 05:05:11PM +0300, Roger Quadros wrote:
> Let's use CONFIG_USB_OTG as a single config option to enable
> USB OTG and the OTG FSM. This makes things a lot less confusing.
> 
> Update all users of CONFIG_USB_OTG_FSM to CONFIG_USB_OTG.
> 
> Signed-off-by: Roger Quadros 
> ---
>  Documentation/usb/chipidea.txt | 2 +-
>  drivers/usb/Makefile   | 1 +
>  drivers/usb/chipidea/Makefile  | 2 +-
>  drivers/usb/chipidea/ci.h  | 2 +-
>  drivers/usb/chipidea/otg_fsm.h | 2 +-
>  drivers/usb/common/Makefile| 3 ++-
>  drivers/usb/core/Kconfig   | 8 
>  drivers/usb/phy/Kconfig| 2 +-
>  8 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
> index 678741b..3b1f263 100644
> --- a/Documentation/usb/chipidea.txt
> +++ b/Documentation/usb/chipidea.txt
> @@ -5,7 +5,7 @@ with 2 Freescale i.MX6Q sabre SD boards.
>  
>  1.1 How to enable OTG FSM in menuconfig
>  ---
> -Select CONFIG_USB_OTG_FSM, rebuild kernel Image and modules.
> +Select CONFIG_USB_OTG, rebuild kernel Image and modules.
>  If you want to check some internal variables for otg fsm,
>  mount debugfs, there are 2 files which can show otg fsm
>  variables and some controller registers value:
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index dca7856..16a5b55 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/
>  obj-$(CONFIG_USB_GADGET) += gadget/
>  
>  obj-$(CONFIG_USB_COMMON) += common/
> +obj-$(CONFIG_USB_OTG)+= common/

Why we need to add above line?

Peter
>  
>  obj-$(CONFIG_USBIP_CORE) += usbip/
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 518e445..45aa24d 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -3,7 +3,7 @@ obj-$(CONFIG_USB_CHIPIDEA)+= ci_hdrc.o
>  ci_hdrc-y:= core.o otg.o debug.o
>  ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC)   += udc.o
>  ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)  += host.o
> -ci_hdrc-$(CONFIG_USB_OTG_FSM)+= otg_fsm.o
> +ci_hdrc-$(CONFIG_USB_OTG)+= otg_fsm.o
>  
>  # Glue/Bridge layers go here
>  
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index c523975..1a32b8c 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -406,7 +406,7 @@ static inline u32 hw_test_and_write(struct ci_hdrc *ci, 
> enum ci_hw_regs reg,
>   */
>  static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
>  {
> -#ifdef CONFIG_USB_OTG_FSM
> +#ifdef CONFIG_USB_OTG
>   struct usb_otg_caps *otg_caps = &ci->platdata->ci_otg_caps;
>  
>   return ci->is_otg && ci->roles[CI_ROLE_HOST] &&
> diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
> index 6366fe3..2d451bb 100644
> --- a/drivers/usb/chipidea/otg_fsm.h
> +++ b/drivers/usb/chipidea/otg_fsm.h
> @@ -64,7 +64,7 @@
>  
>  #define TB_AIDL_BDIS (20)/* 4ms ~ 150ms, section 5.2.1 */
>  
> -#if IS_ENABLED(CONFIG_USB_OTG_FSM)
> +#if IS_ENABLED(CONFIG_USB_OTG)
>  
>  int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
>  int ci_otg_fsm_work(struct ci_hdrc *ci);
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 6bbb3ec..f8f2c88 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -6,5 +6,6 @@ obj-$(CONFIG_USB_COMMON)+= usb-common.o
>  usb-common-y   += common.o
>  usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  
> -obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>  obj-$(CONFIG_USB_ULPI_BUS)   += ulpi.o
> +usbotg-y := usb-otg-fsm.o
> +obj-$(CONFIG_USB_OTG)+= usbotg.o
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index dd28010..ae228d0 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -75,14 +75,6 @@ config USB_OTG_BLACKLIST_HUB
> and software costs by not supporting external hubs.  So
> are "Embedded Hosts" that don't offer OTG support.
>  
> -config USB_OTG_FSM
> - tristate "USB 2.0 OTG FSM implementation"
> - depends on USB && USB_OTG
> - select USB_PHY
> - help
> -   Implements OTG Finite State Machine as specified in On-The-Go
> -   and Embedded Host Supplement to the USB Revision 2.0 Specification.
> -
>  config USB_ULPI_BUS
>   tristate "USB ULPI PHY interface support"
>   depends on USB_SUPPORT
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index c690474..06794e2 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -20,7 +20,7 @@ config AB8500_USB
>  
>  config FSL_USB2_OTG
>   bool "Freescale USB OTG Transceiver Driver"
> - depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG_FSM && PM
> + depends on USB_EHCI_FSL && USB_FSL_USB2 && USB_OTG && PM
>

[PATCH 0/5] usb: chipidea: Add usb charger detection support

2016-04-18 Thread Li Jun
Some usb module soc implementations can support usb charger type
detection, which can be used by usb charger framework to control
the charging current. user can define the charge detection api of the
platform data if want to enable it.
There are 2 api and one flag used for it, if the usb hardware can
automatically complete the detection process, only platdata->usb_charger_det
is required.
Some imx platform need software implement the detection process, and
need pull up and down DP line before and after secondary detection
so platdata->usb_charger_secondary_det is also required and need set
pull_dp_for_charger flag, tested with i.mx7d, support for i.mx6 will
be added later.

This patch set is based on Baolin's usb charger framework:
http://comments.gmane.org/gmane.linux.power-management.general/74394

Li Jun (5):
  usb: chipidea: add usb charger detection support
  usb: chipidea: imx: add usb charger dectection for imx platforms
  usb: chipidea: imx: add imx7d usbmisc compatible string
  usb: chipidea: imx: add usb charger detection for imx7d
  doc: usb: ci-hdrc-usb2: add property usb-charger-detection

 .../devicetree/bindings/usb/ci-hdrc-usb2.txt   |   2 +
 drivers/usb/chipidea/ci_hdrc_imx.c |  46 -
 drivers/usb/chipidea/ci_hdrc_imx.h |   3 +
 drivers/usb/chipidea/udc.c |  40 +
 drivers/usb/chipidea/usbmisc_imx.c | 194 +
 include/linux/usb/chipidea.h   |   4 +
 6 files changed, 288 insertions(+), 1 deletion(-)

-- 
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 2/5] usb: chipidea: imx: add usb charger dectection for imx platforms

2016-04-18 Thread Li Jun
Some imx usb charger dectection need software implement the whole process,
during which, pull up&down DP line via usb core is required.

Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 43 ++
 drivers/usb/chipidea/ci_hdrc_imx.h |  3 +++
 drivers/usb/chipidea/usbmisc_imx.c | 33 +
 3 files changed, 79 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index dedc33e..83a59d6 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -244,6 +244,38 @@ static void imx_disable_unprepare_clks(struct device *dev)
}
 }
 
+static enum usb_charger_type imx_usb_charger_det(struct ci_hdrc *ci)
+{
+   struct device *dev = ci->dev->parent;
+   struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+   struct imx_usbmisc_data *mdata = data->usbmisc_data;
+   int ret = UNKNOWN_TYPE;
+
+   if (!mdata || !ci->gadget.charger)
+   return ret;
+
+   if (ci->vbus_active)
+   ret = imx_usbmisc_charger_det(mdata);
+
+   return ret;
+}
+
+static enum usb_charger_type imx_usb_charger_secondary_det(struct ci_hdrc *ci)
+{
+   struct device *dev = ci->dev->parent;
+   struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+   struct imx_usbmisc_data *mdata = data->usbmisc_data;
+   int ret = UNKNOWN_TYPE;
+
+   if (!mdata || !ci->gadget.charger)
+   return ret;
+
+   if (ci->vbus_active)
+   ret = imx_usbmisc_charger_secondary_det(mdata);
+
+   return ret;
+}
+
 static int ci_hdrc_imx_probe(struct platform_device *pdev)
 {
struct ci_hdrc_imx_data *data;
@@ -254,6 +286,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
int ret;
const struct of_device_id *of_id;
const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
+   struct device_node *np = pdev->dev.of_node;
 
of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
if (!of_id)
@@ -292,6 +325,16 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
if (pdata.flags & CI_HDRC_SUPPORTS_RUNTIME_PM)
data->supports_runtime_pm = true;
 
+   if (of_find_property(np, "usb-charger-detection", NULL) &&
+   data->usbmisc_data) {
+   pdata.usb_charger_det = imx_usb_charger_det;
+   if (imx_platform_flag->flags & CI_HDRC_PULL_DP_FOR_CHARGER) {
+   pdata.pull_dp_for_charger = true;
+   pdata.usb_charger_secondary_det =
+   imx_usb_charger_secondary_det;
+   }
+   }
+
ret = imx_usbmisc_init(data->usbmisc_data);
if (ret) {
dev_err(&pdev->dev, "usbmisc init failed, ret=%d\n", ret);
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h 
b/drivers/usb/chipidea/ci_hdrc_imx.h
index 635717e..e34c284 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -23,5 +23,8 @@ struct imx_usbmisc_data {
 int imx_usbmisc_init(struct imx_usbmisc_data *);
 int imx_usbmisc_init_post(struct imx_usbmisc_data *);
 int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *, bool);
+enum usb_charger_type imx_usbmisc_charger_det(struct imx_usbmisc_data *data);
+enum usb_charger_type imx_usbmisc_charger_secondary_det(
+   struct imx_usbmisc_data *data);
 
 #endif /* __DRIVER_USB_CHIPIDEA_CI_HDRC_IMX_H */
diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index ab8b027..3d70712 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "ci_hdrc_imx.h"
 
@@ -87,6 +88,9 @@ struct usbmisc_ops {
int (*post)(struct imx_usbmisc_data *data);
/* It's called when we need to enable/disable usb wakeup */
int (*set_wakeup)(struct imx_usbmisc_data *data, bool enabled);
+   enum usb_charger_type (*charger_det)(struct imx_usbmisc_data *data);
+   enum usb_charger_type (*charger_secondary_det)
+   (struct imx_usbmisc_data *data);
 };
 
 struct imx_usbmisc {
@@ -455,6 +459,35 @@ int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, 
bool enabled)
 }
 EXPORT_SYMBOL_GPL(imx_usbmisc_set_wakeup);
 
+enum usb_charger_type imx_usbmisc_charger_det(struct imx_usbmisc_data *data)
+{
+   struct imx_usbmisc *usbmisc;
+
+   if (!data)
+   return -EINVAL;
+
+   usbmisc = dev_get_drvdata(data->dev);
+   if (!usbmisc->ops->charger_det)
+   return -EINVAL;
+   return usbmisc->ops->charger_det(data);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_charger_det);
+
+enum usb_charger_type imx_usbmisc_charger_secondary_det(
+   struct imx_usbmisc_data *data)
+{
+   stru

[PATCH 1/5] usb: chipidea: add usb charger detection support

2016-04-18 Thread Li Jun
Some usb module implementations has functionality of detect usb charger
type, it can be used by usb charger framework to report max current drawn
from charger in different situations.

Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/udc.c   | 40 
 include/linux/usb/chipidea.h |  4 
 2 files changed, 44 insertions(+)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 065f5d9..4d2187e 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1510,6 +1510,37 @@ static const struct usb_ep_ops usb_ep_ops = {
 /**
  * GADGET block
  */
+static enum usb_charger_type ci_usb_charger_det(struct usb_charger *charger)
+{
+   struct ci_hdrc *ci;
+   enum usb_charger_type ret = UNKNOWN_TYPE;
+
+   if (!charger || !charger->gadget)
+   return ret;
+
+   ci = container_of(charger->gadget, struct ci_hdrc, gadget);
+   if (ci->vbus_active) {
+   if (ci->platdata->usb_charger_det) {
+   ret = ci->platdata->usb_charger_det(ci);
+   if (ret != UNKNOWN_TYPE)
+   return ret;
+   }
+
+   if (ci->platdata->pull_dp_for_charger) {
+   hw_device_reset(ci);
+   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
+   }
+
+   if (ci->platdata->usb_charger_secondary_det)
+   ret = ci->platdata->usb_charger_secondary_det(ci);
+
+   if (ci->platdata->pull_dp_for_charger)
+   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
+   }
+
+   return ret;
+}
+
 static int ci_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
 {
struct ci_hdrc *ci = container_of(_gadget, struct ci_hdrc, gadget);
@@ -1522,6 +1553,9 @@ static int ci_udc_vbus_session(struct usb_gadget 
*_gadget, int is_active)
gadget_ready = 1;
spin_unlock_irqrestore(&ci->lock, flags);
 
+   if (_gadget->charger && is_active)
+   usb_charger_detect_type(_gadget->charger);
+
if (gadget_ready) {
if (is_active) {
pm_runtime_get_sync(&_gadget->dev);
@@ -1922,6 +1956,9 @@ static int udc_start(struct ci_hdrc *ci)
if (retval)
goto destroy_eps;
 
+   if (ci->gadget.charger && ci->platdata->usb_charger_det)
+   ci->gadget.charger->charger_detect = ci_usb_charger_det;
+
pm_runtime_no_callbacks(&ci->gadget.dev);
pm_runtime_enable(&ci->gadget.dev);
 
@@ -1946,6 +1983,9 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
if (!ci->roles[CI_ROLE_GADGET])
return;
 
+   if (ci->gadget.charger)
+   ci->gadget.charger->charger_detect = NULL;
+
usb_del_gadget_udc(&ci->gadget);
 
destroy_eps(ci);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 5dd75fa..7fc781d 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -55,10 +55,14 @@ struct ci_hdrc_platform_data {
 #define CI_HDRC_OVERRIDE_AHB_BURST BIT(9)
 #define CI_HDRC_OVERRIDE_TX_BURST  BIT(10)
 #define CI_HDRC_OVERRIDE_RX_BURST  BIT(11)
+#define CI_HDRC_PULL_DP_FOR_CHARGERBIT(12)
enum usb_dr_modedr_mode;
 #define CI_HDRC_CONTROLLER_RESET_EVENT 0
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT   1
void(*notify_event) (struct ci_hdrc *ci, unsigned event);
+   enum usb_charger_type (*usb_charger_det)(struct ci_hdrc *ci);
+   boolpull_dp_for_charger;
+   enum usb_charger_type (*usb_charger_secondary_det)(struct ci_hdrc *ci);
struct regulator*reg_vbus;
struct usb_otg_caps ci_otg_caps;
booltpl_support;
-- 
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 5/5] doc: usb: ci-hdrc-usb2: add property usb-charger-detection

2016-04-18 Thread Li Jun
Set this property if your usb module has usb charger detect function
and you want to use it.

Signed-off-by: Li Jun 
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt 
b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 1084e2b..52d269e 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -76,6 +76,8 @@ Optional properties:
   needs to make sure it does not send more than 90%
   maximum_periodic_data_per_frame. The use case is multiple transactions, but
   less frame rate.
+- usb-charger-detection: use usb charger detect function provided by usb
+  module; if your usb charger type detection done by PMIC, don't enable this.
 
 i.mx specific properties
 - fsl,usbmisc: phandler of non-core register device, with one
-- 
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 3/5] usb: chipidea: imx: add imx7d usbmisc compatible string

2016-04-18 Thread Li Jun
Add imx7d usbmisc compatible string.

Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/usbmisc_imx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index 3d70712..daa7c02 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -525,6 +525,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
.compatible = "fsl,imx6ul-usbmisc",
.data = &imx6sx_usbmisc_ops,
},
+   {
+   .compatible = "fsl,imx7d-usbmisc",
+   .data = &imx7d_usbmisc_ops,
+   },
{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
-- 
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 4/5] usb: chipidea: imx: add usb charger detection for imx7d

2016-04-18 Thread Li Jun
Adds imx7d usb charger detection implementation, which needs pull up
DP line before do secondary detection and pull down DP afterwards.

Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/ci_hdrc_imx.c |   3 +-
 drivers/usb/chipidea/usbmisc_imx.c | 157 +
 2 files changed, 159 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 83a59d6..5a6b712 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -67,7 +67,8 @@ static const struct ci_hdrc_imx_platform_flag imx6ul_usb_data 
= {
 };
 
 static const struct ci_hdrc_imx_platform_flag imx7d_usb_data = {
-   .flags = CI_HDRC_SUPPORTS_RUNTIME_PM,
+   .flags = CI_HDRC_SUPPORTS_RUNTIME_PM |
+   CI_HDRC_PULL_DP_FOR_CHARGER,
 };
 
 static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
diff --git a/drivers/usb/chipidea/usbmisc_imx.c 
b/drivers/usb/chipidea/usbmisc_imx.c
index daa7c02..4f4ae56 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -74,6 +74,11 @@
 #define VF610_OVER_CUR_DIS BIT(7)
 
 #define MX7D_USBNC_USB_CTRL2   0x4
+#define MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_ENBIT(8)
+#define MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK  (BIT(7) | BIT(6))
+#define MX7D_USBNC_USB_CTRL2_OPMODE(v) (v << 6)
+#define MX7D_USBNC_USB_CTRL2_OPMODE_NON_DRIVING
MX7D_USBNC_USB_CTRL2_OPMODE(1)
+
 #define MX7D_USB_VBUS_WAKEUP_SOURCE_MASK   0x3
 #define MX7D_USB_VBUS_WAKEUP_SOURCE(v) (v << 0)
 #define MX7D_USB_VBUS_WAKEUP_SOURCE_VBUS   MX7D_USB_VBUS_WAKEUP_SOURCE(0)
@@ -81,6 +86,18 @@
 #define MX7D_USB_VBUS_WAKEUP_SOURCE_BVALID MX7D_USB_VBUS_WAKEUP_SOURCE(2)
 #define MX7D_USB_VBUS_WAKEUP_SOURCE_SESS_END   MX7D_USB_VBUS_WAKEUP_SOURCE(3)
 
+#define MX7D_USB_OTG_PHY_CFG2_CHRG_DCDENB  BIT(3)
+#define MX7D_USB_OTG_PHY_CFG2_CHRG_VDATSRCENB0 BIT(2)
+#define MX7D_USB_OTG_PHY_CFG2_CHRG_VDATDETENB0 BIT(1)
+#define MX7D_USB_OTG_PHY_CFG2_CHRG_CHRGSEL BIT(0)
+#define MX7D_USB_OTG_PHY_CFG2  0x34
+
+#define MX7D_USB_OTG_PHY_STATUS0x3c
+#define MX7D_USB_OTG_PHY_STATUS_CHRGDETBIT(29)
+#define MX7D_USB_OTG_PHY_STATUS_VBUS_VLD   BIT(3)
+#define MX7D_USB_OTG_PHY_STATUS_LINE_STATE1BIT(1)
+#define MX7D_USB_OTG_PHY_STATUS_LINE_STATE0BIT(0)
+
 struct usbmisc_ops {
/* It's called once when probe a usb device */
int (*init)(struct imx_usbmisc_data *data);
@@ -385,6 +402,144 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data 
*data)
return 0;
 }
 
+static void imx7d_disable_charger_detector(struct imx_usbmisc_data *data)
+{
+   struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+   unsigned long flags;
+   u32 val;
+
+   spin_lock_irqsave(&usbmisc->lock, flags);
+   val = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG2);
+   val &= ~(MX7D_USB_OTG_PHY_CFG2_CHRG_DCDENB |
+   MX7D_USB_OTG_PHY_CFG2_CHRG_VDATSRCENB0 |
+   MX7D_USB_OTG_PHY_CFG2_CHRG_VDATDETENB0 |
+   MX7D_USB_OTG_PHY_CFG2_CHRG_CHRGSEL);
+   writel(val, usbmisc->base + MX7D_USB_OTG_PHY_CFG2);
+
+   /* Set OPMODE to be 2'b00 and disable its override */
+   val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
+   val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK;
+   writel(val, usbmisc->base + MX7D_USBNC_USB_CTRL2);
+
+   val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
+   writel(val & ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_EN,
+   usbmisc->base + MX7D_USBNC_USB_CTRL2);
+   spin_unlock_irqrestore(&usbmisc->lock, flags);
+}
+
+static int imx7d_charger_data_contact_detect(struct imx_usbmisc_data *data)
+{
+   struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+   int i, data_pin_contact_count = 0;
+   unsigned long flags;
+   u32 val;
+
+   spin_lock_irqsave(&usbmisc->lock, flags);
+
+   /* check if vbus is valid */
+   val = readl(usbmisc->base + MX7D_USB_OTG_PHY_STATUS);
+   if (!(val & MX7D_USB_OTG_PHY_STATUS_VBUS_VLD)) {
+   dev_err(data->dev, "vbus is error\n");
+   spin_unlock_irqrestore(&usbmisc->lock, flags);
+   return -EINVAL;
+   }
+
+   /*
+* - Do not check whether a charger is connected to the USB port
+* - Check whether the USB plug has been in contact with each other
+*/
+   val = readl(usbmisc->base + MX7D_USB_OTG_PHY_CFG2);
+   writel(val | MX7D_USB_OTG_PHY_CFG2_CHRG_DCDENB,
+   usbmisc->base + MX7D_USB_OTG_PHY_CFG2);
+
+   spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+   /* Check if plug is connected */
+   for (i = 0; i < 100; i = i + 1) {
+   val = readl(usbmisc->base + MX7D_USB_OTG_PHY_STATUS);
+   if (!(val & MX7D_USB_OTG_PHY_STATUS_LINE_STATE0)) {
+  

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
>> >>> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
>> >>
>> >> According to the spec we should always be talking about unit loads (1
>> >> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
>> >> work for SS capable ports and SS gadgets (we have quite a few of them,
>> >> actually). You're missing the opportunity of charging at 900mA.
>> >
>> > I follow the DCP/SDP/CDP/ACA type's default current limitation and
>> > user can set them what they want.
>> 
>> no, the user CANNOT set it to what they want. If you get enumerated
>> @100mA and the user just decides to set it to 2000mA, s/he could even
>> melt the USB connector. The kernel _must_ prevent such cases.
>
> root should be allowed to do that.

root should not be allowed to put user at risk. The usb connector is a
path to earth ground in most (all?) desktop computers. Allowing root to
put that connector in a situation where it could melt the connector and
short mains should never be allowed.

> Very often, you want to charge using 1.8A from an old desktop PC.

if that old desktop's port is not a charging port, you shouldn't be
allowed to do that. Not ever.

> N900 will simply not charge from .5A.

it used to with original maemo userspace/kernel.

>> a) you are connected to a dedicated charger
>> 
>>  In this case, you can get up to 2000mA depending on the charger.
>> 
>>  If $this charger can give you or not 2000mA is not detectable,
>>  so what do charging ICs do ? They slowly increase the attached
>>  load accross VBUS/GND and measure VBUS value. When IC notices
>>  VBUS dropping bit, step back to previous load.
>> 
>>  This means you will always charger with maximum rating of DCP.
>> 
>>  Why would user change this ? More is unsafe, less is just
>>  stupid.
>
> Actually, less is not stupid. Charging li-ion battery from li-ion battery 
> might
> be stupid. Imagine I'm on train, with device like N900 (50% battery) and 
> power bank
> (3Ah). I'm actively using the device. If I let it charge at full current, 
> I'll waste
> energy. If I limit current to approximately the power consumption, it will 
> run the
> powerbank empty, first, then empty the internal battery, maximizing total 
> time I
> can use the device.

why would you waste energy ? What the charger chip would do is charge
battery to maximum then just to maintenance charge from that point
on. Where is energy being wasted other than normal heat dissipation ?

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
> Hi!
>
>> > It's your HW :-) You tell me if it's really necessary. But, hey, if you
>> > get enumerated @500mA, this is the host telling you it _CAN_ give you
>> > 500mA. In that case, why wouldn't you ?
>
> Dunno, perhaps not to drain battery in host too quickly?
> Or perhaps you are charging from external battery?
>
>> >>> why RW ? Who's going to use these ? Also, you're not documenting this
>> >>> new sysfs file.
>> >>
>> >> Cause we have show and store operation for SDP type. If users want to
>> >> know or set the SDP current, they can use the sysfs file.
>> >> I'll add the documentation for it.
>> >
>> > but why would the user change it ? Here's the thing: you have a few
>> > posibilities for this:
>> >
>> > a) you are connected to a dedicated charger
>> >
>> > In this case, you can get up to 2000mA depending on the charger.
>> >
>> > If $this charger can give you or not 2000mA is not detectable,
>> > so what do charging ICs do ? They slowly increase the attached
>> > load accross VBUS/GND and measure VBUS value. When IC notices
>> > VBUS dropping bit, step back to previous load.
>> >
>> > This means you will always charger with maximum rating of DCP.
>> >
>> > Why would user change this ? More is unsafe, less is just
>> > stupid.
>
> Less is not neccessarily stupid. First, it is useful for debugging, second, 
> you
> don't know how much this charger can give you. You measured you can get 1.8A,
> but the note on the charger says 1.5A. You may want to go with 1.5A.
>
> Also, there are several incompatible standards for detecting
> "dedicated charger". IIRC iPhone has different one from iPad. So it is
> quite important to be able to control this manually.

manually ??? Hell no! Charger IC should be able to do this no
problem. I would be surprised if there's any charger IC out there which
blindly connects a 1.8A load from the start. What these ICs do is that
they slowly increment the load and check voltage level. They'll continue
to do that up to the maximum you listed (1.8A, let's say). As soon as
voltage drops a bit, charger IC knows that it use previous load.

>> > d) you are connected to a standard port and get enumerated with your
>> > 100mA configuration.
>> >
>> > you *know* 100mA is okay. So you connect a 100mA load and get it
>> > over with.
>> >
>> > This means you will always charger with maximum rating for this
>> > SDP.
>> >
>> > Why would user change this ? More is unsafe, less is just
>> > stupid.
>
> I've needed to override 100mA default many times. Maybe it is unsafe,
> but it is useful.

still unsafe. If you really wanna do that, you're welcome to removing
safety margins from your own kernel, but we're definitely not going to
ship this to millions of users.

> (And with USB 5V connected directly to pretty beefy PC power supply...
> it is sometimes safer than it looks).

you're not considering the thermal dissipation on the USB connector
itself. Many of them might not use good metals because they assume the
maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
you could, literally, melt the connector.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 1/5] usb: chipidea: add usb charger detection support

2016-04-18 Thread kbuild test robot
Hi Li,

[auto build test ERROR on peter.chen-usb/ci-for-usb-next]
[also build test ERROR on v4.6-rc4 next-20160418]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Li-Jun/usb-chipidea-Add-usb-charger-detection-support/20160418-161638
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb 
ci-for-usb-next
config: xtensa-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/usb/chipidea/udc.c:1513:56: warning: 'struct usb_charger' declared 
inside parameter list
static enum usb_charger_type ci_usb_charger_det(struct usb_charger *charger)
   ^
   drivers/usb/chipidea/udc.c:1513:56: warning: its scope is only this 
definition or declaration, which is probably not what you want
   drivers/usb/chipidea/udc.c:1513:30: error: return type is an incomplete type
static enum usb_charger_type ci_usb_charger_det(struct usb_charger *charger)
 ^
   drivers/usb/chipidea/udc.c: In function 'ci_usb_charger_det':
   drivers/usb/chipidea/udc.c:1516:7: error: variable 'ret' has initializer but 
incomplete type
 enum usb_charger_type ret = UNKNOWN_TYPE;
  ^
   drivers/usb/chipidea/udc.c:1516:30: error: 'UNKNOWN_TYPE' undeclared (first 
use in this function)
 enum usb_charger_type ret = UNKNOWN_TYPE;
 ^
   drivers/usb/chipidea/udc.c:1516:30: note: each undeclared identifier is 
reported only once for each function it appears in
   drivers/usb/chipidea/udc.c:1516:24: error: storage size of 'ret' isn't known
 enum usb_charger_type ret = UNKNOWN_TYPE;
   ^
>> drivers/usb/chipidea/udc.c:1518:26: error: dereferencing pointer to 
>> incomplete type
 if (!charger || !charger->gadget)
 ^
   drivers/usb/chipidea/udc.c:1519:3: warning: 'return' with a value, in 
function returning void
  return ret;
  ^
   In file included from include/linux/delay.h:10:0,
from drivers/usb/chipidea/udc.c:13:
   drivers/usb/chipidea/udc.c:1521:27: error: dereferencing pointer to 
incomplete type
 ci = container_of(charger->gadget, struct ci_hdrc, gadget);
  ^
   include/linux/kernel.h:824:49: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
   drivers/usb/chipidea/udc.c:1524:4: error: invalid use of undefined type 
'enum usb_charger_type'
   ret = ci->platdata->usb_charger_det(ci);
   ^
   drivers/usb/chipidea/udc.c:1526:5: warning: 'return' with a value, in 
function returning void
return ret;
^
   drivers/usb/chipidea/udc.c:1535:4: error: invalid use of undefined type 
'enum usb_charger_type'
   ret = ci->platdata->usb_charger_secondary_det(ci);
   ^
   drivers/usb/chipidea/udc.c:1541:2: warning: 'return' with a value, in 
function returning void
 return ret;
 ^
   drivers/usb/chipidea/udc.c:1516:24: warning: unused variable 'ret' 
[-Wunused-variable]
 enum usb_charger_type ret = UNKNOWN_TYPE;
   ^
   drivers/usb/chipidea/udc.c: In function 'ci_udc_vbus_session':
   drivers/usb/chipidea/udc.c:1556:13: error: 'struct usb_gadget' has no member 
named 'charger'
 if (_gadget->charger && is_active)
^
   drivers/usb/chipidea/udc.c:1557:3: error: implicit declaration of function 
'usb_charger_detect_type' [-Werror=implicit-function-declaration]
  usb_charger_detect_type(_gadget->charger);
  ^
   drivers/usb/chipidea/udc.c:1557:34: error: 'struct usb_gadget' has no member 
named 'charger'
  usb_charger_detect_type(_gadget->charger);
 ^
   drivers/usb/chipidea/udc.c: In function 'udc_start':
   drivers/usb/chipidea/udc.c:1959:16: error: 'struct usb_gadget' has no member 
named 'charger'
 if (ci->gadget.charger && ci->platdata->usb_charger_det)
   ^
   drivers/usb/chipidea/udc.c:1960:13: error: 'struct usb_gadget' has no member 
named 'charger'
  ci->gadget.charger->charger_detect = ci_usb_charger_det;
^
   drivers/usb/chipidea/udc.c: In function 'ci_hdrc_gadget_destroy':
   drivers/usb/chipidea/udc.c:1986:16

Re: [PATCH 1/5] usb: chipidea: add usb charger detection support

2016-04-18 Thread kbuild test robot
Hi Li,

[auto build test ERROR on peter.chen-usb/ci-for-usb-next]
[also build test ERROR on v4.6-rc4 next-20160418]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Li-Jun/usb-chipidea-Add-usb-charger-detection-support/20160418-161638
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb 
ci-for-usb-next
config: x86_64-randconfig-x001-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers/usb/chipidea/udc.c:1513:56: warning: 'struct usb_charger' declared 
>> inside parameter list
static enum usb_charger_type ci_usb_charger_det(struct usb_charger *charger)
   ^
>> drivers/usb/chipidea/udc.c:1513:56: warning: its scope is only this 
>> definition or declaration, which is probably not what you want
>> drivers/usb/chipidea/udc.c:1513:30: error: return type is an incomplete type
static enum usb_charger_type ci_usb_charger_det(struct usb_charger *charger)
 ^
   drivers/usb/chipidea/udc.c: In function 'ci_usb_charger_det':
>> drivers/usb/chipidea/udc.c:1516:7: error: variable 'ret' has initializer but 
>> incomplete type
 enum usb_charger_type ret = UNKNOWN_TYPE;
  ^
>> drivers/usb/chipidea/udc.c:1516:30: error: 'UNKNOWN_TYPE' undeclared (first 
>> use in this function)
 enum usb_charger_type ret = UNKNOWN_TYPE;
 ^
   drivers/usb/chipidea/udc.c:1516:30: note: each undeclared identifier is 
reported only once for each function it appears in
>> drivers/usb/chipidea/udc.c:1516:24: error: storage size of 'ret' isn't known
 enum usb_charger_type ret = UNKNOWN_TYPE;
   ^
>> drivers/usb/chipidea/udc.c:1518:26: error: dereferencing pointer to 
>> incomplete type 'struct usb_charger'
 if (!charger || !charger->gadget)
 ^
>> drivers/usb/chipidea/udc.c:1519:10: warning: 'return' with a value, in 
>> function returning void
  return ret;
 ^
>> drivers/usb/chipidea/udc.c:1524:4: error: invalid use of undefined type 
>> 'enum usb_charger_type'
   ret = ci->platdata->usb_charger_det(ci);
   ^
   drivers/usb/chipidea/udc.c:1526:12: warning: 'return' with a value, in 
function returning void
return ret;
   ^
   drivers/usb/chipidea/udc.c:1535:4: error: invalid use of undefined type 
'enum usb_charger_type'
   ret = ci->platdata->usb_charger_secondary_det(ci);
   ^
   drivers/usb/chipidea/udc.c:1541:9: warning: 'return' with a value, in 
function returning void
 return ret;
^
>> drivers/usb/chipidea/udc.c:1516:24: warning: unused variable 'ret' 
>> [-Wunused-variable]
 enum usb_charger_type ret = UNKNOWN_TYPE;
   ^
   drivers/usb/chipidea/udc.c: In function 'ci_udc_vbus_session':
>> drivers/usb/chipidea/udc.c:1556:13: error: 'struct usb_gadget' has no member 
>> named 'charger'
 if (_gadget->charger && is_active)
^
>> drivers/usb/chipidea/udc.c:1557:3: error: implicit declaration of function 
>> 'usb_charger_detect_type' [-Werror=implicit-function-declaration]
  usb_charger_detect_type(_gadget->charger);
  ^
   drivers/usb/chipidea/udc.c:1557:34: error: 'struct usb_gadget' has no member 
named 'charger'
  usb_charger_detect_type(_gadget->charger);
 ^
   drivers/usb/chipidea/udc.c: In function 'udc_start':
   drivers/usb/chipidea/udc.c:1959:16: error: 'struct usb_gadget' has no member 
named 'charger'
 if (ci->gadget.charger && ci->platdata->usb_charger_det)
   ^
   drivers/usb/chipidea/udc.c:1960:13: error: 'struct usb_gadget' has no member 
named 'charger'
  ci->gadget.charger->charger_detect = ci_usb_charger_det;
^
   drivers/usb/chipidea/udc.c: In function 'ci_hdrc_gadget_destroy':
   drivers/usb/chipidea/udc.c:1986:16: error: 'struct usb_gadget' has no member 
named 'charger'
 if (ci->gadget.charger)
   ^
   drivers/usb/chipidea/udc.c:1987:13: error: 'struct usb_gadget' has no member 
named 'charger'
  ci->gadget.charger->charger_detect = NULL;
^
   cc1: some warnings being treated as errors

vim +1513 drivers/usb/chipidea/udc.c

  1507  .fifo_flush=

RE: [PATCH v10 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Jun Li
Hi

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Monday, April 11, 2016 7:15 PM
> To: Jun Li 
> Cc: ba...@kernel.org; gre...@linuxfoundation.org; s...@kernel.org;
> dbarysh...@gmail.com; dw...@infradead.org; peter.c...@freescale.com;
> st...@rowland.harvard.edu; r.bald...@samsung.com;
> yoshihiro.shimoda...@renesas.com; lee.jo...@linaro.org; broo...@kernel.org;
> ckee...@opensource.wolfsonmicro.com; patc...@opensource.wolfsonmicro.com;
> linux...@vger.kernel.org; linux-usb@vger.kernel.org; device-
> mainlin...@lists.linuxfoundation.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v10 1/4] gadget: Introduce the usb charger framework
> 
> Hi Jun,
> 
> Sorry for late reply.
> 
> >> >> >> >> +/*
> >> >> >> >> + * usb_charger_detect_type() - detect the charger type
> manually.
> >> >> >> >> + * @uchger - usb charger device
> >> >> >> >> + *
> >> >> >> >> + * Note: You should ensure you need to detect the charger
> >> >> >> >> +type manually on your
> >> >> >> >> + * platform.
> >> >> >> >> + * You should call it at the right gadget state to avoid
> >> >> >> >> +affecting gadget
> >> >> >> >> + * enumeration.
> >> >> >> >> + */
> >> >> >> >> +int usb_charger_detect_type(struct usb_charger *uchger) {
> >> >> >> >> + enum usb_charger_type type;
> >> >> >> >> +
> >> >> >> >> + if (WARN(!uchger->charger_detect,
> >> >> >> >> +  "charger detection method should not be NULL"))
> >> >> >> >> + return -EINVAL;
> >> >> >> >> +
> >> >> >> >> + type = uchger->charger_detect(uchger);
> >> >> >> >> +
> >> >> >> >> + mutex_lock(&uchger->lock);
> >> >> >> >> + uchger->type = type;
> >> >> >> >> + mutex_unlock(&uchger->lock);
> >> >> >> >> +
> >> >> >> >> + return 0;
> >> >> >> >> +}
> >> >> >> >> +EXPORT_SYMBOL_GPL(usb_charger_detect_type);
> >> >> >> >
> >> >> >> > I still recommend have a central place to call
> >> >> >> > usb_charger_detect_type() to cover real charger detection
> >> >> >> > instead of leaving this question open to diff users. This can
> >> >> >> > be done after vbus-on is detected and before do gadget
> >> >> >> > connect. I don't think this
> >> >> >> will make your framework complicated.
> >> >> >>
> >> >> >> This API is used for detecting the charger type manually
> >> >> >> (software charger detection), so if user's udc driver is needed
> >> >> >> to do this, then they can issue it at their right place to make
> it more flexible.
> >> >> >> But let us see other people's suggestion. Thanks.
> >> >> >
> >> >> > Ok, actually this API can be used for what ever charger
> >> >> > detection type, user can implement any method(manual detection,
> >> >> > directly read result from some HW...what ever) in
> >> >> > uchger->charger_detect(), target is
> >> >>
> >> >> But reading result from some HW dose not means *detect*, actually
> >> >> is
> >> *get*.
> >> >> We can not mix them together with the different semantics.
> >> >
> >> > It doesn’t matter here, you already define the _get_ API for just
> >> > read the charger type from the saved value(uchger->type), so we can
> >> > think the API to make uchger->type from UNKNOW to ONE KNOWN type is
> >> > *detect*, because we don't need 2 separate API one for *get* type
> >> > from HW and another one for *detect* from HW, only one API involve
> >> > HW access
> >> is enough.
> >>
> >> But another issue is some users may need to get the charger type from
> >> power supply by "power_supply_get_property()" function, we need to
> >> integrate with the power supply things in the usb charger framework,
> >> not user to implement that.
> >
> > Why this user(your case) can't put the charger type get by
> > "power_supply_get_property()" in its uchger->charger_detect(), any
> > special reason prevent you doing it? I am not sure if this case is
> > very common, even if it is, you also can put it in
> > usb_charger_detect_type()
> >
> > usb_charger_detect_type()
> > {
> > If can get from power_supply_get_property()
> > ...
> > else if uchger->charger_detect
> > uchger->charger_detect();
> > ...
> > }
> 
> If user want to implement above method, they need to get the
> 'power_supply' structure to do this action, which maybe can not get in
> some contexts. So we need to integrate with the power supply things to
> avoid this situation. IIRC, Felipe suggested me to flesh out the charger
> getting method.
> 

Okay, then I agree with you to let user do that with more flexibility,
I tried to enable usb charger detection on one NXP i.mx platform based on
this framework, works fine, thanks!
 
Li Jun
> Hi Felipe, what do you think of Jun's suggestion? Thanks.
> 
> >
> > I don't know if most design of usb charger detection now days is as
> > easy as your PMIC for software(HW does the whole detection process and
> > prevent the vbus generation until detection has completed), anyway if
> > your framework can guarantee the detection pro

Re: [PATCH 2/5] usb: chipidea: imx: add usb charger dectection for imx platforms

2016-04-18 Thread kbuild test robot
Hi Li,

[auto build test WARNING on peter.chen-usb/ci-for-usb-next]
[also build test WARNING on next-20160418]
[cannot apply to v4.6-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Li-Jun/usb-chipidea-Add-usb-charger-detection-support/20160418-161638
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb 
ci-for-usb-next
config: xtensa-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/usb/chipidea/ci_hdrc_imx.c:247:30: error: return type is an 
incomplete type
static enum usb_charger_type imx_usb_charger_det(struct ci_hdrc *ci)
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c: In function 'imx_usb_charger_det':
   drivers/usb/chipidea/ci_hdrc_imx.c:252:12: error: 'UNKNOWN_TYPE' undeclared 
(first use in this function)
 int ret = UNKNOWN_TYPE;
   ^
   drivers/usb/chipidea/ci_hdrc_imx.c:252:12: note: each undeclared identifier 
is reported only once for each function it appears in
   drivers/usb/chipidea/ci_hdrc_imx.c:254:27: error: 'struct usb_gadget' has no 
member named 'charger'
 if (!mdata || !ci->gadget.charger)
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:255:3: warning: 'return' with a value, in 
function returning void
  return ret;
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:258:3: error: invalid use of undefined 
type 'enum usb_charger_type'
  ret = imx_usbmisc_charger_det(mdata);
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:260:2: warning: 'return' with a value, in 
function returning void
 return ret;
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c: At top level:
   drivers/usb/chipidea/ci_hdrc_imx.c:263:30: error: return type is an 
incomplete type
static enum usb_charger_type imx_usb_charger_secondary_det(struct ci_hdrc 
*ci)
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c: In function 
'imx_usb_charger_secondary_det':
   drivers/usb/chipidea/ci_hdrc_imx.c:268:12: error: 'UNKNOWN_TYPE' undeclared 
(first use in this function)
 int ret = UNKNOWN_TYPE;
   ^
   drivers/usb/chipidea/ci_hdrc_imx.c:270:27: error: 'struct usb_gadget' has no 
member named 'charger'
 if (!mdata || !ci->gadget.charger)
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:271:3: warning: 'return' with a value, in 
function returning void
  return ret;
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:274:3: error: invalid use of undefined 
type 'enum usb_charger_type'
  ret = imx_usbmisc_charger_secondary_det(mdata);
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:276:2: warning: 'return' with a value, in 
function returning void
 return ret;
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c: In function 'ci_hdrc_imx_probe':
>> drivers/usb/chipidea/ci_hdrc_imx.c:330:25: warning: assignment from 
>> incompatible pointer type
  pdata.usb_charger_det = imx_usb_charger_det;
^
   drivers/usb/chipidea/ci_hdrc_imx.c:333:36: warning: assignment from 
incompatible pointer type
   pdata.usb_charger_secondary_det =
   ^

vim +330 drivers/usb/chipidea/ci_hdrc_imx.c

   268  int ret = UNKNOWN_TYPE;
   269  
   270  if (!mdata || !ci->gadget.charger)
   271  return ret;
   272  
   273  if (ci->vbus_active)
 > 274  ret = imx_usbmisc_charger_secondary_det(mdata);
   275  
   276  return ret;
   277  }
   278  
   279  static int ci_hdrc_imx_probe(struct platform_device *pdev)
   280  {
   281  struct ci_hdrc_imx_data *data;
   282  struct ci_hdrc_platform_data pdata = {
   283  .name   = dev_name(&pdev->dev),
   284  .capoffset  = DEF_CAPOFFSET,
   285  };
   286  int ret;
   287  const struct of_device_id *of_id;
   288  const struct ci_hdrc_imx_platform_flag *imx_platform_flag;
   289  struct device_node *np = pdev->dev.of_node;
   290  
   291  of_id = of_match_device(ci_hdrc_imx_dt_ids, &pdev->dev);
   292  if (!of_id)
   293  return -ENODEV;
   294  
   295  imx_platform_flag = of_id->data;
   296  
   297  data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
   298  if (!data)
   299  return -ENOMEM;
   300  
   301  platform_set_drvdat

Re: [PATCH 2/5] usb: chipidea: imx: add usb charger dectection for imx platforms

2016-04-18 Thread kbuild test robot
Hi Li,

[auto build test ERROR on peter.chen-usb/ci-for-usb-next]
[also build test ERROR on next-20160418]
[cannot apply to v4.6-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Li-Jun/usb-chipidea-Add-usb-charger-detection-support/20160418-161638
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb 
ci-for-usb-next
config: x86_64-randconfig-x001-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers/usb/chipidea/ci_hdrc_imx.c:247:30: error: return type is an 
>> incomplete type
static enum usb_charger_type imx_usb_charger_det(struct ci_hdrc *ci)
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c: In function 'imx_usb_charger_det':
>> drivers/usb/chipidea/ci_hdrc_imx.c:252:12: error: 'UNKNOWN_TYPE' undeclared 
>> (first use in this function)
 int ret = UNKNOWN_TYPE;
   ^
   drivers/usb/chipidea/ci_hdrc_imx.c:252:12: note: each undeclared identifier 
is reported only once for each function it appears in
>> drivers/usb/chipidea/ci_hdrc_imx.c:254:27: error: 'struct usb_gadget' has no 
>> member named 'charger'
 if (!mdata || !ci->gadget.charger)
  ^
>> drivers/usb/chipidea/ci_hdrc_imx.c:255:10: warning: 'return' with a value, 
>> in function returning void
  return ret;
 ^
>> drivers/usb/chipidea/ci_hdrc_imx.c:258:3: error: invalid use of undefined 
>> type 'enum usb_charger_type'
  ret = imx_usbmisc_charger_det(mdata);
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:260:9: warning: 'return' with a value, in 
function returning void
 return ret;
^
   drivers/usb/chipidea/ci_hdrc_imx.c: At top level:
   drivers/usb/chipidea/ci_hdrc_imx.c:263:30: error: return type is an 
incomplete type
static enum usb_charger_type imx_usb_charger_secondary_det(struct ci_hdrc 
*ci)
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c: In function 
'imx_usb_charger_secondary_det':
   drivers/usb/chipidea/ci_hdrc_imx.c:268:12: error: 'UNKNOWN_TYPE' undeclared 
(first use in this function)
 int ret = UNKNOWN_TYPE;
   ^
   drivers/usb/chipidea/ci_hdrc_imx.c:270:27: error: 'struct usb_gadget' has no 
member named 'charger'
 if (!mdata || !ci->gadget.charger)
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:271:10: warning: 'return' with a value, 
in function returning void
  return ret;
 ^
   drivers/usb/chipidea/ci_hdrc_imx.c:274:3: error: invalid use of undefined 
type 'enum usb_charger_type'
  ret = imx_usbmisc_charger_secondary_det(mdata);
  ^
   drivers/usb/chipidea/ci_hdrc_imx.c:276:9: warning: 'return' with a value, in 
function returning void
 return ret;
^
   drivers/usb/chipidea/ci_hdrc_imx.c: In function 'ci_hdrc_imx_probe':
>> drivers/usb/chipidea/ci_hdrc_imx.c:330:25: error: assignment from 
>> incompatible pointer type [-Werror=incompatible-pointer-types]
  pdata.usb_charger_det = imx_usb_charger_det;
^
   drivers/usb/chipidea/ci_hdrc_imx.c:333:36: error: assignment from 
incompatible pointer type [-Werror=incompatible-pointer-types]
   pdata.usb_charger_secondary_det =
   ^
   cc1: some warnings being treated as errors

vim +247 drivers/usb/chipidea/ci_hdrc_imx.c

   241  clk_disable_unprepare(data->clk_ipg);
   242  } else {
   243  clk_disable_unprepare(data->clk);
   244  }
   245  }
   246  
 > 247  static enum usb_charger_type imx_usb_charger_det(struct ci_hdrc *ci)
   248  {
   249  struct device *dev = ci->dev->parent;
   250  struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
   251  struct imx_usbmisc_data *mdata = data->usbmisc_data;
 > 252  int ret = UNKNOWN_TYPE;
   253  
 > 254  if (!mdata || !ci->gadget.charger)
 > 255  return ret;
   256  
   257  if (ci->vbus_active)
 > 258  ret = imx_usbmisc_charger_det(mdata);
   259  
   260  return ret;
   261  }
   262  
   263  static enum usb_charger_type imx_usb_charger_secondary_det(struct 
ci_hdrc *ci)
   264  {
   265  struct device *dev = ci->dev->parent;
   266  struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
   267  struct imx_usbmisc_data *mdata = data->usbmisc_data;
   268  int ret = UNKNOWN_TYPE;
   269  
   270  if (!mdata || !ci->gadget.charger)
   271 

Re: [PATCH 2/5] usb: chipidea: imx: add usb charger dectection for imx platforms

2016-04-18 Thread kbuild test robot
Hi Li,

[auto build test ERROR on peter.chen-usb/ci-for-usb-next]
[also build test ERROR on next-20160418]
[cannot apply to v4.6-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Li-Jun/usb-chipidea-Add-usb-charger-detection-support/20160418-161638
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb 
ci-for-usb-next
config: i386-randconfig-r0-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/usb/chipidea/usbmisc_imx.c:17:31: fatal error: linux/usb/charger.h: 
>> No such file or directory
   compilation terminated.

vim +17 drivers/usb/chipidea/usbmisc_imx.c

11  
12  #include 
13  #include 
14  #include 
15  #include 
16  #include 
  > 17  #include 
18  
19  #include "ci_hdrc_imx.h"
20  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data


Re: [PATCH v2] watchdog: add driver for StreamLabs USB watchdog device

2016-04-18 Thread Oliver Neukum
On Mon, 2016-04-18 at 03:53 +0100, Alexey Klimov wrote:
> This patch creates new driver that supports StreamLabs usb watchdog
> device. This device plugs into 9-pin usb header and connects to
> reset pin and reset button on common PC.
> 
> USB commands used to communicate with device were reverse
> engineered using usbmon.

Almost. I see only one issue.

> +struct streamlabs_wdt {
> + struct watchdog_device wdt_dev;
> + struct usb_interface *intf;
> +
> + struct mutex lock;
> + u8 buffer[BUFFER_LENGTH];

That is wrong.

> +};
> +

[..]

> +static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev, u16 
> cmd)
> +{
> + struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
> + struct usb_device *usbdev;
> + int retval;
> + int size;
> + unsigned long timeout_msec;
> +
> + int retry_counter = 10; /* how many times to re-send stop cmd */
> +
> + mutex_lock(&streamlabs_wdt->lock);
> +
> + if (unlikely(!streamlabs_wdt->intf)) {
> + mutex_unlock(&streamlabs_wdt->lock);
> + return -ENODEV;
> + }
> +
> + usbdev = interface_to_usbdev(streamlabs_wdt->intf);
> + timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
> +
> + do {
> + usb_streamlabs_wdt_prepare_buf((u16 *) streamlabs_wdt->buffer,
> + cmd, timeout_msec);
> + /* send command to watchdog */
> + retval = usb_interrupt_msg(usbdev, usb_sndintpipe(usbdev, 0x02),
> + streamlabs_wdt->buffer, BUFFER_TRANSFER_LENGTH,

Because of this line.

The problem is subtle. Your buffer and your lock share a cacheline.
On some architecture the cache is not consistent with respect to DMA.
On them cachelines holding a buffer for DMA need to be flushed to RAM
and invalidated and you may read from them only after DMA has finished.

Thus you may have prepared a cacheline for DMA but somebody tries taking
the lock. Then the cacheline with the lock is read from RAM. If that
happens before you finish the DMA the data resulting from DMA is lost.

The fix is to allocate the buffer with its own allocation. The VM
subsystem makes sure separate allocation don't share cachelines.

That is the long explanation for what I mean when I say that you violate
the DMA rules.

Regards
Oliver


--
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 v10 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Baolin Wang
Hi Jun,

On 18 April 2016 at 16:27, Jun Li  wrote:
>> >>
>> >> But another issue is some users may need to get the charger type from
>> >> power supply by "power_supply_get_property()" function, we need to
>> >> integrate with the power supply things in the usb charger framework,
>> >> not user to implement that.
>> >
>> > Why this user(your case) can't put the charger type get by
>> > "power_supply_get_property()" in its uchger->charger_detect(), any
>> > special reason prevent you doing it? I am not sure if this case is
>> > very common, even if it is, you also can put it in
>> > usb_charger_detect_type()
>> >
>> > usb_charger_detect_type()
>> > {
>> > If can get from power_supply_get_property()
>> > ...
>> > else if uchger->charger_detect
>> > uchger->charger_detect();
>> > ...
>> > }
>>
>> If user want to implement above method, they need to get the
>> 'power_supply' structure to do this action, which maybe can not get in
>> some contexts. So we need to integrate with the power supply things to
>> avoid this situation. IIRC, Felipe suggested me to flesh out the charger
>> getting method.
>>
>
> Okay, then I agree with you to let user do that with more flexibility,
> I tried to enable usb charger detection on one NXP i.mx platform based on
> this framework, works fine, thanks!

That's great. Thanks.

-- 
Baolin.wang
Best Regards
--
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: chipidea: add flag CI_HDRC_DP_ALWAYS_PULLUP

2016-04-18 Thread maitysanchayan
Hello Peter,

I tested this on a Colibri Vybrid VF61 module with the patches applied
on top of for-next branch.

root@colibri-vf:~# uname -a
Linux colibri-vf 4.6.0-rc1-00044-gc76529e-dirty #120 Mon Apr 18 13:46:34 IST 
2016 armv7l GNU/Linux

Host and client mode work only on boot up. Trying to change the mode by 
disconnecting/reconnecting
the cable or plugging in a USB device does not work. Also when the USB client 
mode is working on
boot up, disconnecting the cable results in the below stack trace.

root@colibri-vf:~# ping 192.168.11.234  
  
PING 192.168.11.234 (192.168.11.234): 56 data bytes
64 bytes from 192.168.11.234: seq=0 ttl=64 time=5.399 ms
^C
--- 192.168.11.234 ping statistics ---
1 packets transmitted, 1 packets received, 0% packet loss
round-trip min/avg/max = 5.399/5.399/5.399 ms
- On Cable disconnection-
root@colibri-vf:~# [   23.923607] irq 35: nobody cared (try booting with the 
"irqpoll" option)
[   23.930354] CPU: 0 PID: 0 Comm: swapper Not tainted 
4.6.0-rc1-00044-gc76529e-dirty #120
[   23.938359] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
[   23.944807] Backtrace: 
[   23.947327] [<8010b56c>] (dump_backtrace) from [<8010b764>] 
(show_stack+0x18/0x1c)
[   23.954900]  r7:0023 r6: r5: r4:8f4ad240
[   23.960644] [<8010b74c>] (show_stack) from [<803a1c00>] 
(dump_stack+0x24/0x28)
[   23.967893] [<803a1bdc>] (dump_stack) from [<80148150>] 
(__report_bad_irq+0x30/0xb8)
[   23.975655] [<80148120>] (__report_bad_irq) from [<801484a8>] 
(note_interrupt+0x260/0x2b0)
[   23.983918]  r5: r4:8f4ad240
[   23.987538] [<80148248>] (note_interrupt) from [<801460cc>] 
(handle_irq_event_percpu+0xe0/0x154)
[   23.996328]  r10: r9:80b35683 r8:8f4ad240 r7:0023 r6: 
r5:
[   24.004242]  r4: r3:
[   24.007860] [<80145fec>] (handle_irq_event_percpu) from [<80146170>] 
(handle_irq_event+0x30/0x44)
[   24.016732]  r10:80b0210c r9:90003100 r8:8f406000 r7:80b01f10 r6: 
r5:80b14ed0
[   24.024646]  r4:8f4ad240
[   24.027206] [<80146140>] (handle_irq_event) from [<80148d2c>] 
(handle_fasteoi_irq+0xa8/0x170)
[   24.035737]  r5:80b14ed0 r4:8f4ad240
[   24.039363] [<80148c84>] (handle_fasteoi_irq) from [<801457a0>] 
(generic_handle_irq+0x2c/0x3c)
[   24.047973]  r7:80b01f10 r6: r5:0023 r4:80b14d8c
[   24.053706] [<80145774>] (generic_handle_irq) from [<80145a30>] 
(__handle_domain_irq+0x5c/0xb0)
[   24.062420] [<801459d4>] (__handle_domain_irq) from [<801013d0>] 
(gic_handle_irq+0x50/0x84)
[   24.070771]  r9:90003100 r8:80b01e00 r7:90002100 r6:9000210c r5:80b023b0 
r4:80b14ec0
[   24.078611] [<80101380>] (gic_handle_irq) from [<8010c214>] 
(__irq_svc+0x54/0x70)
[   24.086101] Exception stack(0x80b01e00 to 0x80b01e48)
[   24.091173] 1e00: 80b360c0 0020 80b36080  0002 0010 
 
[   24.099363] 1e20: 8f406000 90003100 80b0210c 80b01eac 0020 80b01e50 
8011cf18 8011caec
[   24.107545] 1e40: 600c0113 
[   24.111039]  r9:90003100 r8:8f406000 r7:80b01e34 r6: r5:600c0113 
r4:8011caec
[   24.118892] [<8011ca4c>] (__do_softirq) from [<8011cf18>] 
(irq_exit+0xb8/0xf4)
[   24.126114]  r10:80b0210c r9:90003100 r8:8f406000 r7: r6: 
r5:0010
[   24.134028]  r4:80b14d8c
[   24.136587] [<8011ce60>] (irq_exit) from [<80145a34>] 
(__handle_domain_irq+0x60/0xb0)
[   24.144429] [<801459d4>] (__handle_domain_irq) from [<801013d0>] 
(gic_handle_irq+0x50/0x84)
[   24.152782]  r9:90003100 r8:80b01f10 r7:90002100 r6:9000210c r5:80b023b0 
r4:80b14ec0
[   24.160621] [<80101380>] (gic_handle_irq) from [<8010c214>] 
(__irq_svc+0x54/0x70)
[   24.168110] Exception stack(0x80b01f10 to 0x80b01f58)
[   24.173175] 1f00: 0001  
 80115ac0
[   24.181365] 1f20:  80b0210c  80b29c78 80b02114 80b0 
80b0210c 80b01f6c
[   24.189554] 1f40: 80b01f70 80b01f60 80108200 80108204 600c0013 
[   24.196173]  r9:80b0 r8:80b02114 r7:80b01f44 r6: r5:600c0013 
r4:80108204
[   24.204026] [<801081c4>] (arch_cpu_idle) from [<8013dea8>] 
(default_idle_call+0x28/0x34)
[   24.212134] [<8013de80>] (default_idle_call) from [<8013e018>] 
(cpu_startup_entry+0x164/0x1c0)
[   24.220776] [<8013deb4>] (cpu_startup_entry) from [<806fa2a4>] 
(rest_init+0x78/0x7c)
[   24.228518]  r7:
[   24.231097] [<806fa22c>] (rest_init) from [<80a00ce0>] 
(start_kernel+0x370/0x37c)
[   24.238599] [<80a00970>] (start_kernel) from [<80008078>] (0x80008078)
[   24.245127] handlers:
[   24.247415] [<804f2dcc>] ci_irq
[   24.250578] Disabling IRQ #35

Below is a diff of the changes to the kernel after applying these two patches

diff --git a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi 
b/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
index a8a8e43..e85b534 100644
--- a/arch/arm/boot/dts/vf-colibri-eval-v3.dtsi
+++ b/arc

Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

2016-04-18 Thread Jacek Anaszewski

Hi Pavel,

On 04/15/2016 01:53 PM, Pavel Machek wrote:

Hi!


How about implementing patterns as a specific typer of triggers?
Let's say we have ledtrig-rgb-pattern:


Well, we'd need ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, ... , as we
can have more than one rgb led. But yes.


Triggers can have many listeners, i.e. led_trigger_event() sets
brightness on all LED class devices registered on given trigger.
We could have led_trigger_rgb_event() that would set brightness
on all groups-of-three LEDs registered on given rgb-trigger.


I do not understand that.


Triggers are defined as kernel source of led events.

Currently we have two types of triggers as far as the source of
led event is concerned:
- triggers that are created per LED class device and therefore each
  LED class device has their own source of kernel event,
  initialized on trigger activation (e.g. ledtrig-timer,
  ledtrig-heartbeat, ledtrig-oneshot),
- triggers that propagate kernel events from one source to many
  listeners (e.g.ledtrig-ide-disk, ledtrig-cpu) - they internally
  use led_trigger_event(), which iterates through all LED class devices
  registered on a trigger and applies the same brightness.

In case of RGB trigger we'd like to have a common source of kernel
events for three LED class devices. After deeper analysis I'd modify
the approach a bit, in order to add a capability of generating kernel
led events from user space.

Let's say that we have LED RGB driver that exposes three LED class
devices: lp5523:red, lp5523:green, lp5523 blue.

$echo "rgb-pattern-lp5523" > /sys/class/leds/lp5523::red/trigger
$echo "red" > /sys/class/leds/lp5523::red/rgb_color
$echo "rgb-pattern-lp5523" > /sys/class/leds/lp5523::green/trigger
$echo "green" > /sys/class/leds/lp5523::green/rgb_color
$echo "rgb-pattern-lp5523" > /sys/class/leds/lp5523::blue/trigger
$echo "blue" > /sys/class/leds/lp5523::blue/rgb_color

led-rgb-pattern trigger would create a new trigger each time a non
existing rgp-pattern-* suffix is passed. In order to make it possible
for the user space to generate trigger events, a dedicated sysfs
interface would have to be exposed. How about creating a new LED RGB
class device that would expose "color" sysfs attribute with three space
separated R G B values? The device would appear in the sysfs after
rgb-pattern trigger is created.

Internally the LED RGB class device would use a new
led_trigger_rgb_event() to set the color:


void led_trigger_rgb_event(struct led_trigger *trig,
enum led_brightness red,
enum led_brightness green,
enum led_brightness blue,
{
struct led_classdev *led_cdev;

if (!trig)
return;

read_lock(&trig->leddev_list_lock);
list_for_each_entry(led_cdev, &trig->led_cdevs, trig_list)
   if (led_cdev>flags & LED_RGB_COLOR_R)
   led_set_brightness(led_cdev, red);
   else if (led_cdev>flags & LED_RGB_COLOR_G)
   led_set_brightness(led_cdev, green);
   else if (led_cdev>flags & LED_RGB_COLOR_B)
   led_set_brightness(led_cdev, blue);
read_unlock(&trig->leddev_list_lock);
}






I agree that ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, etc. would
be also needed to add a capability of setting different colors on
different LED devices.


Ok.


For patterns, I'd suggest array of (r g b time) values.

Pattern engines can do stuff like "slowly turn LED from off to red, then switch 
color to
white, then slowly turn it to yellow, then turn it off at once" with defined 
speeds
for "slowly" and option of either linear on non-linear brightness ramping.

The last option might be a bit too much, but I believe we should support the 
rest.


Yes, that's an interesting idea. It also turns out that trigger based
patterns could be also used for defining generic patterns for a group
of monochrome LEDs.


Yes, controlling monochrome LEDs synchronously is another task for
patterns. Actually, N900 uses that to control 6 keyboard backlight
LEDs synchronously... and yes, it would be somehow nice to preserve
this functionality.



--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: f_mass_storage: test whether thread is running before starting another

2016-04-18 Thread Andrzej Pietrasiewicz

Hi Michał,

W dniu 07.04.2016 o 18:40, Michal Nazarewicz pisze:

On Thu, 7 Apr 2016, Michal Nazarewicz wrote:

This makes me suspect it’s not possible to link a function instance to
the same configuration twice, but now that I think about it, I’m not
quite sure what would happen if one did:

ln -s functions/mass_storage.0 configs/c.1/foo
ln -s functions/mass_storage.0 configs/c.1/bar


On Thu, Apr 07 2016, Alan Stern wrote:

Do you think it would be worthwhile to check for this possibility in
the driver and report an error?


I think this should be (if it isn’t already) blocked on configfs side.
I cannot see a legitimate use of such configuration and I wouldn’t be
surprised if other function drivers broke as well.



Sorry about late response.

The function responsible for verifying if a symlink can be made is
in drivers/usb/gadget/configfs.c: config_usb_cfg_link()

There is a comment from the author:

/*
 * Make sure this function is from within our _this_ gadget and not
 * from another gadget or a random directory.
 * Also a function instance can only be linked once.
 */

This is the code fragment of interest:

list_for_each_entry(f, &cfg->func_list, list) {
if (f->fi == fi) {
ret = -EEXIST;
goto out;
}
}


AP
--
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: gadget: pch_udc: don't free devm allocated memory

2016-04-18 Thread Felipe Balbi
Coccinelle caught this instance of us kfree()ing
devm-allocated memory. The solution is just to not
do anything in our gadget_release.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/gadget/udc/pch_udc.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index b2b70d4e2f5b..ebc51ec5790a 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -2835,17 +2835,6 @@ static void pch_udc_setup_ep0(struct pch_udc_dev *dev)
 }
 
 /**
- * gadget_release() - Free the gadget driver private data
- * @pdev   reference to struct pci_dev
- */
-static void gadget_release(struct device *pdev)
-{
-   struct pch_udc_dev *dev = dev_get_drvdata(pdev);
-
-   kfree(dev);
-}
-
-/**
  * pch_udc_pcd_reinit() - This API initializes the endpoint structures
  * @dev:   Reference to the driver structure
  */
@@ -3151,8 +3140,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
 
/* Put the device in disconnected state till a driver is bound */
pch_udc_set_disconnect(dev);
-   retval = usb_add_gadget_udc_release(&pdev->dev, &dev->gadget,
-   gadget_release);
+   retval = usb_add_gadget_udc(&pdev->dev, &dev->gadget);
if (retval)
goto finished;
return 0;
-- 
2.8.0.rc2

--
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/3] usb: dwc3: core: add fifo space helper

2016-04-18 Thread Felipe Balbi
this helper will be used, initially, to dump space
of different queues and fifos in dwc3 to
debugfs. Later, it'll be used to issue remote wakeup
when we want to start a transfer and there's
something in a TX FIFO.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.c | 14 ++
 drivers/usb/dwc3/core.h | 14 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 6f57929f09b4..c050a88c16d4 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -60,6 +60,20 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
dwc3_writel(dwc->regs, DWC3_GCTL, reg);
 }
 
+u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type)
+{
+   struct dwc3 *dwc = dep->dwc;
+   u32 reg;
+
+   dwc3_writel(dwc->regs, DWC3_GDBGFIFOSPACE,
+   DWC3_GDBGFIFOSPACE_NUM(dep->number) |
+   DWC3_GDBGFIFOSPACE_TYPE(type));
+
+   reg = dwc3_readl(dwc->regs, DWC3_GDBGFIFOSPACE);
+
+   return DWC3_GDBGFIFOSPACE_SPACE_AVAILABLE(reg);
+}
+
 /**
  * dwc3_core_soft_reset - Issues core soft reset and PHY reset
  * @dwc: pointer to our context structure
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 832da3f2e03a..c3ea4270bf33 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -152,6 +152,19 @@
 
 /* Bit fields */
 
+/* Global Debug Queue/FIFO Space Available Register */
+#define DWC3_GDBGFIFOSPACE_NUM(n)  ((n) & 0x1f)
+#define DWC3_GDBGFIFOSPACE_TYPE(n) (((n) << 5) & 0x1e0)
+#define DWC3_GDBGFIFOSPACE_SPACE_AVAILABLE(n) (((n) >> 16) & 0x)
+
+#define DWC3_TXFIFOQ   1
+#define DWC3_RXFIFOQ   3
+#define DWC3_TXREQQ5
+#define DWC3_RXREQQ7
+#define DWC3_RXINFOQ   9
+#define DWC3_DESCFETCHQ13
+#define DWC3_EVENTQ15
+
 /* Global Configuration Register */
 #define DWC3_GCTL_PWRDNSCALE(n)((n) << 19)
 #define DWC3_GCTL_U2RSTECN (1 << 16)
@@ -1043,6 +1056,7 @@ struct dwc3_gadget_ep_cmd_params {
 
 /* prototypes */
 void dwc3_set_mode(struct dwc3 *dwc, u32 mode);
+u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type);
 
 /* check whether we are on the DWC_usb31 core */
 static inline bool dwc3_is_usb31(struct dwc3 *dwc)
-- 
2.8.0.rc2

--
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 2/3] usb: dwc3: core: add helper to extract trb type

2016-04-18 Thread Felipe Balbi
This helper will be used later to convert trb type
into a human-readable string for debugfs.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index c3ea4270bf33..bbbd1789596e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -580,6 +580,7 @@ enum dwc3_link_state {
 #define DWC3_TRB_CTRL_IOC  (1 << 11)
 #define DWC3_TRB_CTRL_SID_SOFN(n)  (((n) & 0x) << 14)
 
+#define DWC3_TRBCTL_TYPE(n)((n) & (0x3f << 4))
 #define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1)
 #define DWC3_TRBCTL_CONTROL_SETUP  DWC3_TRB_CTRL_TRBCTL(2)
 #define DWC3_TRBCTL_CONTROL_STATUS2DWC3_TRB_CTRL_TRBCTL(3)
-- 
2.8.0.rc2

--
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/3] usb: dwc3: debugfs: dump out endpoint details

2016-04-18 Thread Felipe Balbi
There's a bunch of information in the debug register
set from dwc3 which is useful in some debugging
scenarios. Let's dump them out in endpoint-specific
directories and designated files.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/debugfs.c | 302 +
 1 file changed, 302 insertions(+)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 81faddc60c8e..6c14f653dc9b 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -618,6 +618,306 @@ static const struct file_operations dwc3_link_state_fops 
= {
.release= single_release,
 };
 
+struct dwc3_ep_file_map {
+   char name[25];
+   int (*show)(struct seq_file *s, void *unused);
+};
+
+static int dwc3_tx_fifo_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_TXFIFOQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_rx_fifo_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_RXFIFOQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_tx_request_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_TXREQQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_rx_request_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_RXREQQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_rx_info_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_RXINFOQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_descriptor_fetch_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_DESCFETCHQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_event_queue_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+   u32 val;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   val = dwc3_core_fifo_space(dep, DWC3_EVENTQ);
+   seq_printf(s, "%u\n", val);
+   spin_unlock_irqrestore(&dwc->lock, flags);
+
+   return 0;
+}
+
+static int dwc3_ep_transfer_type_show(struct seq_file *s, void *unused)
+{
+   struct dwc3_ep  *dep = s->private;
+   struct dwc3 *dwc = dep->dwc;
+   unsigned long   flags;
+
+   spin_lock_irqsave(&dwc->lock, flags);
+   if (!(dep->flags & DWC3_EP_ENABLED) ||
+   !dep->endpoint.desc) {
+   seq_printf(s, "--\n");
+   goto out;
+   }
+
+   switch (usb_endpoint_type(dep->endpoint.desc)) {
+   case USB_ENDPOINT_XFER_CONTROL:
+   seq_printf(s, "control\n");
+   break;
+   case USB_ENDPOINT_XFER_ISOC:
+   seq_printf(s, "isochronous\n");
+   break;
+   case USB_ENDPOINT_XFER_BULK:
+   seq_printf(s, "bulk\n");
+   break;
+   case USB_ENDPOINT_XFER_INT:
+   seq_printf(s, "interrupt\n");
+   break;
+   default:
+   seq_printf(s, "--\n");
+  

[PATCHv5 0/3] usb: storage: increase limit to 2048 sectors

2016-04-18 Thread Felipe Balbi
Hi,

Here's v5 of the 2048 sector limit patch series.

Changes since v4:
- Respect possible TAPE device types quirks

Changes since v3:
- Respect USB_FL_MAX_SECTORS* if they are set for USB3 devices

Changes since v2:
- Add a patch fixing multi-line comment style
- s/limitting/limiting
- Remove US_FL_USB3 flag

Felipe Balbi (3):
  usb: storage: scsiglue: further describe our 240 sector limit
  usb: storage: scsiglue: limit USB3 devices to 2048 sectors
  usb: storage: fix multi-line comment style

 drivers/usb/storage/alauda.c   |  22 ++-
 drivers/usb/storage/cypress_atacb.c|  34 ++--
 drivers/usb/storage/datafab.c  |  22 ++-
 drivers/usb/storage/debug.c|   3 +-
 drivers/usb/storage/debug.h|   3 +-
 drivers/usb/storage/ene_ub6250.c   |  25 +--
 drivers/usb/storage/freecom.c  |  75 +---
 drivers/usb/storage/initializers.c |  15 +-
 drivers/usb/storage/initializers.h |  15 +-
 drivers/usb/storage/isd200.c   |  51 ++---
 drivers/usb/storage/jumpshot.c |  22 ++-
 drivers/usb/storage/karma.c|   3 +-
 drivers/usb/storage/option_ms.c|   6 +-
 drivers/usb/storage/protocol.c |  12 +-
 drivers/usb/storage/protocol.h |   3 +-
 drivers/usb/storage/realtek_cr.c   |  12 +-
 drivers/usb/storage/scsiglue.c | 169 -
 drivers/usb/storage/scsiglue.h |   3 +-
 drivers/usb/storage/sddr09.c   |  82 +---
 drivers/usb/storage/sddr55.c   |  45 +++--
 drivers/usb/storage/shuttle_usbat.c|  16 +-
 drivers/usb/storage/sierra_ms.c|   3 +-
 drivers/usb/storage/transport.c| 165 ++--
 drivers/usb/storage/transport.h|   3 +-
 drivers/usb/storage/uas.c  |   3 +-
 drivers/usb/storage/unusual_alauda.h   |   3 +-
 drivers/usb/storage/unusual_cypress.h  |   3 +-
 drivers/usb/storage/unusual_datafab.h  |   6 +-
 drivers/usb/storage/unusual_devs.h | 334 ++---
 drivers/usb/storage/unusual_freecom.h  |   3 +-
 drivers/usb/storage/unusual_isd200.h   |   3 +-
 drivers/usb/storage/unusual_jumpshot.h |   3 +-
 drivers/usb/storage/unusual_karma.h|   3 +-
 drivers/usb/storage/unusual_onetouch.h |   6 +-
 drivers/usb/storage/unusual_realtek.h  |   3 +-
 drivers/usb/storage/unusual_sddr09.h   |   3 +-
 drivers/usb/storage/unusual_sddr55.h   |   3 +-
 drivers/usb/storage/unusual_uas.h  |   3 +-
 drivers/usb/storage/unusual_usbat.h|   3 +-
 drivers/usb/storage/usb.c  |  98 ++
 drivers/usb/storage/usb.h  |  14 +-
 drivers/usb/storage/usual-tables.c |   3 +-
 42 files changed, 851 insertions(+), 455 deletions(-)

-- 
2.8.0.rc2

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


[PATCHv5 2/3] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

2016-04-18 Thread Felipe Balbi
USB3 devices, because they are much newer, have much
less chance of having issues with larger transfers.

We still keep a limit because anything above 2048
sectors really rendered negligible speed
improvements, so we will simply ignore
that. Transferring 1MiB should already give us
pretty good performance.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/storage/scsiglue.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 9da1fb3d0ff4..88920142e375 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -133,6 +133,11 @@ static int slave_configure(struct scsi_device *sdev)
 * let the queue segment size sort out the real limit.
 */
blk_queue_max_hw_sectors(sdev->request_queue, 0x7F);
+   } else if (us->pusb_dev->speed >= USB_SPEED_SUPER) {
+   /* USB3 devices will be limited to 2048 sectors. This gives us
+* better throughput on most devices.
+*/
+   blk_queue_max_hw_sectors(sdev->request_queue, 2048);
}
 
/* Some USB host controllers can't do DMA; they have to use PIO.
-- 
2.8.0.rc2

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


[PATCHv5 1/3] usb: storage: scsiglue: further describe our 240 sector limit

2016-04-18 Thread Felipe Balbi
Just so we have some sort of documentation as to why
we limit our Mass Storage transfers to 240 sectors,
let's update the comment to make clearer that
devices were found that would choke with larger
transfers.

While at that, also make sure to clarify that other
operating systems have similar, albeit different,
limits on mass storage transfers.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/storage/scsiglue.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 90901861bfc0..9da1fb3d0ff4 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -565,7 +565,24 @@ static const struct scsi_host_template 
usb_stor_host_template = {
/* lots of sg segments can be handled */
.sg_tablesize = SCSI_MAX_SG_CHAIN_SEGMENTS,
 
-   /* limit the total size of a transfer to 120 KB */
+
+   /*
+* Limit the total size of a transfer to 120 KB.
+*
+* Some devices are known to choke with anything larger. It seems like
+* the problem stems from the fact that original IDE controllers had
+* only an 8-bit register to hold the number of sectors in one transfer
+* and even those couldn't handle a full 256 sectors.
+*
+* Because we want to make sure we interoperate with as many devices as
+* possible, we will maintain a 240 sector transfer size limit for USB
+* Mass Storage devices.
+*
+* Tests show that other operating have similar limits with Microsoft
+* Windows 7 limiting transfers to 128 sectors for both USB2 and USB3
+* and Apple Mac OS X 10.11 limiting transfers to 256 sectors for USB2
+* and 2048 for USB3 devices.
+*/
.max_sectors =  240,
 
/* merge commands... this seems to help performance, but
-- 
2.8.0.rc2

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


[PATCHv5 3/3] usb: storage: fix multi-line comment style

2016-04-18 Thread Felipe Balbi
No functional changes here, just making sure our
storage driver uses a consistent multi-line comment
style.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/storage/alauda.c   |  22 ++-
 drivers/usb/storage/cypress_atacb.c|  34 ++--
 drivers/usb/storage/datafab.c  |  22 ++-
 drivers/usb/storage/debug.c|   3 +-
 drivers/usb/storage/debug.h|   3 +-
 drivers/usb/storage/ene_ub6250.c   |  25 +--
 drivers/usb/storage/freecom.c  |  75 +---
 drivers/usb/storage/initializers.c |  15 +-
 drivers/usb/storage/initializers.h |  15 +-
 drivers/usb/storage/isd200.c   |  51 ++---
 drivers/usb/storage/jumpshot.c |  22 ++-
 drivers/usb/storage/karma.c|   3 +-
 drivers/usb/storage/option_ms.c|   6 +-
 drivers/usb/storage/protocol.c |  12 +-
 drivers/usb/storage/protocol.h |   3 +-
 drivers/usb/storage/realtek_cr.c   |  12 +-
 drivers/usb/storage/scsiglue.c | 147 ++-
 drivers/usb/storage/scsiglue.h |   3 +-
 drivers/usb/storage/sddr09.c   |  82 +---
 drivers/usb/storage/sddr55.c   |  45 +++--
 drivers/usb/storage/shuttle_usbat.c|  16 +-
 drivers/usb/storage/sierra_ms.c|   3 +-
 drivers/usb/storage/transport.c| 165 ++--
 drivers/usb/storage/transport.h|   3 +-
 drivers/usb/storage/uas.c  |   3 +-
 drivers/usb/storage/unusual_alauda.h   |   3 +-
 drivers/usb/storage/unusual_cypress.h  |   3 +-
 drivers/usb/storage/unusual_datafab.h  |   6 +-
 drivers/usb/storage/unusual_devs.h | 334 ++---
 drivers/usb/storage/unusual_freecom.h  |   3 +-
 drivers/usb/storage/unusual_isd200.h   |   3 +-
 drivers/usb/storage/unusual_jumpshot.h |   3 +-
 drivers/usb/storage/unusual_karma.h|   3 +-
 drivers/usb/storage/unusual_onetouch.h |   6 +-
 drivers/usb/storage/unusual_realtek.h  |   3 +-
 drivers/usb/storage/unusual_sddr09.h   |   3 +-
 drivers/usb/storage/unusual_sddr55.h   |   3 +-
 drivers/usb/storage/unusual_uas.h  |   3 +-
 drivers/usb/storage/unusual_usbat.h|   3 +-
 drivers/usb/storage/usb.c  |  98 ++
 drivers/usb/storage/usb.h  |  14 +-
 drivers/usb/storage/usual-tables.c |   3 +-
 42 files changed, 829 insertions(+), 455 deletions(-)

diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c
index 171fa7d793bc..1d8b03c81030 100644
--- a/drivers/usb/storage/alauda.c
+++ b/drivers/usb/storage/alauda.c
@@ -829,8 +829,10 @@ static int alauda_write_lba(struct us_data *us, u16 lba,
 
pba = MEDIA_INFO(us).lba_to_pba[zone][lba_offset];
if (pba == 1) {
-   /* Maybe it is impossible to write to PBA 1.
-  Fake success, but don't do anything. */
+   /*
+* Maybe it is impossible to write to PBA 1.
+* Fake success, but don't do anything.
+*/
printk(KERN_WARNING
   "alauda_write_lba: avoid writing to pba 1\n");
return USB_STOR_TRANSPORT_GOOD;
@@ -977,10 +979,12 @@ static int alauda_read_data(struct us_data *us, unsigned 
long address,
usb_stor_dbg(us, "Read %d zero pages (LBA %d) page 
%d\n",
 pages, lba, page);
 
-   /* This is not really an error. It just means
-  that the block has never been written.
-  Instead of returning USB_STOR_TRANSPORT_ERROR
-  it is better to return all zero data. */
+   /*
+* This is not really an error. It just means
+* that the block has never been written.
+* Instead of returning USB_STOR_TRANSPORT_ERROR
+* it is better to return all zero data.
+*/
 
memset(buffer, 0, len);
} else {
@@ -1222,8 +1226,10 @@ static int alauda_transport(struct scsi_cmnd *srb, 
struct us_data *us)
}
 
if (srb->cmnd[0] == ALLOW_MEDIUM_REMOVAL) {
-   /* sure.  whatever.  not like we can stop the user from popping
-  the media out of the device (no locking doors, etc) */
+   /*
+* sure.  whatever.  not like we can stop the user from popping
+* the media out of the device (no locking doors, etc)
+*/
return USB_STOR_TRANSPORT_GOOD;
}
 
diff --git a/drivers/usb/storage/cypress_atacb.c 
b/drivers/usb/storage/cypress_atacb.c
index c80d3dec9a34..5e4af44d7d9f 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -110,13 +110,17 @@ static void cypress_atacb_passthrough(struct scsi_cmnd 
*srb, struct us_data *us)
/* first build the ATACB command */
srb->cmd_len = 16;
 
-   s

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
Hi!

> >> >>> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
> >> >>
> >> >> According to the spec we should always be talking about unit loads (1
> >> >> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
> >> >> work for SS capable ports and SS gadgets (we have quite a few of them,
> >> >> actually). You're missing the opportunity of charging at 900mA.
> >> >
> >> > I follow the DCP/SDP/CDP/ACA type's default current limitation and
> >> > user can set them what they want.
> >> 
> >> no, the user CANNOT set it to what they want. If you get enumerated
> >> @100mA and the user just decides to set it to 2000mA, s/he could even
> >> melt the USB connector. The kernel _must_ prevent such cases.
> >
> > root should be allowed to do that.
> 
> root should not be allowed to put user at risk. The usb connector is a
> path to earth ground in most (all?) desktop computers. Allowing root to
> put that connector in a situation where it could melt the connector and
> short mains should never be allowed.

Be sure that hardware people put reasonable precautions...

> > Very often, you want to charge using 1.8A from an old desktop PC.
> 
> if that old desktop's port is not a charging port, you shouldn't be
> allowed to do that. Not ever.

Yes, Felipe just decided that I should not be able to charge my N900
in useful way.

> > N900 will simply not charge from .5A.
> 
> it used to with original maemo userspace/kernel.

Yeah, at about 10% per night.

> >> a) you are connected to a dedicated charger
> >> 
> >>In this case, you can get up to 2000mA depending on the charger.
> >> 
> >>If $this charger can give you or not 2000mA is not detectable,
> >>so what do charging ICs do ? They slowly increase the attached
> >>load accross VBUS/GND and measure VBUS value. When IC notices
> >>VBUS dropping bit, step back to previous load.
> >> 
> >>This means you will always charger with maximum rating of DCP.
> >> 
> >>Why would user change this ? More is unsafe, less is just
> >>stupid.
> >
> > Actually, less is not stupid. Charging li-ion battery from li-ion battery 
> > might
> > be stupid. Imagine I'm on train, with device like N900 (50% battery) and 
> > power bank
> > (3Ah). I'm actively using the device. If I let it charge at full current, 
> > I'll waste
> > energy. If I limit current to approximately the power consumption, it will 
> > run the
> > powerbank empty, first, then empty the internal battery, maximizing total 
> > time I
> > can use the device.
> 
> why would you waste energy ? What the charger chip would do is charge
> battery to maximum then just to maintenance charge from that point
> on. Where is energy being wasted other than normal heat dissipation ?

Physics 101, of course wasted energy goes to heat. Lets not waste
energy by charging li-ion from li-ion when it is not required.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
>> > Very often, you want to charge using 1.8A from an old desktop PC.
>> 
>> if that old desktop's port is not a charging port, you shouldn't be
>> allowed to do that. Not ever.
>
> Yes, Felipe just decided that I should not be able to charge my N900
> in useful way.

you can do whatever you want with *your* kernel binary, but we're not
gonna ship something potentially dangerous. If that PC port is telling
you it can only allow 100mA, you should *not* be allowed to overcome
that limitation from the device side, sorry.

>> >> a) you are connected to a dedicated charger
>> >> 
>> >>   In this case, you can get up to 2000mA depending on the charger.
>> >> 
>> >>   If $this charger can give you or not 2000mA is not detectable,
>> >>   so what do charging ICs do ? They slowly increase the attached
>> >>   load accross VBUS/GND and measure VBUS value. When IC notices
>> >>   VBUS dropping bit, step back to previous load.
>> >> 
>> >>   This means you will always charger with maximum rating of DCP.
>> >> 
>> >>   Why would user change this ? More is unsafe, less is just
>> >>   stupid.
>> >
>> > Actually, less is not stupid. Charging li-ion battery from li-ion battery 
>> > might
>> > be stupid. Imagine I'm on train, with device like N900 (50% battery) and 
>> > power bank
>> > (3Ah). I'm actively using the device. If I let it charge at full current, 
>> > I'll waste
>> > energy. If I limit current to approximately the power consumption, it will 
>> > run the
>> > powerbank empty, first, then empty the internal battery, maximizing total 
>> > time I
>> > can use the device.
>> 
>> why would you waste energy ? What the charger chip would do is charge
>> battery to maximum then just to maintenance charge from that point
>> on. Where is energy being wasted other than normal heat dissipation ?
>
> Physics 101, of course wasted energy goes to heat. Lets not waste
> energy by charging li-ion from li-ion when it is not required.

your cellphone has no means to know that it's connected to a Li-Ion
battery. We don't have visibility on what we're connected to, just how
much it can source.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
Hi!

> >> > a) you are connected to a dedicated charger
> >> >
> >> > In this case, you can get up to 2000mA depending on the charger.
> >> >
> >> > If $this charger can give you or not 2000mA is not detectable,
> >> > so what do charging ICs do ? They slowly increase the attached
> >> > load accross VBUS/GND and measure VBUS value. When IC notices
> >> > VBUS dropping bit, step back to previous load.
> >> >
> >> > This means you will always charger with maximum rating of DCP.
> >> >
> >> > Why would user change this ? More is unsafe, less is just
> >> > stupid.
> >
> > Less is not neccessarily stupid. First, it is useful for debugging, second, 
> > you
> > don't know how much this charger can give you. You measured you can get 
> > 1.8A,
> > but the note on the charger says 1.5A. You may want to go with 1.5A.
> >
> > Also, there are several incompatible standards for detecting
> > "dedicated charger". IIRC iPhone has different one from iPad. So it is
> > quite important to be able to control this manually.
> 
> manually ??? Hell no! Charger IC should be able to do this no
> problem. I would be surprised if there's any charger IC out there which
> blindly connects a 1.8A load from the start. What these ICs do is that
> they slowly increment the load and check voltage level. They'll continue
> to do that up to the maximum you listed (1.8A, let's say). As soon as
> voltage drops a bit, charger IC knows that it use previous load.

As I explained, if the note on the wall charger says 1.5A, you want to
do 1.5A, not 1.8A. You can measure voltage on the charger, but you
don't know its temperature.

> >> > d) you are connected to a standard port and get enumerated with your
> >> > 100mA configuration.
> >> >
> >> > you *know* 100mA is okay. So you connect a 100mA load and get it
> >> > over with.
> >> >
> >> > This means you will always charger with maximum rating for this
> >> > SDP.
> >> >
> >> > Why would user change this ? More is unsafe, less is just
> >> > stupid.
> >
> > I've needed to override 100mA default many times. Maybe it is unsafe,
> > but it is useful.
> 
> still unsafe. If you really wanna do that, you're welcome to removing
> safety margins from your own kernel, but we're definitely not going to
> ship this to millions of users.

Not more unsafe than loading wall chargers with "lets see how much we
can get out of it" algorithm. Plus actually required to charge your
machines in useful way. So it is important that common API
exists. Whether it gets enalbed at production is different question.

Unfortunately, there's more than one standard for detecting charger,
so manual control will probably be required.

> > (And with USB 5V connected directly to pretty beefy PC power supply...
> > it is sometimes safer than it looks).
> 
> you're not considering the thermal dissipation on the USB connector
> itself. Many of them might not use good metals because they assume the
> maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
> you could, literally, melt the connector.

If you are dissipating 2.5W at the connector, you are doing something
very wrong. You should not be short-circuiting your USB... even when
the ports are usually designed to survive that.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
On Mon 2016-04-18 13:30:54, Felipe Balbi wrote:
> 
> Hi,
> 
> Pavel Machek  writes:
> >> > Very often, you want to charge using 1.8A from an old desktop PC.
> >> 
> >> if that old desktop's port is not a charging port, you shouldn't be
> >> allowed to do that. Not ever.
> >
> > Yes, Felipe just decided that I should not be able to charge my N900
> > in useful way.
> 
> you can do whatever you want with *your* kernel binary, but we're not
> gonna ship something potentially dangerous. If that PC port is telling
> you it can only allow 100mA, you should *not* be allowed to overcome
> that limitation from the device side, sorry.

Yes, and if your cpu tells you it can only run on 2GHz, you should not
be able to run it on 2.1GHz. And you should not be able to use
non-VESA VGA modes, because you could overheat coils in the monitor.

You are shipping something potentially dangerous already, because you
know, USB-A-to-micro-B cables with D+/D- connected to simulate charger
are actually very common. The world did not end, yet, so it is clearly
not as bad as you make it be.

> >> >> a) you are connected to a dedicated charger
> >> >> 
> >> >> In this case, you can get up to 2000mA depending on the charger.
> >> >> 
> >> >> If $this charger can give you or not 2000mA is not detectable,
> >> >> so what do charging ICs do ? They slowly increase the attached
> >> >> load accross VBUS/GND and measure VBUS value. When IC notices
> >> >> VBUS dropping bit, step back to previous load.
> >> >> 
> >> >> This means you will always charger with maximum rating of DCP.
> >> >> 
> >> >> Why would user change this ? More is unsafe, less is just
> >> >> stupid.
> >> >
> >> > Actually, less is not stupid. Charging li-ion battery from li-ion 
> >> > battery might
> >> > be stupid. Imagine I'm on train, with device like N900 (50% battery) and 
> >> > power bank
> >> > (3Ah). I'm actively using the device. If I let it charge at full 
> >> > current, I'll waste
> >> > energy. If I limit current to approximately the power consumption, it 
> >> > will run the
> >> > powerbank empty, first, then empty the internal battery, maximizing 
> >> > total time I
> >> > can use the device.
> >> 
> >> why would you waste energy ? What the charger chip would do is charge
> >> battery to maximum then just to maintenance charge from that point
> >> on. Where is energy being wasted other than normal heat dissipation ?
> >
> > Physics 101, of course wasted energy goes to heat. Lets not waste
> > energy by charging li-ion from li-ion when it is not required.
> 
> your cellphone has no means to know that it's connected to a Li-Ion
> battery. We don't have visibility on what we're connected to, just how
> much it can source.

But cellphone user knows what he connected his charger to, and that's
why it is useful to be able to lower the current. Even when you said
"less is just stupid" I demonstrated it is not, at least in case when
your power source is a battery.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
> On Mon 2016-04-18 13:30:54, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Pavel Machek  writes:
>> >> > Very often, you want to charge using 1.8A from an old desktop PC.
>> >> 
>> >> if that old desktop's port is not a charging port, you shouldn't be
>> >> allowed to do that. Not ever.
>> >
>> > Yes, Felipe just decided that I should not be able to charge my N900
>> > in useful way.
>> 
>> you can do whatever you want with *your* kernel binary, but we're not
>> gonna ship something potentially dangerous. If that PC port is telling
>> you it can only allow 100mA, you should *not* be allowed to overcome
>> that limitation from the device side, sorry.
>
> Yes, and if your cpu tells you it can only run on 2GHz, you should not
> be able to run it on 2.1GHz. And you should not be able to use
> non-VESA VGA modes, because you could overheat coils in the monitor.

heh, yada-yada-yada.

> You are shipping something potentially dangerous already, because you
> know, USB-A-to-micro-B cables with D+/D- connected to simulate charger
> are actually very common. The world did not end, yet, so it is clearly
> not as bad as you make it be.

That's not even a supported cable assembly. If you break your HW, that's
your own fault for not using proper cables. There's nothing the kernel
can do about that anyway.

If you're willing to by a "special" cable just to overcome safety limits
set by the various USB specifications, go head. But don't ask me to
support you when things go wrong.

>> >> >> a) you are connected to a dedicated charger
>> >> >> 
>> >> >>In this case, you can get up to 2000mA depending on the charger.
>> >> >> 
>> >> >>If $this charger can give you or not 2000mA is not detectable,
>> >> >>so what do charging ICs do ? They slowly increase the attached
>> >> >>load accross VBUS/GND and measure VBUS value. When IC notices
>> >> >>VBUS dropping bit, step back to previous load.
>> >> >> 
>> >> >>This means you will always charger with maximum rating of DCP.
>> >> >> 
>> >> >>Why would user change this ? More is unsafe, less is just
>> >> >>stupid.
>> >> >
>> >> > Actually, less is not stupid. Charging li-ion battery from li-ion 
>> >> > battery might
>> >> > be stupid. Imagine I'm on train, with device like N900 (50% battery) 
>> >> > and power bank
>> >> > (3Ah). I'm actively using the device. If I let it charge at full 
>> >> > current, I'll waste
>> >> > energy. If I limit current to approximately the power consumption, it 
>> >> > will run the
>> >> > powerbank empty, first, then empty the internal battery, maximizing 
>> >> > total time I
>> >> > can use the device.
>> >> 
>> >> why would you waste energy ? What the charger chip would do is charge
>> >> battery to maximum then just to maintenance charge from that point
>> >> on. Where is energy being wasted other than normal heat dissipation ?
>> >
>> > Physics 101, of course wasted energy goes to heat. Lets not waste
>> > energy by charging li-ion from li-ion when it is not required.
>> 
>> your cellphone has no means to know that it's connected to a Li-Ion
>> battery. We don't have visibility on what we're connected to, just how
>> much it can source.
>
> But cellphone user knows what he connected his charger to, and that's
> why it is useful to be able to lower the current. Even when you said
> "less is just stupid" I demonstrated it is not, at least in case when
> your power source is a battery.

okay, so let's do this. How much more "time" do you get by doing this ?
Without numbers showing that it's considerably better, we can't do
anything.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Felipe Balbi  writes:
>> But cellphone user knows what he connected his charger to, and that's
>> why it is useful to be able to lower the current. Even when you said
>> "less is just stupid" I demonstrated it is not, at least in case when

and btw, you haven't demonstrated anything. You merely stated that it
isn't without references or numbers, or any source of trustworthy
information. I'm not really into 'believing'.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
Hi!

> >> manually ??? Hell no! Charger IC should be able to do this no
> >> problem. I would be surprised if there's any charger IC out there which
> >> blindly connects a 1.8A load from the start. What these ICs do is that
> >> they slowly increment the load and check voltage level. They'll continue
> >> to do that up to the maximum you listed (1.8A, let's say). As soon as
> >> voltage drops a bit, charger IC knows that it use previous load.
> >
> > As I explained, if the note on the wall charger says 1.5A, you want to
> > do 1.5A, not 1.8A. You can measure voltage on the charger, but you
> > don't know its temperature.
> 
> phone can't read what it says in the wall charger, nor can it know that
> it's connected charger ABC and not charger XYZ. Think of the user
> experience. You can't expect users to tell you "okay phone, the charger
> reads that maximum is 1.5A, so please don't go over that."

Of course, we may do something sensible by default. But manual
controls should still be present. You called them "stupid" but they
are not.

Note that just because you detected wall charger does not even mean
you are connected to wall charger. See the link below.

> >> still unsafe. If you really wanna do that, you're welcome to removing
> >> safety margins from your own kernel, but we're definitely not going to
> >> ship this to millions of users.
> >
> > Not more unsafe than loading wall chargers with "lets see how much we
> > can get out of it" algorithm. Plus actually required to charge your
> 
> it actually _is_ more unsafe. You could burn mother boards with that. If
> host tells you it only has 100mA power budget left, why are you trying
> to get more ?

No, you can't burn motherboard like that... You can force emergency
shutdowns, which is also bad, but... There are many devices that break
this aspect of USB protocol.

https://www.kickstarter.com/projects/1785889318/doubbletime-charging-cable-full-battery-in-1-2-the

> > Unfortunately, there's more than one standard for detecting charger,
> > so manual control will probably be required.
> 
> n900 never had manual control of anything. It was just using information
> given by the battery IC, charger IC and twl4030 madc.

Manual control of n900 charging is done by:

echo 1800 > /sys/class/power_supply/bq24150a-0/current_limit

> >> > (And with USB 5V connected directly to pretty beefy PC power supply...
> >> > it is sometimes safer than it looks).
> >> 
> >> you're not considering the thermal dissipation on the USB connector
> >> itself. Many of them might not use good metals because they assume the
> >> maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
> >> you could, literally, melt the connector.
> >
> > If you are dissipating 2.5W at the connector, you are doing something
> > very wrong. You should not be short-circuiting your USB... even when
> > the ports are usually designed to survive that.
> 
> yes. You shouldn't. You also shouldn't go over that limit. If you have a
> 500mA total power budget, we should not let anybody try to draw more
> because we have no control over what's on the side of the wire.

They already can go over the limit, for example using cable linked
above. I have several such cables here. I also have various wall
supplies that are not detected as a wall supply by N900. So I either
have to remember and connect them with the "special" cable, or
(easier) use the override above to get useful charging.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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


IMPORTANT MAIL TO YOU

2016-04-18 Thread verifelaw
I am Capt. Lawrence Tyman, an officer in US Army,and also a West Point
Graduate, serving in the Military with the 82nd Air Borne Division
Peace keeping force deployed from Afganistan to Syria.  
We were moved to Syria from Iraq as the last batch just left,and i
really need your help in assisting me with the safe keeping of 1 military
trunk box contain funds amount of $10.2M which i secured on a raiding we 
carried out in 
January in one of the chief Syrian IsIs base which i headed the squard as the 
Captain.  With every possible arrangement to lift this box out, is intended to 
arrive 
Belgium from there a diplomat will deliver it to your designated location
I hope you can be trusted? You will be rewarded handsomely if you could help
me secure the funds until I conclude my service here in 3 month to meet you 
while we can 
plan head to head on a good and profitable business or company i can invest my 
funds in your country.
If you can be trusted and willing to support me in securing this safely kindly 
indicate 
by Letting me know this (1) Your name (2) Your address (3) Age (4) Occupation 
and 
i will explain further when i get a response from you
kindly contact me in this my private email address below: lawrencety...@gmx.com

Regards,
Capt. Lawrence Tyman
--
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 v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad controller binding

2016-04-18 Thread Thierry Reding
On Tue, Mar 15, 2016 at 10:01:25AM +0100, Linus Walleij wrote:
> On Fri, Mar 4, 2016 at 5:19 PM, Thierry Reding  
> wrote:
> 
> > From: Thierry Reding 
> >
> > This is an old version of the binding that isn't flexible enough to
> > describe all aspects of the XUSB pad controller. Specifically with the
> > addition of XUSB support (for SuperSpeed USB) the existing binding is
> > no longer suitable.
> >
> > Signed-off-by: Thierry Reding 
> 
> That's unfortunate, not to say unelegant. I want to know Stephen's
> opinion on these patches (probably they are in another mail)
> before merging.
> 
> Will the new binding also work with SuperDuperSpeed USB and
> SuperSuperMegaUltraOrtonSpeed USB I wonder... or will we
> change the bindings again?

Linus, Stephen's gave his Acked-by on this patch (provided it gets
merged into patch 1/9, which I've done locally), would you be willing to
give your Acked-by so that this change can go in via the PHY tree where
the new bindings are introduced?

Thanks,
Thierry


signature.asc
Description: PGP signature


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
On Mon 2016-04-18 13:55:17, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Balbi  writes:
> >> But cellphone user knows what he connected his charger to, and that's
> >> why it is useful to be able to lower the current. Even when you said
> >> "less is just stupid" I demonstrated it is not, at least in case when
> 
> and btw, you haven't demonstrated anything. You merely stated that it
> isn't without references or numbers, or any source of trustworthy
> information. I'm not really into 'believing'.

You are not really into reading, either, it seems. Or into the
electronics. Or into physics.

You seem to understand that charging li-ion from li-ion produces too
much heat. (And yes, it does, DC-DC convertors are not 100%
effective). Converting energy into heat is not a good idea.

If you need numbers or references to understand basic physics, you'll
need to google it yourself, I'm afraid. 

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
>> >> > a) you are connected to a dedicated charger
>> >> >
>> >> > In this case, you can get up to 2000mA depending on the charger.
>> >> >
>> >> > If $this charger can give you or not 2000mA is not detectable,
>> >> > so what do charging ICs do ? They slowly increase the attached
>> >> > load accross VBUS/GND and measure VBUS value. When IC notices
>> >> > VBUS dropping bit, step back to previous load.
>> >> >
>> >> > This means you will always charger with maximum rating of DCP.
>> >> >
>> >> > Why would user change this ? More is unsafe, less is just
>> >> > stupid.
>> >
>> > Less is not neccessarily stupid. First, it is useful for debugging, 
>> > second, you
>> > don't know how much this charger can give you. You measured you can get 
>> > 1.8A,
>> > but the note on the charger says 1.5A. You may want to go with 1.5A.
>> >
>> > Also, there are several incompatible standards for detecting
>> > "dedicated charger". IIRC iPhone has different one from iPad. So it is
>> > quite important to be able to control this manually.
>> 
>> manually ??? Hell no! Charger IC should be able to do this no
>> problem. I would be surprised if there's any charger IC out there which
>> blindly connects a 1.8A load from the start. What these ICs do is that
>> they slowly increment the load and check voltage level. They'll continue
>> to do that up to the maximum you listed (1.8A, let's say). As soon as
>> voltage drops a bit, charger IC knows that it use previous load.
>
> As I explained, if the note on the wall charger says 1.5A, you want to
> do 1.5A, not 1.8A. You can measure voltage on the charger, but you
> don't know its temperature.

phone can't read what it says in the wall charger, nor can it know that
it's connected charger ABC and not charger XYZ. Think of the user
experience. You can't expect users to tell you "okay phone, the charger
reads that maximum is 1.5A, so please don't go over that."

>> >> > d) you are connected to a standard port and get enumerated with your
>> >> > 100mA configuration.
>> >> >
>> >> > you *know* 100mA is okay. So you connect a 100mA load and get it
>> >> > over with.
>> >> >
>> >> > This means you will always charger with maximum rating for this
>> >> > SDP.
>> >> >
>> >> > Why would user change this ? More is unsafe, less is just
>> >> > stupid.
>> >
>> > I've needed to override 100mA default many times. Maybe it is unsafe,
>> > but it is useful.
>> 
>> still unsafe. If you really wanna do that, you're welcome to removing
>> safety margins from your own kernel, but we're definitely not going to
>> ship this to millions of users.
>
> Not more unsafe than loading wall chargers with "lets see how much we
> can get out of it" algorithm. Plus actually required to charge your

it actually _is_ more unsafe. You could burn mother boards with that. If
host tells you it only has 100mA power budget left, why are you trying
to get more ?

> machines in useful way. So it is important that common API
> exists. Whether it gets enalbed at production is different question.

as I said, if you wanna do some unsafe manual method, be my guest; but
I'm not convinced that every user of the linux kernel wants that in
their pockets.

> Unfortunately, there's more than one standard for detecting charger,
> so manual control will probably be required.

n900 never had manual control of anything. It was just using information
given by the battery IC, charger IC and twl4030 madc.

>> > (And with USB 5V connected directly to pretty beefy PC power supply...
>> > it is sometimes safer than it looks).
>> 
>> you're not considering the thermal dissipation on the USB connector
>> itself. Many of them might not use good metals because they assume the
>> maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
>> you could, literally, melt the connector.
>
> If you are dissipating 2.5W at the connector, you are doing something
> very wrong. You should not be short-circuiting your USB... even when
> the ports are usually designed to survive that.

yes. You shouldn't. You also shouldn't go over that limit. If you have a
500mA total power budget, we should not let anybody try to draw more
because we have no control over what's on the side of the wire.

-- 
balbi


signature.asc
Description: PGP signature


RE: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread David Laight
From: Pavel Machek
> Sent: 18 April 2016 11:40
...
> > >> > Actually, less is not stupid. Charging li-ion battery from li-ion 
> > >> > battery might
> > >> > be stupid. Imagine I'm on train, with device like N900 (50% battery) 
> > >> > and power bank
> > >> > (3Ah). I'm actively using the device. If I let it charge at full 
> > >> > current, I'll waste
> > >> > energy. If I limit current to approximately the power consumption, it 
> > >> > will run the
> > >> > powerbank empty, first, then empty the internal battery, maximizing 
> > >> > total time I
> > >> > can use the device.
> > >>
> > >> why would you waste energy ? What the charger chip would do is charge
> > >> battery to maximum then just to maintenance charge from that point
> > >> on. Where is energy being wasted other than normal heat dissipation ?
> > >
> > > Physics 101, of course wasted energy goes to heat. Lets not waste
> > > energy by charging li-ion from li-ion when it is not required.
> >
> > your cellphone has no means to know that it's connected to a Li-Ion
> > battery. We don't have visibility on what we're connected to, just how
> > much it can source.
> 
> But cellphone user knows what he connected his charger to, and that's
> why it is useful to be able to lower the current. Even when you said
> "less is just stupid" I demonstrated it is not, at least in case when
> your power source is a battery.

It reality you may want the phone/tablet to be configurable to take power
from USB, but disable the li-ion charging circuit.
That will maximise the time you get when running from an external battery.
I connect my tablet to the 1A output (which discharges the internal battery
slowly) rather than the 2A one (which will charge it with some cables).

Getting 2A from a USB charger seems to be very device/cable/charger dependant.
We've two Samsung chargers that look similar, but have completely different
characteristics. Both claim 2A at 5v (one says 5.3v), one claims 1.5A at
something like 12v as well - not sure we should be using that one for
'random' devices.

David

--
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 v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
Hi!

On Mon 2016-04-18 10:59:23, David Laight wrote:
> From: Pavel Machek
> > Sent: 18 April 2016 11:40
> ...
> > > >> > Actually, less is not stupid. Charging li-ion battery from li-ion 
> > > >> > battery might
> > > >> > be stupid. Imagine I'm on train, with device like N900 (50% battery) 
> > > >> > and power bank
> > > >> > (3Ah). I'm actively using the device. If I let it charge at full 
> > > >> > current, I'll waste
> > > >> > energy. If I limit current to approximately the power consumption, 
> > > >> > it will run the
> > > >> > powerbank empty, first, then empty the internal battery, maximizing 
> > > >> > total time I
> > > >> > can use the device.
> > > >>
> > > >> why would you waste energy ? What the charger chip would do is charge
> > > >> battery to maximum then just to maintenance charge from that point
> > > >> on. Where is energy being wasted other than normal heat dissipation ?
> > > >
> > > > Physics 101, of course wasted energy goes to heat. Lets not waste
> > > > energy by charging li-ion from li-ion when it is not required.
> > >
> > > your cellphone has no means to know that it's connected to a Li-Ion
> > > battery. We don't have visibility on what we're connected to, just how
> > > much it can source.
> > 
> > But cellphone user knows what he connected his charger to, and that's
> > why it is useful to be able to lower the current. Even when you said
> > "less is just stupid" I demonstrated it is not, at least in case when
> > your power source is a battery.
> 
> It reality you may want the phone/tablet to be configurable to take power
> from USB, but disable the li-ion charging circuit.
> That will maximise the time you get when running from an external battery.
> I connect my tablet to the 1A output (which discharges the internal battery
> slowly) rather than the 2A one (which will charge it with some
> cables).

Yes, being able to power device from external without charging the
battery is useful, too.

But I'd still like to control individual currents, too. If I have
power bank and two devices, I may want to select which one charges
faster.

Best regards,   
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad controller binding

2016-04-18 Thread Linus Walleij
On Mon, Apr 18, 2016 at 1:12 PM, Thierry Reding
 wrote:
> On Tue, Mar 15, 2016 at 10:01:25AM +0100, Linus Walleij wrote:
>> On Fri, Mar 4, 2016 at 5:19 PM, Thierry Reding  
>> wrote:
>>
>> > From: Thierry Reding 
>> >
>> > This is an old version of the binding that isn't flexible enough to
>> > describe all aspects of the XUSB pad controller. Specifically with the
>> > addition of XUSB support (for SuperSpeed USB) the existing binding is
>> > no longer suitable.
>> >
>> > Signed-off-by: Thierry Reding 
>>
>> That's unfortunate, not to say unelegant. I want to know Stephen's
>> opinion on these patches (probably they are in another mail)
>> before merging.
>>
>> Will the new binding also work with SuperDuperSpeed USB and
>> SuperSuperMegaUltraOrtonSpeed USB I wonder... or will we
>> change the bindings again?
>
> Linus, Stephen's gave his Acked-by on this patch (provided it gets
> merged into patch 1/9, which I've done locally), would you be willing to
> give your Acked-by so that this change can go in via the PHY tree where
> the new bindings are introduced?

Yes I trust Stephen to know what's right for Tegra.
Acked-by: Linus Walleij 

Yours,
Linus Walleij
--
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 v10 2/9] dt-bindings: pinctrl: Deprecate Tegra XUSB pad controller binding

2016-04-18 Thread Thierry Reding
On Tue, Mar 15, 2016 at 10:01:25AM +0100, Linus Walleij wrote:
> On Fri, Mar 4, 2016 at 5:19 PM, Thierry Reding  
> wrote:
> 
> > From: Thierry Reding 
> >
> > This is an old version of the binding that isn't flexible enough to
> > describe all aspects of the XUSB pad controller. Specifically with the
> > addition of XUSB support (for SuperSpeed USB) the existing binding is
> > no longer suitable.
> >
> > Signed-off-by: Thierry Reding 
> 
> That's unfortunate, not to say unelegant. I want to know Stephen's
> opinion on these patches (probably they are in another mail)
> before merging.
> 
> Will the new binding also work with SuperDuperSpeed USB and
> SuperSuperMegaUltraOrtonSpeed USB I wonder... or will we
> change the bindings again?

To answer this question: yes, I think that it will be flexible enough to
support future generations of this IP, provided that the design doesn't
change in a fundamental way.

The change from the pinctrl-based bindings to this new one is that each
lane provided by the XUSB pad controller is exposed as a separate PHY
object and hence allows fine-grained control over when which lane is
powered up. It also allows each lane to be easily configured because it
has its own device tree node.

Thierry


signature.asc
Description: PGP signature


Re: [PATCH v10 4/9] phy: Add Tegra XUSB pad controller support

2016-04-18 Thread Thierry Reding
On Fri, Mar 04, 2016 at 05:19:34PM +0100, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Add a new driver for the XUSB pad controller found on NVIDIA Tegra SoCs.
> This hardware block used to be exposed as a pin controller, but it turns
> out that this isn't a good fit. The new driver and DT binding much more
> accurately describe the hardware and are more flexible in supporting new
> SoC generations.
> 
> Signed-off-by: Thierry Reding 
> ---
> Changes in v9:
> - export public API for direct use by the xHCI driver (replaces mailbox
>   API which had introduced a nasty circular dependency)
> 
>  drivers/phy/Kconfig|2 +
>  drivers/phy/Makefile   |2 +
>  drivers/phy/tegra/Kconfig  |8 +
>  drivers/phy/tegra/Makefile |5 +
>  drivers/phy/tegra/xusb-tegra124.c  | 1747 
> 
>  drivers/phy/tegra/xusb.c   | 1017 
>  drivers/phy/tegra/xusb.h   |  421 +++
>  drivers/pinctrl/tegra/pinctrl-tegra-xusb.c |   20 +-

Hi Linus,

the changes to the existing pinctrl driver here would need an Acked-by
from you as well. Effectively this turns the pinctrl driver into library
code that is used by the PHY driver to preserve backwards-compatibility
with older bindings.

Here's the hunk that does this:

[...]
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c 
> b/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c
> index 2f06029c9405..946cda3fee35 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c
> @@ -873,7 +873,7 @@ static const struct of_device_id 
> tegra_xusb_padctl_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, tegra_xusb_padctl_of_match);
>  
> -static int tegra_xusb_padctl_probe(struct platform_device *pdev)
> +int tegra_xusb_padctl_legacy_probe(struct platform_device *pdev)
>  {
>   struct tegra_xusb_padctl *padctl;
>   const struct of_device_id *match;
> @@ -955,8 +955,9 @@ reset:
>   reset_control_assert(padctl->rst);
>   return err;
>  }
> +EXPORT_SYMBOL_GPL(tegra_xusb_padctl_legacy_probe);
>  
> -static int tegra_xusb_padctl_remove(struct platform_device *pdev)
> +int tegra_xusb_padctl_legacy_remove(struct platform_device *pdev)
>  {
>   struct tegra_xusb_padctl *padctl = platform_get_drvdata(pdev);
>   int err;
> @@ -969,17 +970,4 @@ static int tegra_xusb_padctl_remove(struct 
> platform_device *pdev)
>  
>   return err;
>  }
> -
> -static struct platform_driver tegra_xusb_padctl_driver = {
> - .driver = {
> - .name = "tegra-xusb-padctl",
> - .of_match_table = tegra_xusb_padctl_of_match,
> - },
> - .probe = tegra_xusb_padctl_probe,
> - .remove = tegra_xusb_padctl_remove,
> -};
> -module_platform_driver(tegra_xusb_padctl_driver);
> -
> -MODULE_AUTHOR("Thierry Reding ");
> -MODULE_DESCRIPTION("Tegra 124 XUSB Pad Control driver");
> -MODULE_LICENSE("GPL v2");
> +EXPORT_SYMBOL_GPL(tegra_xusb_padctl_legacy_remove);

Since this merely implements the binding change, does your Acked-by on
the binding apply to this part as well?

Thanks,
Thierry


signature.asc
Description: PGP signature


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
> On Mon 2016-04-18 13:55:17, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Felipe Balbi  writes:
>> >> But cellphone user knows what he connected his charger to, and that's
>> >> why it is useful to be able to lower the current. Even when you said
>> >> "less is just stupid" I demonstrated it is not, at least in case when
>> 
>> and btw, you haven't demonstrated anything. You merely stated that it
>> isn't without references or numbers, or any source of trustworthy
>> information. I'm not really into 'believing'.
>
> You are not really into reading, either, it seems. Or into the
> electronics. Or into physics.
>
> You seem to understand that charging li-ion from li-ion produces too
> much heat. (And yes, it does, DC-DC convertors are not 100%
> effective). Converting energy into heat is not a good idea.
>
> If you need numbers or references to understand basic physics, you'll
> need to google it yourself, I'm afraid. 

still doesn't change the fact that you haven't demonstrated anything. If
you can't be helpful and/or constructive, you may also go away and use
whatever unsafe setup you want to use.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH v10 3/9] dt-bindings: phy: tegra-xusb-padctl: Add Tegra210 support

2016-04-18 Thread Thierry Reding
On Wed, Apr 06, 2016 at 07:08:24PM +0200, Thierry Reding wrote:
[...]
> I attached what I came up with. It extends the OF PHY provider registry
> by allowing an additional node to be specified that if specified will
> serve as the parent for the child lookup (and hence overrides the
> default node that's taken from the struct device).
> 
> It is a fairly trivial patch, and you'll notice the bulk of the changes
> is adding the additional parameter in a number of different places. The
> only thing I'm not quite happy about is that we start needing to pass a
> fairly large number of arguments. But perhaps it's still okay.
> 
> An alternative would be to make struct phy_provider embeddable into
> driver private structures. That way they could be completely initialized
> by a driver before being passed to the __of_phy_provider_register()
> function (much like struct gpio_chip and others). That would be a fairly
> intrusive change, one that I'd be willing to take on, though I'd like to
> have Kishon's opinion on this before going ahead.
> 
> For reference, here's what I'm imagining:
> 
>   struct foo_phy_provider {
>   struct phy_provider base;
>   ...
>   };
> 
>   int foo_probe(struct platform_device *pdev)
>   {
>   struct foo_phy_provider *priv;
>   ...
> 
>   priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>   if (!priv)
>   return -ENOMEM;
> 
>   priv->base.dev = &pdev->dev;
>   priv->base.of_xlate = foo_of_xlate;
> 
>   err = devm_of_phy_provider_register(&priv->base);
>   if (err < 0)
>   return err;
> 
>   ...
>   }
> 
> And of course adapt the signature of the __of_phy_provider_register()
> function and remove the allocation from it.
> 
> Thierry
> 
> --- >8 ---
> From 15e5348a1a63837efd00309fdce5cda979498f77 Mon Sep 17 00:00:00 2001
> From: Thierry Reding 
> Date: Tue, 5 Apr 2016 17:17:34 +0200
> Subject: [PATCH] phy: core: Allow children node to be overridden
> 
> In order to more flexibly support device tree bindings, allow drivers to
> override the container of the child nodes. By default the device node of
> the PHY provider is assumed to be the parent for children, but bindings
> may decide to add additional levels for better organization.
> 
> Signed-off-by: Thierry Reding 
> ---
>  Documentation/phy.txt   | 16 ++--
>  drivers/phy/phy-core.c  | 27 +--
>  include/linux/phy/phy.h | 31 +--
>  3 files changed, 56 insertions(+), 18 deletions(-)

Hi Kishon,

I'd like to take this through the Tegra tree along with the remainder of
the XUSB pad controller and XHCI patches that depend on it. The nice
thing is that it's fairly unintrusive but gives people an easy way to
support more flexible bindings by allowing the OF node to be overridden
when registering the PHY provider.

Are you okay with the change below? If so, could you provide an
Acked-by? I can provide a stable branch for you to pull into the PHY
tree that I'd like to use as a base for the remainder of the series.
I did a fair amount of compile-testing and upon inspection the API that
the patch modifies isn't called in many places, everyone uses the
convenience macros. The stable branch will be helpful in fixing up any
new users, should any appear during the development cycle.

Thanks,
Thierry

> diff --git a/Documentation/phy.txt b/Documentation/phy.txt
> index b388c5af9e72..0aa994bd9a91 100644
> --- a/Documentation/phy.txt
> +++ b/Documentation/phy.txt
> @@ -31,16 +31,28 @@ should provide its own implementation of of_xlate. 
> of_xlate is used only for
>  dt boot case.
>  
>  #define of_phy_provider_register(dev, xlate)\
> -__of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +__of_phy_provider_register((dev), NULL, THIS_MODULE, (xlate))
>  
>  #define devm_of_phy_provider_register(dev, xlate)   \
> -__devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
> +__devm_of_phy_provider_register((dev), NULL, THIS_MODULE, (xlate))
>  
>  of_phy_provider_register and devm_of_phy_provider_register macros can be 
> used to
>  register the phy_provider and it takes device and of_xlate as
>  arguments. For the dt boot case, all PHY providers should use one of the 
> above
>  2 macros to register the PHY provider.
>  
> +Often the device tree nodes associated with a PHY provider will contain a set
> +of children that each represent a single PHY. Some bindings may nest the 
> child
> +nodes within extra levels for context and extensibility, in which case the 
> low
> +level of_phy_provider_register_full() and 
> devm_of_phy_provider_register_full()
> +macros can be used to override the node containing the children.
> +
> +#define of_phy_provider_register_full(dev, children, xlate) \
> + __of_phy_provider_register(dev, children, THIS_MODULE, 

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
>> >> manually ??? Hell no! Charger IC should be able to do this no
>> >> problem. I would be surprised if there's any charger IC out there which
>> >> blindly connects a 1.8A load from the start. What these ICs do is that
>> >> they slowly increment the load and check voltage level. They'll continue
>> >> to do that up to the maximum you listed (1.8A, let's say). As soon as
>> >> voltage drops a bit, charger IC knows that it use previous load.
>> >
>> > As I explained, if the note on the wall charger says 1.5A, you want to
>> > do 1.5A, not 1.8A. You can measure voltage on the charger, but you
>> > don't know its temperature.
>> 
>> phone can't read what it says in the wall charger, nor can it know that
>> it's connected charger ABC and not charger XYZ. Think of the user
>> experience. You can't expect users to tell you "okay phone, the charger
>> reads that maximum is 1.5A, so please don't go over that."
>
> Of course, we may do something sensible by default. But manual
> controls should still be present. You called them "stupid" but they
> are not.
>
> Note that just because you detected wall charger does not even mean
> you are connected to wall charger. See the link below.

that's a horrible product. So what ? If you want to use that, be my
guest. Just, again, don't ask for support when things start falling
apart ;-)

>> >> still unsafe. If you really wanna do that, you're welcome to removing
>> >> safety margins from your own kernel, but we're definitely not going to
>> >> ship this to millions of users.
>> >
>> > Not more unsafe than loading wall chargers with "lets see how much we
>> > can get out of it" algorithm. Plus actually required to charge your
>> 
>> it actually _is_ more unsafe. You could burn mother boards with that. If
>> host tells you it only has 100mA power budget left, why are you trying
>> to get more ?
>
> No, you can't burn motherboard like that... You can force emergency
> shutdowns, which is also bad, but... There are many devices that break
> this aspect of USB protocol.
>
> https://www.kickstarter.com/projects/1785889318/doubbletime-charging-cable-full-battery-in-1-2-the

we can't prevent people from coming up with bad devices/cables/whatever,
right ? But we can make sure that overcoming a 500mA power budget on a
USB 2.0 port will not be allowed.

>> > Unfortunately, there's more than one standard for detecting charger,
>> > so manual control will probably be required.
>> 
>> n900 never had manual control of anything. It was just using information
>> given by the battery IC, charger IC and twl4030 madc.
>
> Manual control of n900 charging is done by:
>
> echo 1800 > /sys/class/power_supply/bq24150a-0/current_limit

yes, that's fine. And if you're connected to a dedicated charger (DCP)
which follows USB Battery Charger specification, we *know* that it
should, per the spec, source up to 2A, so this is fine.

However, if you're connected to SDP (regular PC port), which has a power
budget of 500mA per-port (meaning, that if you're behind a bus powered
hub, you can't even get 500mA), then this write() of yours should be
invalid. It should *not* be a successful write() as that creates an
unsafe and potentially dangerous scenario for the user.

>> >> > (And with USB 5V connected directly to pretty beefy PC power supply...
>> >> > it is sometimes safer than it looks).
>> >> 
>> >> you're not considering the thermal dissipation on the USB connector
>> >> itself. Many of them might not use good metals because they assume the
>> >> maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
>> >> you could, literally, melt the connector.
>> >
>> > If you are dissipating 2.5W at the connector, you are doing something
>> > very wrong. You should not be short-circuiting your USB... even when
>> > the ports are usually designed to survive that.
>> 
>> yes. You shouldn't. You also shouldn't go over that limit. If you have a
>> 500mA total power budget, we should not let anybody try to draw more
>> because we have no control over what's on the side of the wire.
>
> They already can go over the limit, for example using cable linked
> above. I have several such cables here. I also have various wall

people can use unsupported cable assemblies if they want, but you can't
expect kernel to support you.

> supplies that are not detected as a wall supply by N900. So I either
> have to remember and connect them with the "special" cable, or
> (easier) use the override above to get useful charging.

those supplies are not supported by N900. N900 was designed with USB
Battery Chaging specification in mind and Nokia is not around anymore to
give you SW updates. Sorry, but that's not the kernel's fault.

The point is the following: there are a handful of people who would
*know* how to fiddle with these limits, many would not. The vast
majority would not. And, considering this is something completely out of
spec, and, again, potentially dangerous to the user, we are 

[PATCH] cp210x: Add ID for Link ECU

2016-04-18 Thread Mike Manning

The Link ECU is an aftermarket ECU computer for vehicles that provides full 
tuning abilities as well as datalogging and displaying capabilities via the USB 
to Serial adapter built into the device.

Signed-off-by: Mike Manning 
---
 drivers/usb/serial/cp210x.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index bdc0f2f..7f45d00 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -140,6 +140,8 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x10C4, 0xF004) }, /* Elan Digital Systems USBcount50 */
 	{ USB_DEVICE(0x10C5, 0xEA61) }, /* Silicon Labs MobiData GPRS USB Modem */
 	{ USB_DEVICE(0x10CE, 0xEA6A) }, /* Silicon Labs MobiData GPRS USB Modem 100EU */
+	{ USB_DEVICE(0x12B8, 0xEC60) }, /* Link G4 ECU */
+	{ USB_DEVICE(0x12B8, 0xEC62) }, /* Link G4+ ECU */
 	{ USB_DEVICE(0x13AD, 0x) }, /* Baltech card reader */
 	{ USB_DEVICE(0x1555, 0x0004) }, /* Owen AC4 USB-RS485 Converter */
 	{ USB_DEVICE(0x166A, 0x0201) }, /* Clipsal 5500PACA C-Bus Pascal Automation Controller */


Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
On Mon 2016-04-18 14:42:58, Felipe Balbi wrote:
> 
> Hi,
> 
> Pavel Machek  writes:
> > On Mon 2016-04-18 13:55:17, Felipe Balbi wrote:
> >> 
> >> Hi,
> >> 
> >> Felipe Balbi  writes:
> >> >> But cellphone user knows what he connected his charger to, and that's
> >> >> why it is useful to be able to lower the current. Even when you said
> >> >> "less is just stupid" I demonstrated it is not, at least in case when
> >> 
> >> and btw, you haven't demonstrated anything. You merely stated that it
> >> isn't without references or numbers, or any source of trustworthy
> >> information. I'm not really into 'believing'.
> >
> > You are not really into reading, either, it seems. Or into the
> > electronics. Or into physics.
> >
> > You seem to understand that charging li-ion from li-ion produces too
> > much heat. (And yes, it does, DC-DC convertors are not 100%
> > effective). Converting energy into heat is not a good idea.
> >
> > If you need numbers or references to understand basic physics, you'll
> > need to google it yourself, I'm afraid. 
> 
> still doesn't change the fact that you haven't demonstrated anything. If
> you can't be helpful and/or constructive, you may also go away and use
> whatever unsafe setup you want to use.

So you still claim that it is "stupid" to charge at anything lower
than maximum allowed current?

Even after I described the example with device running off power bank
on a train? Would you explain your reasoning?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Pavel Machek
Hi!

> > Of course, we may do something sensible by default. But manual
> > controls should still be present. You called them "stupid" but they
> > are not.
> >
> > Note that just because you detected wall charger does not even mean
> > you are connected to wall charger. See the link below.
> 
> that's a horrible product. So what ? If you want to use that, be my
> guest. Just, again, don't ask for support when things start falling
> apart ;-)

So you have USB spec? So what? There are many such products out there,
and I have at least two such cables here.

They did not cause end of the world, yet, and they are actually very useful.

> >> >> still unsafe. If you really wanna do that, you're welcome to removing
> >> >> safety margins from your own kernel, but we're definitely not going to
> >> >> ship this to millions of users.
> >> >
> >> > Not more unsafe than loading wall chargers with "lets see how much we
> >> > can get out of it" algorithm. Plus actually required to charge your
> >> 
> >> it actually _is_ more unsafe. You could burn mother boards with that. If
> >> host tells you it only has 100mA power budget left, why are you trying
> >> to get more ?
> >
> > No, you can't burn motherboard like that... You can force emergency
> > shutdowns, which is also bad, but... There are many devices that break
> > this aspect of USB protocol.
> >
> > https://www.kickstarter.com/projects/1785889318/doubbletime-charging-cable-full-battery-in-1-2-the
> 
> we can't prevent people from coming up with bad devices/cables/whatever,
> right ? But we can make sure that overcoming a 500mA power budget on a
> USB 2.0 port will not be allowed.

No, you can't, because you don't know if you are connected to USB 2.0
port. (Because non-compliant cables exist).

> >> > Unfortunately, there's more than one standard for detecting charger,
> >> > so manual control will probably be required.
> >> 
> >> n900 never had manual control of anything. It was just using information
> >> given by the battery IC, charger IC and twl4030 madc.
> >
> > Manual control of n900 charging is done by:
> >
> > echo 1800 > /sys/class/power_supply/bq24150a-0/current_limit
> 
> yes, that's fine. And if you're connected to a dedicated charger (DCP)
> which follows USB Battery Charger specification, we *know* that it
> should, per the spec, source up to 2A, so this is fine.

BTW have you ever seen such USB-compliant dedicated charger? I have
more than 5 chargers here, and not one of them is 2A. (Most are .5A,
some are 1A, one is 1.2A).

> However, if you're connected to SDP (regular PC port), which has a power
> budget of 500mA per-port (meaning, that if you're behind a bus powered
> hub, you can't even get 500mA), then this write() of yours should be
> invalid. It should *not* be a successful write() as that creates an
> unsafe and potentially dangerous scenario for the user.

Yes, USB is "potentially dangerous", because noone follows the
specs. Fortunately, everyone knows (except you?) that noone follows
the specs, so the hardware can deal with that, and they include
(poly?) fuses where neccessary.

> > They already can go over the limit, for example using cable linked
> > above. I have several such cables here. I also have various wall
> 
> people can use unsupported cable assemblies if they want, but you can't
> expect kernel to support you.

Then we won't have useful charging support in kernel.

> > supplies that are not detected as a wall supply by N900. So I either
> > have to remember and connect them with the "special" cable, or
> > (easier) use the override above to get useful charging.
> 
> those supplies are not supported by N900. N900 was designed with USB
> Battery Chaging specification in mind and Nokia is not around anymore to
> give you SW updates. Sorry, but that's not the kernel's fault.
> 
> The point is the following: there are a handful of people who would
> *know* how to fiddle with these limits, many would not. The vast
> majority would not. And, considering this is something completely out of
> spec, and, again, potentially dangerous to the user, we are not going to
> support it.

You speak about dangerous where little danger exist; number of
non-compliant cables and USB devices is very high (take your power
bank. Does it really limit to .5A when charging from computer?) and we
should support them, not cry "dangerous" and force everyone to come
with their own "solutions".

> You may use your hacked up cables, not a problem. I did that myself
> during N900 development (though I was using a lab power supply with a 2A
> current limit sourcing 5V) to test port type detection and charging
> algorithm. But that's really not something any company (or this
> community) will support.

Fortunately, that's not your decision and community already decided
the other way.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cun

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
>> > Of course, we may do something sensible by default. But manual
>> > controls should still be present. You called them "stupid" but they
>> > are not.
>> >
>> > Note that just because you detected wall charger does not even mean
>> > you are connected to wall charger. See the link below.
>> 
>> that's a horrible product. So what ? If you want to use that, be my
>> guest. Just, again, don't ask for support when things start falling
>> apart ;-)
>
> So you have USB spec? So what? There are many such products out there,
> and I have at least two such cables here.
>
> They did not cause end of the world, yet, and they are actually very
> useful.

they are also breaking safety requirements and, as I already said, are
potentially dangerous to the user.

>> >> >> still unsafe. If you really wanna do that, you're welcome to removing
>> >> >> safety margins from your own kernel, but we're definitely not going to
>> >> >> ship this to millions of users.
>> >> >
>> >> > Not more unsafe than loading wall chargers with "lets see how much we
>> >> > can get out of it" algorithm. Plus actually required to charge your
>> >> 
>> >> it actually _is_ more unsafe. You could burn mother boards with that. If
>> >> host tells you it only has 100mA power budget left, why are you trying
>> >> to get more ?
>> >
>> > No, you can't burn motherboard like that... You can force emergency
>> > shutdowns, which is also bad, but... There are many devices that break
>> > this aspect of USB protocol.
>> >
>> > https://www.kickstarter.com/projects/1785889318/doubbletime-charging-cable-full-battery-in-1-2-the
>> 
>> we can't prevent people from coming up with bad devices/cables/whatever,
>> right ? But we can make sure that overcoming a 500mA power budget on a
>> USB 2.0 port will not be allowed.
>
> No, you can't, because you don't know if you are connected to USB 2.0
> port. (Because non-compliant cables exist).

yeah, we can't also protect car passenger from bad drivers, but we can
give them all a seat belt ;-)

>> >> > Unfortunately, there's more than one standard for detecting charger,
>> >> > so manual control will probably be required.
>> >> 
>> >> n900 never had manual control of anything. It was just using information
>> >> given by the battery IC, charger IC and twl4030 madc.
>> >
>> > Manual control of n900 charging is done by:
>> >
>> > echo 1800 > /sys/class/power_supply/bq24150a-0/current_limit
>> 
>> yes, that's fine. And if you're connected to a dedicated charger (DCP)
>> which follows USB Battery Charger specification, we *know* that it
>> should, per the spec, source up to 2A, so this is fine.
>
> BTW have you ever seen such USB-compliant dedicated charger? I have
> more than 5 chargers here, and not one of them is 2A. (Most are .5A,
> some are 1A, one is 1.2A).

yeah, if it has the USB BC logo, it _must_ be compliant and full all
requirements of the USB BC spec.

>> However, if you're connected to SDP (regular PC port), which has a power
>> budget of 500mA per-port (meaning, that if you're behind a bus powered
>> hub, you can't even get 500mA), then this write() of yours should be
>> invalid. It should *not* be a successful write() as that creates an
>> unsafe and potentially dangerous scenario for the user.
>
> Yes, USB is "potentially dangerous", because noone follows the
> specs. Fortunately, everyone knows (except you?) that noone follows
> the specs, so the hardware can deal with that, and they include
> (poly?) fuses where neccessary.

heh :-)

>> > They already can go over the limit, for example using cable linked
>> > above. I have several such cables here. I also have various wall
>> 
>> people can use unsupported cable assemblies if they want, but you can't
>> expect kernel to support you.
>
> Then we won't have useful charging support in kernel.

oh no, we will. We just won't let users step outside of safety requirements.

>> > supplies that are not detected as a wall supply by N900. So I either
>> > have to remember and connect them with the "special" cable, or
>> > (easier) use the override above to get useful charging.
>> 
>> those supplies are not supported by N900. N900 was designed with USB
>> Battery Chaging specification in mind and Nokia is not around anymore to
>> give you SW updates. Sorry, but that's not the kernel's fault.
>> 
>> The point is the following: there are a handful of people who would
>> *know* how to fiddle with these limits, many would not. The vast
>> majority would not. And, considering this is something completely out of
>> spec, and, again, potentially dangerous to the user, we are not going to
>> support it.
>
> You speak about dangerous where little danger exist; number of
> non-compliant cables and USB devices is very high (take your power
> bank. Does it really limit to .5A when charging from computer?) and we
> should support them, not cry "dangerous" and force everyone to come
> with their own "solutions".

hehe, you're pretty funny at times

>>

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

2016-04-18 Thread Felipe Balbi

Hi,

Pavel Machek  writes:
>> Pavel Machek  writes:
>> > On Mon 2016-04-18 13:55:17, Felipe Balbi wrote:
>> >> 
>> >> Hi,
>> >> 
>> >> Felipe Balbi  writes:
>> >> >> But cellphone user knows what he connected his charger to, and that's
>> >> >> why it is useful to be able to lower the current. Even when you said
>> >> >> "less is just stupid" I demonstrated it is not, at least in case when
>> >> 
>> >> and btw, you haven't demonstrated anything. You merely stated that it
>> >> isn't without references or numbers, or any source of trustworthy
>> >> information. I'm not really into 'believing'.
>> >
>> > You are not really into reading, either, it seems. Or into the
>> > electronics. Or into physics.
>> >
>> > You seem to understand that charging li-ion from li-ion produces too
>> > much heat. (And yes, it does, DC-DC convertors are not 100%
>> > effective). Converting energy into heat is not a good idea.
>> >
>> > If you need numbers or references to understand basic physics, you'll
>> > need to google it yourself, I'm afraid. 
>> 
>> still doesn't change the fact that you haven't demonstrated anything. If
>> you can't be helpful and/or constructive, you may also go away and use
>> whatever unsafe setup you want to use.
>
> So you still claim that it is "stupid" to charge at anything lower
> than maximum allowed current?
>
> Even after I described the example with device running off power bank
> on a train? Would you explain your reasoning?

you still haven't produced evidence of your claim. Claiming without
evidence has virtually no meaning. Anyway, I'll just google a bit later
seen as if you're just unwilling to produce evidence of your claims.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCHv5 1/3] usb: storage: scsiglue: further describe our 240 sector limit

2016-04-18 Thread Sergei Shtylyov

Hello.

On 4/18/2016 1:09 PM, Felipe Balbi wrote:


Just so we have some sort of documentation as to why
we limit our Mass Storage transfers to 240 sectors,
let's update the comment to make clearer that
devices were found that would choke with larger
transfers.

While at that, also make sure to clarify that other
operating systems have similar, albeit different,
limits on mass storage transfers.

Signed-off-by: Felipe Balbi 
---
  drivers/usb/storage/scsiglue.c | 19 ++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 90901861bfc0..9da1fb3d0ff4 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -565,7 +565,24 @@ static const struct scsi_host_template 
usb_stor_host_template = {
/* lots of sg segments can be handled */
.sg_tablesize = SCSI_MAX_SG_CHAIN_SEGMENTS,

-   /* limit the total size of a transfer to 120 KB */
+
+   /*
+* Limit the total size of a transfer to 120 KB.
+*
+* Some devices are known to choke with anything larger. It seems like
+* the problem stems from the fact that original IDE controllers had
+* only an 8-bit register to hold the number of sectors in one transfer
+* and even those couldn't handle a full 256 sectors.
+*
+* Because we want to make sure we interoperate with as many devices as
+* possible, we will maintain a 240 sector transfer size limit for USB
+* Mass Storage devices.
+*
+* Tests show that other operating have similar limits with Microsoft


   "Systems" missing.

[...]

MBR, Sergei

--
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] watchdog: add driver for StreamLabs USB watchdog device

2016-04-18 Thread Guenter Roeck

On 04/18/2016 01:32 AM, Oliver Neukum wrote:

On Mon, 2016-04-18 at 03:53 +0100, Alexey Klimov wrote:

This patch creates new driver that supports StreamLabs usb watchdog
device. This device plugs into 9-pin usb header and connects to
reset pin and reset button on common PC.

USB commands used to communicate with device were reverse
engineered using usbmon.


Almost. I see only one issue.


+struct streamlabs_wdt {
+   struct watchdog_device wdt_dev;
+   struct usb_interface *intf;
+
+   struct mutex lock;
+   u8 buffer[BUFFER_LENGTH];


That is wrong.


+};
+


[..]


+static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev, u16 cmd)
+{
+   struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
+   struct usb_device *usbdev;
+   int retval;
+   int size;
+   unsigned long timeout_msec;
+
+   int retry_counter = 10; /* how many times to re-send stop cmd */
+
+   mutex_lock(&streamlabs_wdt->lock);
+
+   if (unlikely(!streamlabs_wdt->intf)) {
+   mutex_unlock(&streamlabs_wdt->lock);
+   return -ENODEV;
+   }
+
+   usbdev = interface_to_usbdev(streamlabs_wdt->intf);
+   timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
+
+   do {
+   usb_streamlabs_wdt_prepare_buf((u16 *) streamlabs_wdt->buffer,
+   cmd, timeout_msec);
+   /* send command to watchdog */
+   retval = usb_interrupt_msg(usbdev, usb_sndintpipe(usbdev, 0x02),
+   streamlabs_wdt->buffer, BUFFER_TRANSFER_LENGTH,


Because of this line.

The problem is subtle. Your buffer and your lock share a cacheline.
On some architecture the cache is not consistent with respect to DMA.
On them cachelines holding a buffer for DMA need to be flushed to RAM
and invalidated and you may read from them only after DMA has finished.

Thus you may have prepared a cacheline for DMA but somebody tries taking
the lock. Then the cacheline with the lock is read from RAM. If that
happens before you finish the DMA the data resulting from DMA is lost.

The fix is to allocate the buffer with its own allocation. The VM
subsystem makes sure separate allocation don't share cachelines.



Hi Oliver,

For my own education, would adding cacheline_aligned to the buffer variable
declaration solve the problem as well ?

Thanks,
Guenter


That is the long explanation for what I mean when I say that you violate
the DMA rules.

Regards
Oliver





--
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: [PATCHv5 0/3] usb: storage: increase limit to 2048 sectors

2016-04-18 Thread Alan Stern
On Mon, 18 Apr 2016, Felipe Balbi wrote:

> Hi,
> 
> Here's v5 of the 2048 sector limit patch series.
> 
> Changes since v4:
>   - Respect possible TAPE device types quirks
> 
> Changes since v3:
>   - Respect USB_FL_MAX_SECTORS* if they are set for USB3 devices
> 
> Changes since v2:
>   - Add a patch fixing multi-line comment style
>   - s/limitting/limiting
>   - Remove US_FL_USB3 flag
> 
> Felipe Balbi (3):
>   usb: storage: scsiglue: further describe our 240 sector limit
>   usb: storage: scsiglue: limit USB3 devices to 2048 sectors
>   usb: storage: fix multi-line comment style

Acked-by: Alan Stern 

I did not bother to check through the 3/3 patch in detail; I trust you 
to get it right.  :-)

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 v6 01/12] usb: hcd: Initialize hcd->flags to 0

2016-04-18 Thread Alan Stern
On Mon, 18 Apr 2016, Peter Chen wrote:

> On Wed, Apr 06, 2016 at 09:32:22AM +0300, Roger Quadros wrote:
> > On 06/04/16 09:09, Felipe Balbi wrote:
> > > 
> > > Hi,
> > > 
> > > Roger Quadros  writes:
> > >> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > >> index 2ca2cef..6b1930d 100644
> > >> --- a/drivers/usb/core/hcd.c
> > >> +++ b/drivers/usb/core/hcd.c
> > >> @@ -2706,6 +2706,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
> > >>  int retval;
> > >>  struct usb_device *rhdev;
> > >>  
> > >> +hcd->flags = 0;
> > > 
> > > seems like this would make more sense in usb_del_hcd() instead.
> > > 
> > 
> > OK, I'll move it there.
> > 
> 
> It depends on Alan's comments, whether only usb_add_hcd/usb_del_hcd
> pair can be called repeat. If Alan acks it, I have no idea for it.

Most of the host controller drivers were not written with this in mind,
but I think it would be a good thing to allow.  It would speed up the
host/device role switches.

This might mean we need to fix up several drivers to make them work 
correctly in an OTG environment.  It should be possible to do this.  Is 
there any particular reason why it would be difficult for Chipidea?

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 v2] watchdog: add driver for StreamLabs USB watchdog device

2016-04-18 Thread Oliver Neukum
On Mon, 2016-04-18 at 06:57 -0700, Guenter Roeck wrote:
> On 04/18/2016 01:32 AM, Oliver Neukum wrote:
> > On Mon, 2016-04-18 at 03:53 +0100, Alexey Klimov wrote:
> >> This patch creates new driver that supports StreamLabs usb watchdog
> >> device. This device plugs into 9-pin usb header and connects to
> >> reset pin and reset button on common PC.
> >>
> >> USB commands used to communicate with device were reverse
> >> engineered using usbmon.
> >
> > Almost. I see only one issue.
> >
> >> +struct streamlabs_wdt {
> >> +  struct watchdog_device wdt_dev;
> >> +  struct usb_interface *intf;
> >> +
> >> +  struct mutex lock;
> >> +  u8 buffer[BUFFER_LENGTH];
> >
> > That is wrong.
> >
> >> +};
> >> +
> >
> > [..]
> >
> >> +static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev, 
> >> u16 cmd)
> >> +{
> >> +  struct streamlabs_wdt *streamlabs_wdt = watchdog_get_drvdata(wdt_dev);
> >> +  struct usb_device *usbdev;
> >> +  int retval;
> >> +  int size;
> >> +  unsigned long timeout_msec;
> >> +
> >> +  int retry_counter = 10; /* how many times to re-send stop cmd */
> >> +
> >> +  mutex_lock(&streamlabs_wdt->lock);
> >> +
> >> +  if (unlikely(!streamlabs_wdt->intf)) {
> >> +  mutex_unlock(&streamlabs_wdt->lock);
> >> +  return -ENODEV;
> >> +  }
> >> +
> >> +  usbdev = interface_to_usbdev(streamlabs_wdt->intf);
> >> +  timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
> >> +
> >> +  do {
> >> +  usb_streamlabs_wdt_prepare_buf((u16 *) streamlabs_wdt->buffer,
> >> +  cmd, timeout_msec);
> >> +  /* send command to watchdog */
> >> +  retval = usb_interrupt_msg(usbdev, usb_sndintpipe(usbdev, 0x02),
> >> +  streamlabs_wdt->buffer, BUFFER_TRANSFER_LENGTH,
> >
> > Because of this line.
> >
> > The problem is subtle. Your buffer and your lock share a cacheline.
> > On some architecture the cache is not consistent with respect to DMA.
> > On them cachelines holding a buffer for DMA need to be flushed to RAM
> > and invalidated and you may read from them only after DMA has finished.
> >
> > Thus you may have prepared a cacheline for DMA but somebody tries taking
> > the lock. Then the cacheline with the lock is read from RAM. If that
> > happens before you finish the DMA the data resulting from DMA is lost.
> >
> > The fix is to allocate the buffer with its own allocation. The VM
> > subsystem makes sure separate allocation don't share cachelines.
> >
> 
> Hi Oliver,
> 
> For my own education, would adding cacheline_aligned to the buffer 
> variable
> declaration solve the problem as well ?

Possibly. We have never gone that route. The obvious problems is that
I am not sure our alignment is known before boot.

Regards
Oliver


--
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: lpm: add boot flag to disable lpm

2016-04-18 Thread Alan Stern
On Fri, 15 Apr 2016, Matthew Giassa wrote:

> Good afternoon Alan,
> 
> Attached is the requested usbmon output. I started the camera, had a lot
> of config read/write calls complete, and was able to eventually get one
> frame of image data to be saved and rendered.

Obviously something is going on that I don't understand.  Try this 
patch, which has extra debugging output added, in place of the earler 
one.  I won't need to see any usbmon output, just the dmesg log.

(And you don't have to post the entire thing, with hundreds of 
repetitions of the same information over and over again.  Two or three 
iterations will be fine.)

Alan Stern



Index: usb-4.4/include/linux/usb.h
===
--- usb-4.4.orig/include/linux/usb.h
+++ usb-4.4/include/linux/usb.h
@@ -1076,7 +1076,7 @@ struct usbdrv_wrap {
  * for interfaces bound to this driver.
  * @soft_unbind: if set to 1, the USB core will not kill URBs and disable
  * endpoints before calling the driver's disconnect method.
- * @disable_hub_initiated_lpm: if set to 0, the USB core will not allow hubs
+ * @disable_hub_initiated_lpm: if set to 1, the USB core will not allow hubs
  * to initiate lower power link state transitions when an idle timeout
  * occurs.  Device-initiated USB 3.0 link PM will still be allowed.
  *
Index: usb-4.4/drivers/usb/core/driver.c
===
--- usb-4.4.orig/drivers/usb/core/driver.c
+++ usb-4.4/drivers/usb/core/driver.c
@@ -284,7 +284,7 @@ static int usb_probe_interface(struct de
struct usb_device *udev = interface_to_usbdev(intf);
const struct usb_device_id *id;
int error = -ENODEV;
-   int lpm_disable_error;
+   int lpm_disable_error = -ENODEV;
 
dev_dbg(dev, "%s\n", __func__);
 
@@ -336,12 +336,14 @@ static int usb_probe_interface(struct de
 * setting during probe, that should also be fine.  usb_set_interface()
 * will attempt to disable LPM, and fail if it can't disable it.
 */
-   lpm_disable_error = usb_unlocked_disable_lpm(udev);
-   if (lpm_disable_error && driver->disable_hub_initiated_lpm) {
-   dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.",
-   __func__, driver->name);
-   error = lpm_disable_error;
-   goto err;
+   if (driver->disable_hub_initiated_lpm) {
+   lpm_disable_error = usb_unlocked_disable_lpm(udev);
+   if (lpm_disable_error) {
+   dev_err(&intf->dev, "%s Failed to disable LPM for 
driver %s\n.",
+   __func__, driver->name);
+   error = lpm_disable_error;
+   goto err;
+   }
}
 
/* Carry out a deferred switch to altsetting 0 */
@@ -391,7 +393,8 @@ static int usb_unbind_interface(struct d
struct usb_interface *intf = to_usb_interface(dev);
struct usb_host_endpoint *ep, **eps = NULL;
struct usb_device *udev;
-   int i, j, error, r, lpm_disable_error;
+   int i, j, error, r;
+   int lpm_disable_error = -ENODEV;
 
intf->condition = USB_INTERFACE_UNBINDING;
 
@@ -399,12 +402,13 @@ static int usb_unbind_interface(struct d
udev = interface_to_usbdev(intf);
error = usb_autoresume_device(udev);
 
-   /* Hub-initiated LPM policy may change, so attempt to disable LPM until
+   /* If hub-initiated LPM policy may change, attempt to disable LPM until
 * the driver is unbound.  If LPM isn't disabled, that's fine because it
 * wouldn't be enabled unless all the bound interfaces supported
 * hub-initiated LPM.
 */
-   lpm_disable_error = usb_unlocked_disable_lpm(udev);
+   if (driver->disable_hub_initiated_lpm)
+   lpm_disable_error = usb_unlocked_disable_lpm(udev);
 
/*
 * Terminate all URBs for this interface unless the driver
Index: usb-4.4/drivers/usb/core/devio.c
===
--- usb-4.4.orig/drivers/usb/core/devio.c
+++ usb-4.4/drivers/usb/core/devio.c
@@ -612,6 +612,8 @@ struct usb_driver usbfs_driver = {
.resume =   driver_resume,
 };
 
+int alantest;
+
 static int claimintf(struct usb_dev_state *ps, unsigned int ifnum)
 {
struct usb_device *dev = ps->dev;
@@ -627,8 +629,11 @@ static int claimintf(struct usb_dev_stat
intf = usb_ifnum_to_if(dev, ifnum);
if (!intf)
err = -ENOENT;
-   else
+   else {
+   alantest = 1;
err = usb_driver_claim_interface(&usbfs_driver, intf, ps);
+   alantest = 0;
+   }
if (err == 0)
set_bit(ifnum, &ps->ifclaimed);
return err;
@@ -648,7 +653,9 @@ static int releaseintf(struct usb_dev_st
if (!intf)
err = -ENOENT;
 

[PATCH 2/2] Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb return in bottom half"

2016-04-18 Thread Bin Liu
This reverts commit 2035772010db634ec8566b658fb1cd87ec47ac77.

Commit 20357720 claims throughput improvement for MSC/UVC, but I
don't see much improvement. Following are the MSC measurement using
dd on AM335x GP EVM.

with BCD_BH:read: 14.9MB/s, write: 20.9MB/s
without BCD_BH: read: 15.2MB/s, write: 21.2MB/s

However with this commit the following regressions have been observed.

1. ASIX usb-ethernet dongle is completely broken on UDP RX.

2. Unpluging a 3G modem, which uses option driver, behind a hub causes
   console log flooding with the following message.

   option_instat_callback: error -71

Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 58487a4..2f8ad7f 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2735,7 +2735,7 @@ static const struct hc_driver musb_hc_driver = {
.description= "musb-hcd",
.product_desc   = "MUSB HDRC host driver",
.hcd_priv_size  = sizeof(struct musb *),
-   .flags  = HCD_USB2 | HCD_MEMORY | HCD_BH,
+   .flags  = HCD_USB2 | HCD_MEMORY,
 
/* not using irq handler or reset hooks from usbcore, since
 * those must be shared with peripheral code for OTG configs
-- 
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 0/2] musb fixes for v4.6-rc5

2016-04-18 Thread Bin Liu
Hi Greg,

Here are a couple fixes for musb for v4.6-rc5.

Tal's patch fixes a musb teardown bug which is uncovered by a g_zero
change in v4.3. HCD_BH flag does not bring any visible benefit but breaks
a few cases.

Please let me know if any change is needed.

Thanks,
-Bin.

Bin Liu (1):
  Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb return
in bottom half"

Tal Shorer (1):
  usb: musb: gadget: nuke endpoint before setting its descriptor to NULL

 drivers/usb/musb/musb_gadget.c | 6 +++---
 drivers/usb/musb/musb_host.c   | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
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 1/2] usb: musb: gadget: nuke endpoint before setting its descriptor to NULL

2016-04-18 Thread Bin Liu
From: Tal Shorer 

Some functions, such as f_sourcesink, rely on an endpoint's desc
field during their requests' complete() callback, so clear it only
_after_ nuking all requests to avoid NULL pointer dereference.

Signed-off-by: Tal Shorer 
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_gadget.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 87bd578..152865b 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1164,12 +1164,12 @@ static int musb_gadget_disable(struct usb_ep *ep)
musb_writew(epio, MUSB_RXMAXP, 0);
}
 
-   musb_ep->desc = NULL;
-   musb_ep->end_point.desc = NULL;
-
/* abort all pending DMA and requests */
nuke(musb_ep, -ESHUTDOWN);
 
+   musb_ep->desc = NULL;
+   musb_ep->end_point.desc = NULL;
+
schedule_work(&musb->irq_work);
 
spin_unlock_irqrestore(&(musb->lock), flags);
-- 
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] watchdog: add driver for StreamLabs USB watchdog device

2016-04-18 Thread Guenter Roeck
On Mon, Apr 18, 2016 at 04:08:38PM +0200, Oliver Neukum wrote:
> On Mon, 2016-04-18 at 06:57 -0700, Guenter Roeck wrote:
> > On 04/18/2016 01:32 AM, Oliver Neukum wrote:
> > > On Mon, 2016-04-18 at 03:53 +0100, Alexey Klimov wrote:
> > >> This patch creates new driver that supports StreamLabs usb watchdog
> > >> device. This device plugs into 9-pin usb header and connects to
> > >> reset pin and reset button on common PC.
> > >>
> > >> USB commands used to communicate with device were reverse
> > >> engineered using usbmon.
> > >
> > > Almost. I see only one issue.
> > >
> > >> +struct streamlabs_wdt {
> > >> +struct watchdog_device wdt_dev;
> > >> +struct usb_interface *intf;
> > >> +
> > >> +struct mutex lock;
> > >> +u8 buffer[BUFFER_LENGTH];
> > >
> > > That is wrong.
> > >
> > >> +};
> > >> +
> > >
> > > [..]
> > >
> > >> +static int usb_streamlabs_wdt_command(struct watchdog_device *wdt_dev, 
> > >> u16 cmd)
> > >> +{
> > >> +struct streamlabs_wdt *streamlabs_wdt = 
> > >> watchdog_get_drvdata(wdt_dev);
> > >> +struct usb_device *usbdev;
> > >> +int retval;
> > >> +int size;
> > >> +unsigned long timeout_msec;
> > >> +
> > >> +int retry_counter = 10; /* how many times to re-send 
> > >> stop cmd */
> > >> +
> > >> +mutex_lock(&streamlabs_wdt->lock);
> > >> +
> > >> +if (unlikely(!streamlabs_wdt->intf)) {
> > >> +mutex_unlock(&streamlabs_wdt->lock);
> > >> +return -ENODEV;
> > >> +}
> > >> +
> > >> +usbdev = interface_to_usbdev(streamlabs_wdt->intf);
> > >> +timeout_msec = wdt_dev->timeout * MSEC_PER_SEC;
> > >> +
> > >> +do {
> > >> +usb_streamlabs_wdt_prepare_buf((u16 *) 
> > >> streamlabs_wdt->buffer,
> > >> +cmd, 
> > >> timeout_msec);
> > >> +/* send command to watchdog */
> > >> +retval = usb_interrupt_msg(usbdev, 
> > >> usb_sndintpipe(usbdev, 0x02),
> > >> +streamlabs_wdt->buffer, 
> > >> BUFFER_TRANSFER_LENGTH,
> > >
> > > Because of this line.
> > >
> > > The problem is subtle. Your buffer and your lock share a cacheline.
> > > On some architecture the cache is not consistent with respect to DMA.
> > > On them cachelines holding a buffer for DMA need to be flushed to RAM
> > > and invalidated and you may read from them only after DMA has finished.
> > >
> > > Thus you may have prepared a cacheline for DMA but somebody tries taking
> > > the lock. Then the cacheline with the lock is read from RAM. If that
> > > happens before you finish the DMA the data resulting from DMA is lost.
> > >
> > > The fix is to allocate the buffer with its own allocation. The VM
> > > subsystem makes sure separate allocation don't share cachelines.
> > >
> > 
> > Hi Oliver,
> > 
> > For my own education, would adding cacheline_aligned to the buffer 
> > variable
> > declaration solve the problem as well ?
> 
> Possibly. We have never gone that route. The obvious problems is that
> I am not sure our alignment is known before boot.
> 
Seems scary. I always thought that the alignment associated with
cacheline_aligned would be the maximum possible for a given
build/architecture. If not, what is the value of having
cacheline_aligned in the first place ?

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: serial: Adding Straizona Focusers to cp210x driver

2016-04-18 Thread Johan Hovold
On Sun, Apr 10, 2016 at 04:35:04PM +0300, Jasem Mutlaq wrote:
> Adding VID:PID for Straizona Focusers to cp210x driver.
> 
> Signed-off-by: Jasem Mutlaq 

Thanks for the patch.

Try running your patch through checkpatch.pl before submitting. It would
have let you know that the patch has some white-space issues (could
also have been added by your mailer).

> ---
>  drivers/usb/serial/cp210x.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index dd47823..d1bed34 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -109,6 +109,7 @@ static const struct usb_device_id id_table[] = {
> { USB_DEVICE(0x10C4, 0x826B) }, /* Cygnal Integrated Products,
> Inc., Fasttrax GPS demonstration module */
> { USB_DEVICE(0x10C4, 0x8281) }, /* Nanotec Plug & Drive */
> { USB_DEVICE(0x10C4, 0x8293) }, /* Telegesis ETRX2USB */
> +   { USB_DEVICE(0x10C4, 0x82F4) }, /* Starizona MicroTouch*/
> { USB_DEVICE(0x10C4, 0x82F9) }, /* Procyon AVS */
> { USB_DEVICE(0x10C4, 0x8341) }, /* Siemens MC35PU GPRS Modem */
> { USB_DEVICE(0x10C4, 0x8382) }, /* Cygnal Integrated Products, Inc.
> */
> @@ -118,6 +119,7 @@ static const struct usb_device_id id_table[] = {
> { USB_DEVICE(0x10C4, 0x8418) }, /* IRZ Automation Teleport SG-10
> GSM/GPRS Modem */
> { USB_DEVICE(0x10C4, 0x846E) }, /* BEI USB Sensor Interface (VCP) */
> { USB_DEVICE(0x10C4, 0x8477) }, /* Balluff RFID */
> +   { USB_DEVICE(0x10C4, 0x84B6) }, /* Starizona Hyperion*/

Missing space before *.

> { USB_DEVICE(0x10C4, 0x85EA) }, /* AC-Services IBUS-IF */
> { USB_DEVICE(0x10C4, 0x85EB) }, /* AC-Services CIS-IBUS */
> { USB_DEVICE(0x10C4, 0x85F8) }, /* Virtenio Preon32 */

Looks good otherwise.

Care to fix that up and resend?

Thanks,
Johan
--
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] watchdog: add driver for StreamLabs USB watchdog device

2016-04-18 Thread Guenter Roeck
On Mon, Apr 18, 2016 at 03:53:36AM +0100, Alexey Klimov wrote:
> This patch creates new driver that supports StreamLabs usb watchdog
> device. This device plugs into 9-pin usb header and connects to
> reset pin and reset button on common PC.
> 
> USB commands used to communicate with device were reverse
> engineered using usbmon.
> 
> Signed-off-by: Alexey Klimov 
> ---
> Changes in v2:
>  -- coding style cleanups
>  -- turn some dev_err messages to dev_dbg
>  -- reimplemented usb_streamlabs_wdt_command() to use loop
>  -- re-worked disconnect routine
>  -- rebased to 4.6-rc2, removed set_timeout method
>  -- removed braces in .options field in streamlabs_wdt_indent
>  -- mem allocation migrated to devm_kzalloc
>  -- buffer for device struct moved inside main struct
> to avoid additional memory allocation
>  -- removed watchdog_init_timeout()
>  -- re-worked usb_streamlabs_wdt_{resume,suspend}
>  -- removed struct usb_device pointer from main driver struct
>  -- buffer preparation for communication migrated to cpu_to_le16()
> functions, also buffer is filled in as u16 elements to
> make this byteorder usable
>  -- added stop command in usb_streamlabs_wdt_disconnect()
>  
>  drivers/watchdog/Kconfig  |  15 ++
>  drivers/watchdog/Makefile |   1 +
>  drivers/watchdog/streamlabs_wdt.c | 313 
> ++
>  3 files changed, 329 insertions(+)
>  create mode 100644 drivers/watchdog/streamlabs_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index fb94765..130cf54 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1766,4 +1766,19 @@ config USBPCWATCHDOG
>  
> Most people will say N.
>  
> +config USB_STREAMLABS_WATCHDOG
> + tristate "StreamLabs USB watchdog driver"
> + depends on USB
> + ---help---
> +   This is the driver for the USB Watchdog dongle from StreamLabs.
> +   If you correctly connect reset pins to motherboard Reset pin and
> +   to Reset button then this device will simply watch your kernel to make
> +   sure it doesn't freeze, and if it does, it reboots your computer
> +   after a certain amount of time.
> +
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called streamlabs_wdt.
> +
> +   Most people will say N. Say yes or M if you want to use such usb 
> device.
>  endif # WATCHDOG
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index feb6270..9d36929 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_WDTPCI) += wdt_pci.o
>  
>  # USB-based Watchdog Cards
>  obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
> +obj-$(CONFIG_USB_STREAMLABS_WATCHDOG) += streamlabs_wdt.o
>  
>  # ALPHA Architecture
>  
> diff --git a/drivers/watchdog/streamlabs_wdt.c 
> b/drivers/watchdog/streamlabs_wdt.c
> new file mode 100644
> index 000..3e34cd8
> --- /dev/null
> +++ b/drivers/watchdog/streamlabs_wdt.c
> @@ -0,0 +1,313 @@
> +/*
> + * StreamLabs USB Watchdog driver
> + *
> + * Copyright (c) 2016 Alexey Klimov 
> + *
> + * This program is free software; you may redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * USB Watchdog device from Streamlabs
> + * http://www.stream-labs.com/products/devices/watchdog/
> + *
> + * USB commands have been reverse engineered using usbmon.
> + */
> +
> +#define DRIVER_AUTHOR "Alexey Klimov "
> +#define DRIVER_DESC "StreamLabs USB watchdog driver"
> +#define DRIVER_NAME "usb_streamlabs_wdt"
> +
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE("GPL");
> +
> +#define USB_STREAMLABS_WATCHDOG_VENDOR   0x13c0
> +#define USB_STREAMLABS_WATCHDOG_PRODUCT  0x0011
> +
> +/*
> + * one buffer is used for communication, however transmitted message is only
> + * 32 bytes long
> + */
> +#define BUFFER_TRANSFER_LENGTH   32
> +#define BUFFER_LENGTH64
> +#define USB_TIMEOUT  350
> +
> +#define STREAMLABS_CMD_START 0xaacc
> +#define STREAMLABS_CMD_STOP  0xbbff
> +
> +#define STREAMLABS_WDT_MIN_TIMEOUT   1
> +#define STREAMLABS_WDT_MAX_TIMEOUT   46
> +
An explanation for those min/max limits would be nice.

> +struct streamlabs_wdt {
> + struct watchdog_device wdt_dev;
> + struct usb_interface *intf;
> +
> + struct mutex lock;
> + u8 buffer[BUFFER_LENGTH];
> +};
> +
> +static boo

Greetings!!!

2016-04-18 Thread andreas11
Hi, how are you? My name is J Eric Denials, External Financial Auditor at 
Lloyds Banking Group plc., London. It is a pleasure to contact you at this time 
through this medium. I have a cool and legitimate deal to do with you as you're 
a foreigner, it will be mutually beneficial to both. If you’re interested, 
urgently, get back to me for more explanation about the deal.
Regards
Eric
--
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/2] console: Add persistent scrollback buffers for all VGA consoles

2016-04-18 Thread Manuel Schölling
Add a scrollback buffers for each VGA console. The benefit is that
the scrollback history is not flushed when switching between consoles
but is persistent.
The buffers are allocated on demand when a new console is opened.

This breaks tools like clear_console that rely on flushing the
scrollback history by switching back and forth between consoles
which is why this feature is disabled by default.

Signed-off-by: Manuel Schölling 
---
 drivers/video/console/Kconfig  |  22 +-
 drivers/video/console/vgacon.c | 172 +++--
 2 files changed, 133 insertions(+), 61 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 38da6e2..f101a63 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -43,9 +43,25 @@ config VGACON_SOFT_SCROLLBACK_SIZE
range 1 1024
default "64"
help
- Enter the amount of System RAM to allocate for the scrollback
-buffer.  Each 64KB will give you approximately 16 80x25
-screenfuls of scrollback buffer
+ Enter the amount of System RAM to allocate for scrollback
+ buffers of VGA consoles. Each 64KB will give you approximately
+ 16 80x25 screenfuls of scrollback buffer.
+
+config VGACON_SOFT_SCROLLBACK_FOR_EACH_CONSOLE
+   bool "Persistent Scrollback History for each console"
+   depends on VGACON_SOFT_SCROLLBACK
+   default n
+   help
+ Say Y here if for each VGA console a scrollback buffer should
+ be allocated. The scrollback history will persist when switching
+ between consoles. If you say N here, scrollback is only supported
+ for the active VGA console and scrollback history will be flushed
+ when switching between consoles.
+
+ This breaks legacy versions of tools like clear_console which
+ might raise security issues.
+
+ If you use a RAM-constrained system, say N here.
 
 config MDA_CONSOLE
depends on !M68K && !PARISC && ISA
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 517f565..6c0b9ba 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -1,5 +1,5 @@
 /*
- *  linux/drivers/video/vgacon.c -- Low level VGA based console driver
+ *  linux/drivers/video/console/vgacon.c -- Low level VGA based console driver
  *
  * Created 28 Sep 1997 by Geert Uytterhoeven
  *
@@ -106,12 +106,12 @@ static unsigned char  vga_hardscroll_enabled  
__read_mostly;
 static unsigned char   vga_hardscroll_user_enable __read_mostly = 1;
 static unsigned char   vga_font_is_default = 1;
 static int vga_vesa_blanked;
-static int vga_palette_blanked;
-static int vga_is_gfx;
-static int vga_512_chars;
-static int vga_video_font_height;
-static int vga_scan_lines  __read_mostly;
-static unsigned intvga_rolled_over;
+static int vga_palette_blanked;
+static int vga_is_gfx;
+static int vga_512_chars;
+static int vga_video_font_height;
+static int vga_scan_lines  __read_mostly;
+static unsigned intvga_rolled_over;
 
 static int vgacon_text_mode_force;
 
@@ -182,70 +182,125 @@ static inline void vga_set_mem_top(struct vc_data *c)
 
 #ifdef CONFIG_VGACON_SOFT_SCROLLBACK
 /* software scrollback */
-static void *vgacon_scrollback;
-static int vgacon_scrollback_tail;
-static int vgacon_scrollback_size;
-static int vgacon_scrollback_rows;
-static int vgacon_scrollback_cnt;
-static int vgacon_scrollback_cur;
-static int vgacon_scrollback_save;
-static int vgacon_scrollback_restore;
-
-static void vgacon_scrollback_init(int pitch)
+struct vgacon_scrollback_info {
+   void *data;
+   int tail;
+   int size;
+   int rows;
+   int cnt;
+   int cur;
+   int save;
+   int restore;
+};
+static struct vgacon_scrollback_info *vgacon_scrollback_cur;
+#ifdef CONFIG_VGACON_SOFT_SCROLLBACK_FOR_EACH_CONSOLE
+static struct vgacon_scrollback_info vgacon_scrollbacks[MAX_NR_CONSOLES];
+#else
+static struct vgacon_scrollback_info vgacon_scrollbacks[1];
+#endif
+
+static void vgacon_scrollback_reset(size_t reset_size)
 {
-   int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;
-
-   if (vgacon_scrollback) {
-   vgacon_scrollback_cnt  = 0;
-   vgacon_scrollback_tail = 0;
-   vgacon_scrollback_cur  = 0;
-   vgacon_scrollback_rows = rows - 1;
-   vgacon_scrollback_size = rows * pitch;
+   if (vgacon_scrollback_cur->data && reset_size > 0)
+   memset(vgacon_scrollback_cur->data, 0, reset_size);
+
+   vgacon_scrollback_cur->cnt  = 0;
+   vgacon_scrollback_cur->tail = 0;
+   vgacon_scrollback_cur->cur  = 0;
+}
+
+static void vgacon_scrollback_init(int vc_num)
+{
+   int pitch = vga_video_num_columns * 2;
+   size_t size = CONFIG_

[PATCH 2/2] console: Add ioctl for flushing the scrollback buffer

2016-04-18 Thread Manuel Schölling
Tools like clear_console rely on the fact that scrollback history is
flushed when switching back and forth between consoles.
Persistent scrollback buffers for each console breaks this, so this
patch adds a ioctl() callf for flushing the scrollback history.

Signed-off-by: Manuel Schölling 
---
 drivers/tty/vt/vt_ioctl.c   | 20 
 drivers/usb/misc/sisusbvga/sisusb_con.c |  1 +
 drivers/video/console/dummycon.c|  1 +
 drivers/video/console/mdacon.c  |  6 ++
 drivers/video/console/newport_con.c |  1 +
 drivers/video/console/sticon.c  |  7 +++
 drivers/video/console/vgacon.c  | 23 +++
 include/linux/console.h |  1 +
 include/uapi/linux/vt.h |  1 +
 9 files changed, 61 insertions(+)

diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 97d5a74..18adc23 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -903,6 +903,26 @@ int vt_ioctl(struct tty_struct *tty,
break;
}
 
+   /*
+* flush the specified VT's scollback buffer
+*/
+   case VT_FLUSH_SCROLLBACK: {
+   if (!perm)
+   return -EPERM;
+   if (arg == 0 || arg > MAX_NR_CONSOLES)
+   ret = -ENXIO;
+   else {
+   struct vc_data *data = vc_cons[arg-1].d;
+
+   if (!data)
+   ret = -ENXIO;
+   else
+   ret = data->vc_sw->con_flush_scrollback(data);
+   }
+
+   break;
+   }
+
case PIO_FONT: {
if (!perm)
return -EPERM;
diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c 
b/drivers/usb/misc/sisusbvga/sisusb_con.c
index ace3430..cc5fc10 100644
--- a/drivers/usb/misc/sisusbvga/sisusb_con.c
+++ b/drivers/usb/misc/sisusbvga/sisusb_con.c
@@ -1442,6 +1442,7 @@ static const struct consw sisusb_dummy_con = {
.con_font_copy =SISUSBCONDUMMY,
.con_set_palette =  SISUSBCONDUMMY,
.con_scrolldelta =  SISUSBCONDUMMY,
+   .con_flush_scrollback = SISUSBCONDUMMY,
 };
 
 int
diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index 0efc52f..d2888e5 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -73,5 +73,6 @@ const struct consw dummy_con = {
 .con_font_copy =   DUMMY,
 .con_set_palette = DUMMY,
 .con_scrolldelta = DUMMY,
+.con_flush_scrollback = DUMMY,
 };
 EXPORT_SYMBOL_GPL(dummy_con);
diff --git a/drivers/video/console/mdacon.c b/drivers/video/console/mdacon.c
index 296e945..10ebcbe 100644
--- a/drivers/video/console/mdacon.c
+++ b/drivers/video/console/mdacon.c
@@ -510,6 +510,11 @@ static int mdacon_scrolldelta(struct vc_data *c, int lines)
return 0;
 }
 
+static int mdacon_flush_scrollback(struct vc_data *c)
+{
+   return 0;
+}
+
 static void mdacon_cursor(struct vc_data *c, int mode)
 {
if (mode == CM_ERASE) {
@@ -579,6 +584,7 @@ static const struct consw mda_con = {
.con_blank =mdacon_blank,
.con_set_palette =  mdacon_set_palette,
.con_scrolldelta =  mdacon_scrolldelta,
+   .con_flush_scrollback = mdacon_flush_scrollback,
.con_build_attr =   mdacon_build_attr,
.con_invert_region =mdacon_invert_region,
 };
diff --git a/drivers/video/console/newport_con.c 
b/drivers/video/console/newport_con.c
index bb4e962..d04183c 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -738,6 +738,7 @@ const struct consw newport_con = {
.con_scrolldelta  = newport_scrolldelta,
.con_set_origin   = DUMMY,
.con_save_screen  = DUMMY
+   .con_flush_scrollback = DUMMY,
 };
 
 static int newport_probe(struct gio_device *dev,
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 026fd12..46046d6 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -345,6 +345,12 @@ static void sticon_save_screen(struct vc_data *conp)
 {
 }
 
+static int sticon_flush_scrollback(struct vc_data *c)
+{
+   return 0;
+}
+
+
 static const struct consw sti_con = {
.owner  = THIS_MODULE,
.con_startup= sticon_startup,
@@ -360,6 +366,7 @@ static const struct consw sti_con = {
.con_blank  = sticon_blank,
.con_set_palette= sticon_set_palette,
.con_scrolldelta= sticon_scrolldelta,
+   .con_flush_scrollback   = sticon_flush_scrollback,
.con_set_origin = sticon_set_origin,
.con_save_screen= sticon_save_screen, 
.con_build_attr = sticon_build_attr,
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 6c0b9ba..b5ea94f 100644
--- a/drivers/vi

[PATCH 0/2] Persistent scrollback buffers for all VGA consoles

2016-04-18 Thread Manuel Schölling
Another attempt to add persistent scrollback buffers for all VGA consoles,
so the buffer is not flushed when swithing back and forth between consoles.

Note that breaks tools like bash's clear_console and thus might have
security implications:
clear_console relies on this 'anti-feature' of the kernel to clear the buffer
when consoles are switched.

To offer a way for userland tools to flush the buffer my second patch adds
a ioctl call for that.
Also this feature is disabled by default and security implications are clearly
stated in its documentation.

Manuel Schölling (2):
  console: Add persistent scrollback buffers for all VGA consoles
  console: Add ioctl for flushing the scrollback buffer

 drivers/tty/vt/vt_ioctl.c   |  20 
 drivers/usb/misc/sisusbvga/sisusb_con.c |   1 +
 drivers/video/console/Kconfig   |  22 +++-
 drivers/video/console/dummycon.c|   1 +
 drivers/video/console/mdacon.c  |   6 +
 drivers/video/console/newport_con.c |   1 +
 drivers/video/console/sticon.c  |   7 ++
 drivers/video/console/vgacon.c  | 195 ++--
 include/linux/console.h |   1 +
 include/uapi/linux/vt.h |   1 +
 10 files changed, 194 insertions(+), 61 deletions(-)

-- 
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 0/2] Persistent scrollback buffers for all VGA consoles

2016-04-18 Thread Manuel Schölling
Another attempt to add persistent scrollback buffers for all VGA consoles,
so the buffer is not flushed when swithing back and forth between consoles.

Note that breaks tools like bash's clear_console and thus might have
security implications:
clear_console relies on this 'anti-feature' of the kernel to clear the buffer
when consoles are switched.

To offer a way for userland tools to flush the buffer my second patch adds
a ioctl call for that.
Also this feature is disabled by default and security implications are clearly
stated in its documentation.

Manuel Schölling (2):
  console: Add persistent scrollback buffers for all VGA consoles
  console: Add ioctl for flushing the scrollback buffer

 drivers/tty/vt/vt_ioctl.c   |  20 
 drivers/usb/misc/sisusbvga/sisusb_con.c |   1 +
 drivers/video/console/Kconfig   |  22 +++-
 drivers/video/console/dummycon.c|   1 +
 drivers/video/console/mdacon.c  |   6 +
 drivers/video/console/newport_con.c |   1 +
 drivers/video/console/sticon.c  |   7 ++
 drivers/video/console/vgacon.c  | 195 ++--
 include/linux/console.h |   1 +
 include/uapi/linux/vt.h |   1 +
 10 files changed, 194 insertions(+), 61 deletions(-)

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


Re: [PATCH 2/2] console: Add ioctl for flushing the scrollback buffer

2016-04-18 Thread kbuild test robot
Hi Manuel,

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.6-rc4 next-20160418]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Manuel-Sch-lling/Persistent-scrollback-buffers-for-all-VGA-consoles/20160419-040228
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
usb-testing
config: mips-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips 

All errors (new ones prefixed by >>):

>> drivers/video/console/newport_con.c:741:2: error: request for member 
>> 'con_flush_scrollback' in something not a structure or union
 .con_flush_scrollback = DUMMY,
 ^

vim +/con_flush_scrollback +741 drivers/video/console/newport_con.c

   735  .con_font_set = newport_font_set,
   736  .con_font_default = newport_font_default,
   737  .con_set_palette  = newport_set_palette,
   738  .con_scrolldelta  = newport_scrolldelta,
   739  .con_set_origin   = DUMMY,
   740  .con_save_screen  = DUMMY
 > 741  .con_flush_scrollback = DUMMY,
   742  };
   743  
   744  static int newport_probe(struct gio_device *dev,

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data


Greetings!!!

2016-04-18 Thread andreas122
Hi, how are you? My name is J Eric Denials, External Financial Auditor at 
Lloyds Banking Group plc., London. It is a pleasure to contact you at this time 
through this medium. I have a cool and legitimate deal to do with you as you're 
a foreigner, it will be mutually beneficial to both. If you’re interested, 
urgently, get back to me for more explanation about the deal.
Regards
Eric
--
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


[balbi-usb:next 26/45] drivers/usb/dwc3/gadget.c:757:3: error: expected ')' before 'dep'

2016-04-18 Thread kbuild test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
head:   761aa987a847e0e8b9a8d806f1be82217b88b0d3
commit: ff96650d99902fd63d4511e6d9047e34e6aa37f2 [26/45] usb: dwc3: get rid of 
DWC3_TRB_MASK
config: i386-randconfig-r0-201616 (attached as .config)
reproduce:
git checkout ff96650d99902fd63d4511e6d9047e34e6aa37f2
# save the attached .config to linux build tree
make ARCH=i386 

Note: the balbi-usb/next HEAD 761aa987a847e0e8b9a8d806f1be82217b88b0d3 builds 
fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_one_trb':
>> drivers/usb/dwc3/gadget.c:757:3: error: expected ')' before 'dep'
  dep->trb_enqueue++;
  ^
>> drivers/usb/dwc3/gadget.c:808:1: error: expected expression before '}' token
}
^

vim +757 drivers/usb/dwc3/gadget.c

eeb720fb21 Felipe Balbi   2011-11-28  751   }
c71fc37c19 Felipe Balbi   2011-11-22  752  
53fd88189e Felipe Balbi   2016-04-04  753   dep->trb_enqueue++;
5cd8c48d95 Zhuang Jin Can 2014-05-16  754   /* Skip the LINK-TRB on ISOC */
ff96650d99 Felipe Balbi   2016-04-05  755   if dep->trb_enqueue % 
DWC3_TRB_NUM) == DWC3_TRB_NUM - 1) &&
5cd8c48d95 Zhuang Jin Can 2014-05-16  756   
usb_endpoint_xfer_isoc(dep->endpoint.desc))
53fd88189e Felipe Balbi   2016-04-04 @757   dep->trb_enqueue++;
e5ba5ec833 Pratyush Anand 2013-01-14  758  
f6bafc6a1c Felipe Balbi   2012-02-06  759   trb->size = 
DWC3_TRB_SIZE_LENGTH(length);
f6bafc6a1c Felipe Balbi   2012-02-06  760   trb->bpl = lower_32_bits(dma);
f6bafc6a1c Felipe Balbi   2012-02-06  761   trb->bph = upper_32_bits(dma);
c71fc37c19 Felipe Balbi   2011-11-22  762  
16e78db720 Ido Shayevitz  2012-03-12  763   switch 
(usb_endpoint_type(dep->endpoint.desc)) {
c71fc37c19 Felipe Balbi   2011-11-22  764   case USB_ENDPOINT_XFER_CONTROL:
f6bafc6a1c Felipe Balbi   2012-02-06  765   trb->ctrl = 
DWC3_TRBCTL_CONTROL_SETUP;
c71fc37c19 Felipe Balbi   2011-11-22  766   break;
c71fc37c19 Felipe Balbi   2011-11-22  767  
c71fc37c19 Felipe Balbi   2011-11-22  768   case USB_ENDPOINT_XFER_ISOC:
e5ba5ec833 Pratyush Anand 2013-01-14  769   if (!node)
f6bafc6a1c Felipe Balbi   2012-02-06  770   trb->ctrl = 
DWC3_TRBCTL_ISOCHRONOUS_FIRST;
e5ba5ec833 Pratyush Anand 2013-01-14  771   else
e5ba5ec833 Pratyush Anand 2013-01-14  772   trb->ctrl = 
DWC3_TRBCTL_ISOCHRONOUS;
ca4d44ea2a Felipe Balbi   2016-03-10  773  
ca4d44ea2a Felipe Balbi   2016-03-10  774   /* always enable 
Interrupt on Missed ISOC */
ca4d44ea2a Felipe Balbi   2016-03-10  775   trb->ctrl |= 
DWC3_TRB_CTRL_ISP_IMI;
c71fc37c19 Felipe Balbi   2011-11-22  776   break;
c71fc37c19 Felipe Balbi   2011-11-22  777  
c71fc37c19 Felipe Balbi   2011-11-22  778   case USB_ENDPOINT_XFER_BULK:
c71fc37c19 Felipe Balbi   2011-11-22  779   case USB_ENDPOINT_XFER_INT:
f6bafc6a1c Felipe Balbi   2012-02-06  780   trb->ctrl = 
DWC3_TRBCTL_NORMAL;
c71fc37c19 Felipe Balbi   2011-11-22  781   break;
c71fc37c19 Felipe Balbi   2011-11-22  782   default:
c71fc37c19 Felipe Balbi   2011-11-22  783   /*
c71fc37c19 Felipe Balbi   2011-11-22  784* This is only 
possible with faulty memory because we
c71fc37c19 Felipe Balbi   2011-11-22  785* checked it already :)
c71fc37c19 Felipe Balbi   2011-11-22  786*/
c71fc37c19 Felipe Balbi   2011-11-22  787   BUG();
c71fc37c19 Felipe Balbi   2011-11-22  788   }
c71fc37c19 Felipe Balbi   2011-11-22  789  
ca4d44ea2a Felipe Balbi   2016-03-10  790   /* always enable Continue on 
Short Packet */
f6bafc6a1c Felipe Balbi   2012-02-06  791   trb->ctrl |= DWC3_TRB_CTRL_CSP;
ca4d44ea2a Felipe Balbi   2016-03-10  792  
ca4d44ea2a Felipe Balbi   2016-03-10  793   if (!req->request.no_interrupt)
ca4d44ea2a Felipe Balbi   2016-03-10  794   trb->ctrl |= 
DWC3_TRB_CTRL_IOC | DWC3_TRB_CTRL_ISP_IMI;
ca4d44ea2a Felipe Balbi   2016-03-10  795  
ca4d44ea2a Felipe Balbi   2016-03-10  796   if (last)
f6bafc6a1c Felipe Balbi   2012-02-06  797   trb->ctrl |= 
DWC3_TRB_CTRL_LST;
f6bafc6a1c Felipe Balbi   2012-02-06  798  
e5ba5ec833 Pratyush Anand 2013-01-14  799   if (chain)
e5ba5ec833 Pratyush Anand 2013-01-14  800   trb->ctrl |= 
DWC3_TRB_CTRL_CHN;
e5ba5ec833 Pratyush Anand 2013-01-14  801  
16e78db720 Ido Shayevitz  2012-03-12  802   if 
(usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable)
f6bafc6a1c Felipe Balbi   2012-02-06  803   trb->ctrl |= 
DWC3_TRB_CTRL_SID_SOFN(req->request.stream_id);
c71fc37c19 Felipe Balbi   2011-11-22  804  
f6bafc6a1c Felipe Balbi   2012-02-06  805   trb->ctrl |= DWC3_TRB_CTRL_HWO;
2c4cbe6e5a Felipe Balbi   2014-04-30  806  
2c4cbe6e5a Felipe B

Re: [PATCH 0/2] Persistent scrollback buffers for all VGA consoles

2016-04-18 Thread Jakub Wilk

* Manuel Schölling , 2016-04-18, 21:56:
To offer a way for userland tools to flush the buffer my second patch 
adds a ioctl call for that.


In f8df13e0a901fe55631fed66562369b4dba40f8b, the escape sequence \e[3J 
was added, which erases the whole display including the scroll-back 
buffer. So the new ioctl shouldn't be necessary.


--
Jakub Wilk
--
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: DWC3: Clock Domain Crossing erratum description.

2016-04-18 Thread Kirill Dronov

Hi,
On 04/12/2016 09:33 AM, Felipe Balbi wrote:

Hi,

Kirill Dronov  writes:

Hi Felipe,
On 04/07/2016 08:10 AM, Felipe Balbi wrote:

Hi,

Kirill Dronov  writes:

I'm working on USB device bringup on Intel E3800 – based board. DWC3
core configured as DRD in device mode. The only connected device phy
is SMSC 3310 (USB2 ULPI). DWC3 core version is 2.10A. Gadget zero
driver can be loaded, but device enumeration fails: device is detected
by host, speed is negotiated (HS), host successfully reset device. On
device side – conndone interrupt is followed by linksts change with
link_state 0.  Then host sends USBREQ_GET_DESCRIPTOR and tries to set
address but device does not react – no events generated.

which kernel are you using ?

It's 3.14.29 kernel (WindRiver Linux). I have backported dwc3 upstream

you really need to ask support from WindRiver then. v3.14 is a really
old kernel and even v3.14.29 is over a year old. There's very little
linux-usb can do to help you. You are paying for support from WindRiver,
right ? Might as well use it ;-)


patches up to ~v3.17-rc4 (+ all trace patches). But the same issue
exists for original 3.14.29 kernel.


Please collect driver traces and send them
to us:

# mount -t debugfs none /sys/kernel/debug
# cd /sys/kernel/debug/tracing
# echo 2048 > buffer_size_kb
# echo 1 > events/dwc3/enable

(now trigger the problem)

# cp trace /path/to/non/volatile/media/

Send that trace file (you need to compress it, probably)

I've attached both dev_dbg and trace outputs. Also attached regdump
taken after enumeration failed and gzero is suspended.

yeah, here's the thing:

DCFG = 0x00480804

This tells me that maximum-speed is set to SuperSpeed which matches the
requirements to avoid the metastability problem below. This is even well
commented in the source code.


I'm not sure if I hit “run/stop metastability” issue [“STAR#9000525659:
Clock Domain Crossing on DCTL in USB 2.0 Mode”]. I don't have DWC3

we have a workaround for that, you shouldn't hit it unless you removed
the workaround.

No, I didn't.

yeah, so you won't hit it ;-)


databook or erratum description (other than mentioned in driver code).
Can somebody provide more detailed description of STAR#9000525659?
Where can I get DWC3 Databook?

you need to talk to a Synopsys representative for that.


Unfortunately Synopsis points out to Intel as SoC manufacturer and Intel
share only short notes on driver configuration and loading - no Databook.

right, you need to talk to your lawyer to figure out what are the
requirements, nothing I can do ;-)

In any case, you might wanna try latest mainline and see if you hit the
problem. My bet is that you won't.
I've tried latest mainline: 4.6.0-rc3+ (HEAD of 
github.com/torvalds/linux.git).


Actually the same issue with enumeration is present there (both x86 and 
x86_64 versions).
I've attached x86_64 logs just in case. No dwc quirks or erratum enabled 
in probe.





BTW, your trace output looks really odd. Seems like you're getting a ton
of spurious IRQs.
Well, I see lot of dwc3_gadget_linksts_change_interrupt() but I don't 
know if such states sequence is correct/expected:

upon connection: U0 -> Rx.Detect->SS.Disabled->U0->Rx.Detect->U3
Then gadget is reset by host controller, conndone interrupt generated,
then link changes U3->U0, host reports "new high-speed USB device" and 
tries to send USB_REQ_GET_DESCRIPTOR control message - and get error 
device descriptor read/64, error -71

No events generated on gadget.
Then host resets gadget that leads to link change U0->Rx.Detect, reset, 
conndone interrupts and link change Rx.Detect->U0. And new attempt of 
USB_REQ_GET_DESCRIPTOR.


I have IP v.210a with both USB3_SSPHY_INTERFACE and USB3_HSPHY_INTERFACE 
configured (HSPHY as ULPI) but only USB2 phy attached. Can this cause 
such enumeration issues?

Is something definitely wrong with initialization and enumeration traces?
I see that TUSB1210 was tested with Baytrail board. Are there any other 
known working Baytrail/phy2 combinations (my phy is smsc usb3310)?
Sorry for my generic questions - more specific can be answered by 
Databook that I don't have.



Oh well, you really wanna talk to WindRiver through
your official support channel; there's nothing this forum can do to help
you, sorry.


It seems that WindRiver kernel is not guilty in this case.
---
regards,
Kirill


dwc3_trace.tgz
Description: application/compressed-tar


Problems with MCS7715 USB-attached parallel port

2016-04-18 Thread Alex Henrie
Hi,

I recently bought an MCS7715 USB-attached parallel port,[1] but there
seem to be a couple of problems using it with Linux:

1. The lp, parport, and parport_pc kernel modules are not loaded when
the device is plugged in.
2. After manually loading the kernel modules, /dev/lp0 is not deleted
when the device is unplugged.

I'm using a fully updated copy of Arch Linux, and at first I thought
it was a udev rules problem, but the systemd guys think that it's
actually a kernel bug.[2] Is this something that you could fix, or
guide me thorough fixing? I'd be happy to donate one of these devices
if you don't have one handy for debugging.

-Alex

[1] http://www.newegg.com/Product/Product.aspx?Item=N82E16812709411
[2] 
https://lists.freedesktop.org/archives/systemd-devel/2016-February/035937.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: Problems with MCS7715 USB-attached parallel port

2016-04-18 Thread Greg KH
On Mon, Apr 18, 2016 at 06:11:51PM -0600, Alex Henrie wrote:
> Hi,
> 
> I recently bought an MCS7715 USB-attached parallel port,[1] but there
> seem to be a couple of problems using it with Linux:
> 
> 1. The lp, parport, and parport_pc kernel modules are not loaded when
> the device is plugged in.
> 2. After manually loading the kernel modules, /dev/lp0 is not deleted
> when the device is unplugged.
> 
> I'm using a fully updated copy of Arch Linux, and at first I thought
> it was a udev rules problem, but the systemd guys think that it's
> actually a kernel bug.[2] Is this something that you could fix, or
> guide me thorough fixing? I'd be happy to donate one of these devices
> if you don't have one handy for debugging.

Ah, yeah, this isn't going to work well, as you have found out.

The parallel port subsystem is one of the last ones to be moved to the
driver core, it just now started for the 4.4 kernel release, and isn't
done yet at all.

Sudip is the new maintainer of this, he might be able to answer this
much better than I can.  Sudip?

> -Alex
> 
> [1] http://www.newegg.com/Product/Product.aspx?Item=N82E16812709411
> [2] 
> https://lists.freedesktop.org/archives/systemd-devel/2016-February/035937.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 v6 01/12] usb: hcd: Initialize hcd->flags to 0

2016-04-18 Thread Peter Chen
On Mon, Apr 18, 2016 at 10:11:29AM -0400, Alan Stern wrote:
> On Mon, 18 Apr 2016, Peter Chen wrote:
> 
> > On Wed, Apr 06, 2016 at 09:32:22AM +0300, Roger Quadros wrote:
> > > On 06/04/16 09:09, Felipe Balbi wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > Roger Quadros  writes:
> > > >> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > > >> index 2ca2cef..6b1930d 100644
> > > >> --- a/drivers/usb/core/hcd.c
> > > >> +++ b/drivers/usb/core/hcd.c
> > > >> @@ -2706,6 +2706,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
> > > >>int retval;
> > > >>struct usb_device *rhdev;
> > > >>  
> > > >> +  hcd->flags = 0;
> > > > 
> > > > seems like this would make more sense in usb_del_hcd() instead.
> > > > 
> > > 
> > > OK, I'll move it there.
> > > 
> > 
> > It depends on Alan's comments, whether only usb_add_hcd/usb_del_hcd
> > pair can be called repeat. If Alan acks it, I have no idea for it.
> 
> Most of the host controller drivers were not written with this in mind,
> but I think it would be a good thing to allow.  It would speed up the
> host/device role switches.
> 
> This might mean we need to fix up several drivers to make them work 
> correctly in an OTG environment.  It should be possible to do this.  Is 
> there any particular reason why it would be difficult for Chipidea?
> 

I just want to do clean remove at OTG environment, like rmmod, so I did
this when I worked on chipidea OTG design.
I am worried if there are some resources dedicated for host device, eg,
clocks, gpio. etc.

If OTG framework can know well hcd's add and remove, it is ok for chipidea
just calling usb_add_hcd/usb_del_hcd currently, but I suggested roger
adding platform hcd_ops as optional parameter in case the platform
has special requirement for hcd_ops.

-- 

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 1/2] usb: chipidea: add flag CI_HDRC_DP_ALWAYS_PULLUP

2016-04-18 Thread Peter Chen
On Mon, Apr 18, 2016 at 02:36:06PM +0530, maitysancha...@gmail.com wrote:
> Hello Peter,
> 
> I tested this on a Colibri Vybrid VF61 module with the patches applied
> on top of for-next branch.
> 
> root@colibri-vf:~# uname -a
> Linux colibri-vf 4.6.0-rc1-00044-gc76529e-dirty #120 Mon Apr 18 13:46:34 IST 
> 2016 armv7l GNU/Linux
> 
> Host and client mode work only on boot up. Trying to change the mode by 
> disconnecting/reconnecting
> the cable or plugging in a USB device does not work. Also when the USB client 
> mode is working on
> boot up, disconnecting the cable results in the below stack trace.
> 
> root@colibri-vf:~# ping 192.168.11.234
> 
> PING 192.168.11.234 (192.168.11.234): 56 data bytes
> 64 bytes from 192.168.11.234: seq=0 ttl=64 time=5.399 ms
> ^C
> --- 192.168.11.234 ping statistics ---
> 1 packets transmitted, 1 packets received, 0% packet loss
> round-trip min/avg/max = 5.399/5.399/5.399 ms
> - On Cable disconnection-
> root@colibri-vf:~# [   23.923607] irq 35: nobody cared (try booting with the 
> "irqpoll" option)
> [   23.930354] CPU: 0 PID: 0 Comm: swapper Not tainted 
> 4.6.0-rc1-00044-gc76529e-dirty #120
> [   23.938359] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
> [   23.944807] Backtrace: 
> [   23.947327] [<8010b56c>] (dump_backtrace) from [<8010b764>] 
> (show_stack+0x18/0x1c)
> [   23.954900]  r7:0023 r6: r5: r4:8f4ad240
> [   23.960644] [<8010b74c>] (show_stack) from [<803a1c00>] 
> (dump_stack+0x24/0x28)
> [   23.967893] [<803a1bdc>] (dump_stack) from [<80148150>] 
> (__report_bad_irq+0x30/0xb8)
> [   23.975655] [<80148120>] (__report_bad_irq) from [<801484a8>] 
> (note_interrupt+0x260/0x2b0)
> [   23.983918]  r5: r4:8f4ad240
> [   23.987538] [<80148248>] (note_interrupt) from [<801460cc>] 
> (handle_irq_event_percpu+0xe0/0x154)
> [   23.996328]  r10: r9:80b35683 r8:8f4ad240 r7:0023 r6: 
> r5:
> [   24.004242]  r4: r3:
> [   24.007860] [<80145fec>] (handle_irq_event_percpu) from [<80146170>] 
> (handle_irq_event+0x30/0x44)
> [   24.016732]  r10:80b0210c r9:90003100 r8:8f406000 r7:80b01f10 r6: 
> r5:80b14ed0
> [   24.024646]  r4:8f4ad240
> [   24.027206] [<80146140>] (handle_irq_event) from [<80148d2c>] 
> (handle_fasteoi_irq+0xa8/0x170)
> [   24.035737]  r5:80b14ed0 r4:8f4ad240
> [   24.039363] [<80148c84>] (handle_fasteoi_irq) from [<801457a0>] 
> (generic_handle_irq+0x2c/0x3c)
> [   24.047973]  r7:80b01f10 r6: r5:0023 r4:80b14d8c
> [   24.053706] [<80145774>] (generic_handle_irq) from [<80145a30>] 
> (__handle_domain_irq+0x5c/0xb0)
> [   24.062420] [<801459d4>] (__handle_domain_irq) from [<801013d0>] 
> (gic_handle_irq+0x50/0x84)
> [   24.070771]  r9:90003100 r8:80b01e00 r7:90002100 r6:9000210c r5:80b023b0 
> r4:80b14ec0
> [   24.078611] [<80101380>] (gic_handle_irq) from [<8010c214>] 
> (__irq_svc+0x54/0x70)
> [   24.086101] Exception stack(0x80b01e00 to 0x80b01e48)
> [   24.091173] 1e00: 80b360c0 0020 80b36080  0002 0010 
>  
> [   24.099363] 1e20: 8f406000 90003100 80b0210c 80b01eac 0020 80b01e50 
> 8011cf18 8011caec
> [   24.107545] 1e40: 600c0113 
> [   24.111039]  r9:90003100 r8:8f406000 r7:80b01e34 r6: r5:600c0113 
> r4:8011caec
> [   24.118892] [<8011ca4c>] (__do_softirq) from [<8011cf18>] 
> (irq_exit+0xb8/0xf4)
> [   24.126114]  r10:80b0210c r9:90003100 r8:8f406000 r7: r6: 
> r5:0010
> [   24.134028]  r4:80b14d8c
> [   24.136587] [<8011ce60>] (irq_exit) from [<80145a34>] 
> (__handle_domain_irq+0x60/0xb0)
> [   24.144429] [<801459d4>] (__handle_domain_irq) from [<801013d0>] 
> (gic_handle_irq+0x50/0x84)
> [   24.152782]  r9:90003100 r8:80b01f10 r7:90002100 r6:9000210c r5:80b023b0 
> r4:80b14ec0
> [   24.160621] [<80101380>] (gic_handle_irq) from [<8010c214>] 
> (__irq_svc+0x54/0x70)
> [   24.168110] Exception stack(0x80b01f10 to 0x80b01f58)
> [   24.173175] 1f00: 0001  
>  80115ac0
> [   24.181365] 1f20:  80b0210c  80b29c78 80b02114 80b0 
> 80b0210c 80b01f6c
> [   24.189554] 1f40: 80b01f70 80b01f60 80108200 80108204 600c0013 
> [   24.196173]  r9:80b0 r8:80b02114 r7:80b01f44 r6: r5:600c0013 
> r4:80108204
> [   24.204026] [<801081c4>] (arch_cpu_idle) from [<8013dea8>] 
> (default_idle_call+0x28/0x34)
> [   24.212134] [<8013de80>] (default_idle_call) from [<8013e018>] 
> (cpu_startup_entry+0x164/0x1c0)
> [   24.220776] [<8013deb4>] (cpu_startup_entry) from [<806fa2a4>] 
> (rest_init+0x78/0x7c)
> [   24.228518]  r7:
> [   24.231097] [<806fa22c>] (rest_init) from [<80a00ce0>] 
> (start_kernel+0x370/0x37c)
> [   24.238599] [<80a00970>] (start_kernel) from [<80008078>] (0x80008078)
> [   24.245127] handlers:
> [   24.247415] [<804f2dcc>] ci_irq
> [   24.250578]