[RFC] xhci: fix dma mask setup in xhci.c

2013-05-26 Thread Xenia Ragiadakou
This patch adds a check on whether the host machine
supports the xHC DMA address mask and sets the DMA
mask for coherent DMA address allocation via an
explicit call to dma_set_coherent_mask().

According to DMA-API-HOWTO, if coherent DMA address
mask has not been set explicitly via dma_set_coherent_mask(),
and the driver calls dma_alloc_coherent() or
dma_pool_create() to allocate consistent DMA memory
blocks, the consistent DMA mapping interface will
return by default DMA addresses which are 32-bit
addressable.

Hence, if 64-bit DMA mapping is supported, it
is appropriate to call dma_set_coherent_mask()
with DMA_BIT_MASK(64) to take advantage of it.

Also, according to DMA-API-HOWTO, dma_set_coherent_mask()
is guaranteed to set successfully the same or a smaller
mask as dma_set_mask().

Signed-off-by: Xenia Ragiadakou 
---

After this change, (i have a 64-bit machine) the
consistent_dma_mask_bits entry for xhci controller
that appears under:
/sys/devices/pci:00/:00:/
changed from 32 to 64.
I did not find another way to test this change.

 drivers/usb/host/xhci.c |   32 ++--
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b4aa79d..887db65 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4662,11 +4662,21 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
 */
xhci = hcd_to_xhci(hcd);
temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params);
-   if (HCC_64BIT_ADDR(temp)) {
+   /*
+* Check if host machine supports 64 bit DMA address mask
+* and enable it for both streaming and coherent DMA transfers.
+* Otherwise, use 32bit DMA mask, if it is supported.
+*/
+   if (HCC_64BIT_ADDR(temp) &&
+   !dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64))) {
xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
-   dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64));
+   dma_set_coherent_mask(hcd->self.controller,
+ DMA_BIT_MASK(64));
} else {
-   dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32));
+   if (dma_set_mask(hcd->self.controller, 
DMA_BIT_MASK(32)))
+   goto error;
+   dma_set_coherent_mask(hcd->self.controller,
+ DMA_BIT_MASK(32));
}
return 0;
}
@@ -4700,11 +4710,21 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
xhci_dbg(xhci, "Reset complete\n");
 
temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params);
-   if (HCC_64BIT_ADDR(temp)) {
+   /*
+* Check if host machine supports 64 bit DMA address mask
+* and enable it for both streaming and coherent DMA transfers.
+* Otherwise, use 32bit DMA mask, if it is supported.
+*/
+   if (HCC_64BIT_ADDR(temp) &&
+   !dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64))) {
xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
-   dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64));
+   dma_set_coherent_mask(hcd->self.controller,
+ DMA_BIT_MASK(64));
} else {
-   dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32));
+   if (dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32)))
+   goto error;
+   dma_set_coherent_mask(hcd->self.controller,
+ DMA_BIT_MASK(32));
}
 
xhci_dbg(xhci, "Calling HCD init\n");
-- 
1.7.10.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] MOXA UPORT Serial USB driver

2013-05-26 Thread Andrew Lunn
These two patches add support for MOXA 12XX/14XX/16XX USB serial
devices. The first patch exports a function from the generic usb
serial driver, which the driver in the second patch would like to use.

Andrew Lunn (2):
  USB: Serial: export usb_serial_generic_submit_read_urb()
  usb-serial: Moxa UPORT 12XX/14XX/16XX driver

 drivers/usb/serial/Kconfig   |   30 +
 drivers/usb/serial/Makefile  |1 +
 drivers/usb/serial/generic.c |5 +-
 drivers/usb/serial/mxuport.c | 2005 ++
 drivers/usb/serial/mxuport.h |  184 
 include/linux/usb/serial.h   |2 +
 6 files changed, 2225 insertions(+), 2 deletions(-)
 create mode 100644 drivers/usb/serial/mxuport.c
 create mode 100644 drivers/usb/serial/mxuport.h

-- 
1.7.10.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 1/2] USB: Serial: export usb_serial_generic_submit_read_urb()

2013-05-26 Thread Andrew Lunn
USB Serial drivers can currently use the generic function
usb_serial_generic_submit_read_urbs() to submit a group of urbs when
starting up the device. The read bulk callback needs to be able to
submit a single urb. usb_serial_generic_submit_read_urb() does this,
but it is not exported and so only usable within generic functions,
in particular usb_serial_generic_read_bulk_callback().

Export this function so drivers can use it in there own read bulk
callback routine.

Signed-off-by: Andrew Lunn 
---
 drivers/usb/serial/generic.c |5 +++--
 include/linux/usb/serial.h   |2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 297665f..13e9b3b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -253,8 +253,8 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct 
*tty)
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_chars_in_buffer);
 
-static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
-   int index, gfp_t mem_flags)
+int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
+  int index, gfp_t mem_flags)
 {
int res;
 
@@ -276,6 +276,7 @@ static int usb_serial_generic_submit_read_urb(struct 
usb_serial_port *port,
 
return 0;
 }
+EXPORT_SYMBOL_GPL(usb_serial_generic_submit_read_urb);
 
 int usb_serial_generic_submit_read_urbs(struct usb_serial_port *port,
gfp_t mem_flags)
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index b9b0f7b4..a85e35b 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -337,6 +337,8 @@ extern int usb_serial_generic_get_icount(struct tty_struct 
*tty,
struct serial_icounter_struct *icount);
 extern int usb_serial_generic_register(void);
 extern void usb_serial_generic_deregister(void);
+extern int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
+ int index, gfp_t mem_flags);
 extern int usb_serial_generic_submit_read_urbs(struct usb_serial_port *port,
 gfp_t mem_flags);
 extern void usb_serial_generic_process_read_urb(struct urb *urb);
-- 
1.7.10.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: [RFC 6/6] usb: check usb_hub_to_struct_hub() return value

2013-05-26 Thread Alan Stern
On Fri, 24 May 2013, Sarah Sharp wrote:

> From: Mathias Nyman 
> 
> usb_hub_to_struct_hub() can return NULL in the unlikely cases a hub
> without active configuration, or a hub without ports is found.

I think the proper way to fix the last part is to prevent the hub
driver from binding to a hub with no ports.  Then it will be guaranteed
that usb_hub_to_struct_hub() will never return NULL whenever the hub
driver is bound to the device in question.

Even now, we are guaranteed that the return value will not be NULL if
the device has any child USB devices or any ports (i.e., if its
maxchild value is larger than 0).  Therefore almost all of the checks
added in this patch are not necessary.

> Even if unlikely we need to handle those cases properly.

However, we don't have to if they are not just unlikely but impossible.

> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -730,7 +730,12 @@ int usb_hub_set_port_power(struct usb_device *hdev, int 
> port1,
>  {
>   int ret;
>   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
> - struct usb_port *port_dev = hub->ports[port1 - 1];
> + struct usb_port *port_dev;
> +
> + if (!hub)
> + return -EINVAL;

This routine is called from only two places, and in both places it is 
known that the return value isn't NULL.  On the other hand, it might 
make more sense to pass the hub pointer as an additional argument to 
usb_hub_set_port_power(), so the issue doesn't arise at all.

> @@ -964,6 +969,10 @@ int usb_remove_device(struct usb_device *udev)
>   if (!udev->parent)  /* Can't remove a root hub */
>   return -EINVAL;
>   hub = usb_hub_to_struct_hub(udev->parent);
> +
> + if (!hub)
> + return -EINVAL;

Not needed since we know there is a child device.

> @@ -1733,6 +1742,9 @@ hub_ioctl(struct usb_interface *intf, unsigned int 
> code, void *user_data)
>   struct usb_device *hdev = interface_to_usbdev (intf);
>   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
>  
> + if (!hub)
> + return -EINVAL;

This should be fixed by not binding to hubs with no ports.

> @@ -1769,15 +1781,17 @@ hub_ioctl(struct usb_interface *intf, unsigned int 
> code, void *user_data)
>  static int find_port_owner(struct usb_device *hdev, unsigned port1,
>   struct dev_state ***ppowner)
>  {
> + struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
> +
>   if (hdev->state == USB_STATE_NOTATTACHED)
>   return -ENODEV;
> - if (port1 == 0 || port1 > hdev->maxchild)
> + if (port1 == 0 || port1 > hdev->maxchild || !hub)
>   return -EINVAL;

The test is not needed since the hdev->maxchild cannot be larger than 0
if the hub driver isn't bound.

>   /* This assumes that devices not managed by the hub driver
>* will always have maxchild equal to 0.
>*/
> - *ppowner = &(usb_hub_to_struct_hub(hdev)->ports[port1 - 1]->port_owner);
> + *ppowner = &(hub->ports[port1 - 1]->port_owner);

This change is okay.  The comment should be updated as well, since the
fact it refers to is not an assumption but is guaranteed.

Most of the remaining changes in this patch aren't needed, for reasons
already mentioned.

> @@ -5323,7 +5367,8 @@ void usb_set_hub_port_connect_type(struct usb_device 
> *hdev, int port1,
>  {
>   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
>  
> - hub->ports[port1 - 1]->connect_type = type;
> + if (hub)
> + hub->ports[port1 - 1]->connect_type = type;
>  }
>  
>  /**
> @@ -5339,6 +5384,9 @@ usb_get_hub_port_connect_type(struct usb_device *hdev, 
> int port1)
>  {
>   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
>  
> + if (!hub)
> + return USB_PORT_CONNECT_TYPE_UNKNOWN;
> +
>   return hub->ports[port1 - 1]->connect_type;
>  }
>  
> @@ -5397,6 +5445,9 @@ acpi_handle usb_get_hub_port_acpi_handle(struct 
> usb_device *hdev,
>  {
>   struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
>  
> + if (!hub)
> + return NULL;
> +
>   return DEVICE_ACPI_HANDLE(&hub->ports[port1 - 1]->dev);
>  }

These three changes probably aren't needed either, but verifying it is 
a little more complicated since the callers are in a different source 
file.  I guess it won't hurt to keep them.

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 2/2] usb-serial: Moxa UPORT 12XX/14XX/16XX driver

2013-05-26 Thread Andrew Lunn
On Sun, May 26, 2013 at 06:04:24PM +0200, Johan Hovold wrote:
> On Sun, May 26, 2013 at 01:00:51PM +0200, Andrew Lunn wrote:
> > Add a driver which supports the following Moxa USB to serial converters:
> > *   2 ports : UPort 1250, UPort 1250I
> > *   4 ports : UPort 1410, UPort 1450, UPort 1450I
> > *   8 ports : UPort 1610-8, UPort 1650-8
> > *  16 ports : UPort 1610-16, UPort 1650-16
> > 
> > The UPORT devices don't directy fit the USB serial model. USB serial
> > assumes a bulk in/out endpoint pair per serial port. Thus a dual port
> > USB serial device is expected to have two bulk in/out pairs. The Moxa
> > UPORT only has one pair for data transfer and places a header on each
> > transfer over the endpoint indicating for which port the transfer
> > relates to. There is a second endpoint pair for events, just as modem
> > control lines changing state, setting baud rates etc. Again, a
> > multiplexing header is used on these endpoints.
> >
> > This difference to the model results in some additional code which
> > other drivers don't have:
> > 
> > Some ports need to have a kfifo explicitily allocated since the
> > framework does not allocate one if there is no associated endpoints.
> > The framework will however free it on unload of the module.
> > 
> > All data transfers are made on port0, yet the locks are taken on PortN.
> > urb->context points to PortN, even though the URL is for port0.
> 
> First, thanks for submitting the driver.

And thanks for the quick review.

> As you mention, it does not fit
> the usb-serial model directly, but it seems that there's still quite a
> bit more of generic code that could be reused rather than copied or
> reimplemented.

I will take another look. I was not happy with cut/pasting code from
the generic driver, so tried to minimize it. I will take a deeper look
at your suggestions with the endpoints. 
 
> Consider my comments as a first round of review, there's probably some
> things I've missed and more minor things I'll get back to later.

Sure, no problem. 
 
> > The driver will attempt to load firmware from userspace and compare
> > the available version and the running version. If the available
> > version is newer, it will be download into RAM of the device and
> > started. This is optional and currently the firmware blobs are not yet
> > in linux-firmware. However MOXA have agreed that the blobs can be
> > distributed.
> > 
> > This driver is based on the MOXA driver and retains MOXAs copyright.
> 
> The driver is also quite heavily based on the generic driver and much
> code is copied more or less verbatim. Please make sure to mention in
> this in the header as well.

O.K.

Also, most of the paranoid checks for NULL pointers and closed ports
is from the original MOXA driver. Maybe in the past they have had
problems. I've already take out a lot of dead code, and converted to
generic where i could. I'm happy to continue this.

> > +/* Global variables */
> > +static bool debug;
> 
> Please drop the debug module parameter. usb_serial_debug_data is
> supposed to be controlled using dynamic debugging.

O.K.
 
> > +
> > +/* Table of devices that work with this driver */
> > +static struct usb_device_id mxuport_idtable[] = {
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1250_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1251_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1410_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1450_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1451_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1618_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1658_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1613_PID)},
> > +   {USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID)},
> > +   {}  /* Terminating entry */
> > +};
> > +
> > +MODULE_DEVICE_TABLE(usb, mxuport_idtable);
> > +
> > +/*
> > + * mxuport_prepare_write_buffer - fill in the buffer, ready for
> > + * sending
> > + *
> > + * Add a four byte header containing the port number and the number of
> > + * bytes of data in the message. Return the number of bytes in the
> > + * buffer.
> > + */
> > +static int mxuport_prepare_write_buffer(struct usb_serial_port *port,
> > +   void *dest, size_t size)
> > +{
> > +   struct mxuport_port *mx_port = usb_get_serial_port_data(port);
> > +   unsigned char *buf = dest;
> > +   int count;
> > +
> > +   count = kfifo_out_locked(&port->write_fifo, buf + 4, size - 4,
> > +&port->lock);
> > +
> > +   put_unaligned_be16(mx_port->portno, buf);
> > +   put_unaligned_be16(count, buf + 2);
> > +
> > +   dev_dbg(&port->dev, "%s - port %d, size %d count %d", __func__,
> > +   mx_port->portno, size, count);
> > +
> > +   return count + 4;
> > +}
> > +
> > +/*
> > + * mxuport_write_start - kick off an URB write
> > + * @port:  Pointer to the &struct usb_serial_port data
> > + *
> > + * Returns zero on success, or a negati

[PATCH 13/24] drivers/uwb/whci: Convert to module_pci_driver

2013-05-26 Thread Libo Chen
use module_pci_driver instead of init/exit, make code clean.

Signed-off-by: Libo Chen 
---
 drivers/uwb/whci.c |   13 +
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/drivers/uwb/whci.c b/drivers/uwb/whci.c
index f48093e..deeeba4 100644
--- a/drivers/uwb/whci.c
+++ b/drivers/uwb/whci.c
@@ -253,18 +253,7 @@ static struct pci_driver whci_driver = {
.remove   = whci_remove,
 };
 
-static int __init whci_init(void)
-{
-   return pci_register_driver(&whci_driver);
-}
-
-static void __exit whci_exit(void)
-{
-   pci_unregister_driver(&whci_driver);
-}
-
-module_init(whci_init);
-module_exit(whci_exit);
+module_pci_driver(whci_driver);
 
 MODULE_DESCRIPTION("WHCI UWB Multi-interface Controller enumerator");
 MODULE_AUTHOR("Cambridge Silicon Radio Ltd.");
-- 
1.7.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 v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Chanwoo Choi
Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository (extcon-for-palmas).
- 
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c

This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.

On 05/24/2013 11:31 PM, Kishon Vijay Abraham I wrote:
> From: Graeme Gregory 
>
> This is the driver for the USB comparator built into the palmas chip. It
> handles the various USB OTG events that can be generated by cable
> insertion/removal.
>
> Signed-off-by: Graeme Gregory 
> Signed-off-by: Moiz Sonasath 
> Signed-off-by: Ruchika Kharwar 
> Signed-off-by: Kishon Vijay Abraham I 
> Signed-off-by: George Cherian 
> [kis...@ti.com: adapted palmas usb driver to use the extcon framework]
> Signed-off-by: Sebastien Guiriec 
> ---
> Changes from v4:
> * removed no_control_vbus property (to be used if the platform wants to use
> its own vbus control.
> * removed unnecessary headers
> * moved the palmas_usb_state to palmas.h
> * misc cleanups
> *A checkpatch warning "WARNING: static const char * array should
>  probably be static const char * const"is ignored since it introduces a
>  compilation warning.
> Changes from v3:
> * adapted the driver to extcon framework (so moved to drivers/extcon)
> * removed palmas_usb_(write/read) and replaced all calls with
>   palmas_(read/write).
> * ignored a checkpatch warning in the line 
>   static const char *palmas_extcon_cable[] = {
>   as it seemed to be incorrect?
> * removed all references to OMAP in this driver.
> * couldn't test this driver with mainline as omap5 panda is not booting
>   with mainline.
> * A comment to change to platform_get_irq from regmap is not done as I felt
>   the data should come from regmap in this case. Graeme?
> Changes from v2:
> * Moved the driver to drivers/usb/phy/
> * Added a bit more explanation in Kconfig
>  .../devicetree/bindings/extcon/extcon-twl.txt  |  16 +
>  drivers/extcon/Kconfig |   7 +
>  drivers/extcon/Makefile|   1 +
>  drivers/extcon/extcon-palmas.c | 341 
> +
>  include/linux/mfd/palmas.h |  25 +-
>  5 files changed, 380 insertions(+), 10 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/extcon/extcon-twl.txt
>  create mode 100644 drivers/extcon/extcon-palmas.c
>
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-twl.txt 
> b/Documentation/devicetree/bindings/extcon/extcon-twl.txt
> new file mode 100644
> index 000..8ffdeed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/extcon/extcon-twl.txt
> @@ -0,0 +1,15 @@
> +EXTCON FOR TWL CHIPS
> +
> +PALMAS USB COMPARATOR
> +Required Properties:
> + - compatible : Should be "ti,palmas-usb" or "ti,twl6035-usb"
> + - vbus-supply : phandle to the regulator device tree node.
> +
> +Optional Properties:
> + - ti,wakeup : To enable the wakeup comparator in probe
> +
> +palmas-usb {
> +   compatible = "ti,twl6035-usb", "ti,palmas-usb";
> +   vbus-supply = <&smps10_reg>;
> +   ti,wakeup;
> +};
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index 3297301..63f454e 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -53,4 +53,11 @@ config EXTCON_ARIZONA
> with Wolfson Arizona devices. These are audio CODECs with
> advanced audio accessory detection support.
>  
> +config EXTCON_PALMAS
> + tristate "Palmas USB EXTCON support"
> + depends on MFD_PALMAS
You should add REGULATOR_PALMAS dependency because
palmas_set_switch_smps10() is defined in palmas regulator driver.

+depends on MFD_PALMAS && REGULATOR_PALMAS

> + help
> +   Say Y here to enable support for USB peripheral and USB host
> +   detection by palmas usb.
> +
>  endif # MULTISTATE_SWITCH
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index f98a3c4..540e2c3 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_EXTCON_ADC_JACK) += extcon-adc-jack.o
>  obj-$(CONFIG_EXTCON_MAX77693)+= extcon-max77693.o
>  obj-$(CONFIG_EXTCON_MAX8997) += extcon-max8997.o
>  obj-$(CONFIG_EXTCON_ARIZONA) += extcon-arizona.o
> +obj-$(CONFIG_EXTCON_PALMAS)  += extcon-palmas.o
> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
> new file mode 100644
> index 000..9e613e9
> --- /dev/null
> +++ b/drivers/extcon/extcon-palmas.c
> @@ -0,0 +1,341 @@
> +/*
> + * Palmas USB transceiver driver
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> + * This program is free software; you can 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 ver

Re: [PATCH 3/3] usb: dwc3: use extcon fwrk to receive connect/disconnect notification

2013-05-26 Thread Chanwoo Choi
On 05/24/2013 11:31 PM, Kishon Vijay Abraham I wrote:
> Modified dwc3-omap to receive connect and disconnect notification using
> extcon framework. Also did the necessary cleanups required after
> adapting to extcon framework.
>
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/usb/dwc3/dwc3-omap.c  | 80 
> +--
>  include/linux/usb/dwc3-omap.h | 30 
>  2 files changed, 62 insertions(+), 48 deletions(-)
>  delete mode 100644 include/linux/usb/dwc3-omap.h
>
>
I check the code about extcon framework.

Acked-by: Chanwoo Choi 

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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Kishon Vijay Abraham I

Hi,

On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:

Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository (extcon-for-palmas).
- 
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c

This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.


yeah.. Still there is some confusion with palmas_set_switch_smps10(). I 
think we can remove it for now and add it separately later. By this at 
least we can have device mode fully functional in OMAP5. What do you think?


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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Chanwoo Choi
On 05/27/2013 02:54 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:
>> Hi Kishon,
>>
>> I have some comment about this patch
>> and upload modified patch to following repository (extcon-for-palmas).
>> - 
>> http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c
>>
>> This patchset include patch related to other module
>> ,so I need your opinion to apply this patchset to git repository.
>
> yeah.. Still there is some confusion with palmas_set_switch_smps10(). I think 
> we can remove it for now and add it separately later. By this at least we can 
> have device mode fully functional in OMAP5. What do you think?
>

 I agree your opinion.

But, I propose some fixes about palmas_set_switch_smps10().
I dont' prefer to call global function in exton-palmas.c from 
palmas-regulator.c.
So, Why don't you use regulator consumer instead of global function?
You can register specific regulator for enabling or disabling SMPS10_SWITCH_EN
and then control SMPS10_SWITCH_EN bit through regulator framework in 
extcon-palmas.c
without calling global function.

Thanks,
Chanwoo Choi

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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Laxman Dewangan

On Monday 27 May 2013 11:38 AM, Chanwoo Choi wrote:

On 05/27/2013 02:54 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:

Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository (extcon-for-palmas).
- 
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c

This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.

yeah.. Still there is some confusion with palmas_set_switch_smps10(). I think 
we can remove it for now and add it separately later. By this at least we can 
have device mode fully functional in OMAP5. What do you think?


  I agree your opinion.

But, I propose some fixes about palmas_set_switch_smps10().
I dont' prefer to call global function in exton-palmas.c from 
palmas-regulator.c.
So, Why don't you use regulator consumer instead of global function?
You can register specific regulator for enabling or disabling SMPS10_SWITCH_EN
and then control SMPS10_SWITCH_EN bit through regulator framework in 
extcon-palmas.c
without calling global function.


Along with this, I also like to make the VBUS regulator control to be 
optional here. Currently it is mandatory.

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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Kishon Vijay Abraham I

Hi,

On Monday 27 May 2013 11:52 AM, Laxman Dewangan wrote:

On Monday 27 May 2013 11:38 AM, Chanwoo Choi wrote:

On 05/27/2013 02:54 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:

Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository (extcon-for-palmas).
-
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c


This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.

yeah.. Still there is some confusion with palmas_set_switch_smps10().
I think we can remove it for now and add it separately later. By this
at least we can have device mode fully functional in OMAP5. What do
you think?


  I agree your opinion.

But, I propose some fixes about palmas_set_switch_smps10().
I dont' prefer to call global function in exton-palmas.c from
palmas-regulator.c.
So, Why don't you use regulator consumer instead of global function?
You can register specific regulator for enabling or disabling
SMPS10_SWITCH_EN
and then control SMPS10_SWITCH_EN bit through regulator framework in
extcon-palmas.c
without calling global function.


Along with this, I also like to make the VBUS regulator control to be
optional here. Currently it is mandatory.


But dint you just tell on my v4 of this patch that you don’t require this.
http://www.spinics.net/lists/linux-doc/msg10638.html

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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Laxman Dewangan

On Monday 27 May 2013 12:01 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:52 AM, Laxman Dewangan wrote:

On Monday 27 May 2013 11:38 AM, Chanwoo Choi wrote:

On 05/27/2013 02:54 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:

Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository (extcon-for-palmas).
-
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c


This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.

yeah.. Still there is some confusion with palmas_set_switch_smps10().
I think we can remove it for now and add it separately later. By this
at least we can have device mode fully functional in OMAP5. What do
you think?


   I agree your opinion.

But, I propose some fixes about palmas_set_switch_smps10().
I dont' prefer to call global function in exton-palmas.c from
palmas-regulator.c.
So, Why don't you use regulator consumer instead of global function?
You can register specific regulator for enabling or disabling
SMPS10_SWITCH_EN
and then control SMPS10_SWITCH_EN bit through regulator framework in
extcon-palmas.c
without calling global function.

Along with this, I also like to make the VBUS regulator control to be
optional here. Currently it is mandatory.

But dint you just tell on my v4 of this patch that you don’t require this.
http://www.spinics.net/lists/linux-doc/msg10638.html


In V4, I said remove this VBUS control and my mean was to remove all 
regulator calls for VBUS enabled/disable.
I saw you just remove the platform data option to have this control and 
made VBUS mandatory.


Probably some gap here.

In tegra platform, we dont need VBUS regualtor control from extcon.


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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Kishon Vijay Abraham I

Hi,

On Monday 27 May 2013 12:06 PM, Laxman Dewangan wrote:

On Monday 27 May 2013 12:01 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:52 AM, Laxman Dewangan wrote:

On Monday 27 May 2013 11:38 AM, Chanwoo Choi wrote:

On 05/27/2013 02:54 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:

Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository
(extcon-for-palmas).
-
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c



This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.

yeah.. Still there is some confusion with palmas_set_switch_smps10().
I think we can remove it for now and add it separately later. By this
at least we can have device mode fully functional in OMAP5. What do
you think?


   I agree your opinion.

But, I propose some fixes about palmas_set_switch_smps10().
I dont' prefer to call global function in exton-palmas.c from
palmas-regulator.c.
So, Why don't you use regulator consumer instead of global function?
You can register specific regulator for enabling or disabling
SMPS10_SWITCH_EN
and then control SMPS10_SWITCH_EN bit through regulator framework in
extcon-palmas.c
without calling global function.

Along with this, I also like to make the VBUS regulator control to be
optional here. Currently it is mandatory.

But dint you just tell on my v4 of this patch that you don’t require
this.
http://www.spinics.net/lists/linux-doc/msg10638.html


In V4, I said remove this VBUS control and my mean was to remove all
regulator calls for VBUS enabled/disable.
I saw you just remove the platform data option to have this control and
made VBUS mandatory.

Probably some gap here.


Indeed..
I think then we should stick back to how it was with my v4 or else it 
would break OMAP. The regulator calls can't be moved anywhere else as it 
is specific to PALMAS.


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


Re: [PATCH v5 2/3] extcon: Palmas Extcon Driver

2013-05-26 Thread Laxman Dewangan

On Monday 27 May 2013 12:11 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 12:06 PM, Laxman Dewangan wrote:

On Monday 27 May 2013 12:01 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:52 AM, Laxman Dewangan wrote:

On Monday 27 May 2013 11:38 AM, Chanwoo Choi wrote:

On 05/27/2013 02:54 PM, Kishon Vijay Abraham I wrote:

Hi,

On Monday 27 May 2013 11:04 AM, Chanwoo Choi wrote:

Hi Kishon,

I have some comment about this patch
and upload modified patch to following repository
(extcon-for-palmas).
-
http://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-for-palmas&id=f2b7cb80699cbe1a5fd6c97ef2c600915f8d7f2c



This patchset include patch related to other module
,so I need your opinion to apply this patchset to git repository.

yeah.. Still there is some confusion with palmas_set_switch_smps10().
I think we can remove it for now and add it separately later. By this
at least we can have device mode fully functional in OMAP5. What do
you think?


I agree your opinion.

But, I propose some fixes about palmas_set_switch_smps10().
I dont' prefer to call global function in exton-palmas.c from
palmas-regulator.c.
So, Why don't you use regulator consumer instead of global function?
You can register specific regulator for enabling or disabling
SMPS10_SWITCH_EN
and then control SMPS10_SWITCH_EN bit through regulator framework in
extcon-palmas.c
without calling global function.

Along with this, I also like to make the VBUS regulator control to be
optional here. Currently it is mandatory.

But dint you just tell on my v4 of this patch that you don’t require
this.
http://www.spinics.net/lists/linux-doc/msg10638.html

In V4, I said remove this VBUS control and my mean was to remove all
regulator calls for VBUS enabled/disable.
I saw you just remove the platform data option to have this control and
made VBUS mandatory.

Probably some gap here.

Indeed..
I think then we should stick back to how it was with my v4 or else it
would break OMAP. The regulator calls can't be moved anywhere else as it
is specific to PALMAS.



I was thinking that extcon driver just detect the cable type and notify 
to the client. After cable detection, the next level of configuration 
should be done in the respective client.


On Tegra platform,  for ID pin detection, Tegra SOC is capable of detect 
the ID pin presence or Palma is capable. Depending on the board design, 
how the ID pin routed from USB connector to PMIC or to Tegra, we enable 
corresponding detection logic.
Once the USB driver got notification for ID pin presence (by any means), 
the enabling of VBUS (as the Tegra will work as Host now and need to 
supply VBUS), is done in USB driver.

Not sure about the OMAP here.


So in above context, I really do not want to have the VBUS control on 
extcon driver from Tegra context. If it is require in OMAP context then 
please make it as optional so that we can satisfy for Tegra and Omap 
platform.


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