Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1733150 > unrolled thread

[PATCH 0/5] usb: usb251xb: Add USB2517i hub support and fix some bugs

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] usb: usb251xb: Add USB2517i hub support and fix some bugs Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:40 +0200
    [PATCH 3/5] usb: usb251xb: Add max power/current dts nodes Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:40 +0200
    [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:40 +0200
      Re: [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Greg KH <gregkh@linuxfoundation.org> - 2017-09-16 01:50 +0200
        Re: [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 02:00 +0200
      Re: [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Rob Herring <robh@kernel.org> - 2017-09-20 23:10 +0200
        Re: [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Serge Semin <fancer.lancer@gmail.com> - 2017-09-20 23:20 +0200
          Re: [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Rob Herring <robh@kernel.org> - 2017-09-21 19:00 +0200
            Re: [PATCH 1/5] usb: usb251xb: Add USB2517/i hub support Serge Semin <fancer.lancer@gmail.com> - 2017-09-21 19:50 +0200
    [PATCH 4/5] usb: usb251xb: Use GPIO descriptor consumer interface Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:40 +0200
    [PATCH 5/5] usb: usb251xb: Add copyrights Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:40 +0200
      Re: [PATCH 5/5] usb: usb251xb: Add copyrights Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:50 +0200
        Re: [PATCH 5/5] usb: usb251xb: Add copyrights Greg KH <gregkh@linuxfoundation.org> - 2017-09-16 02:00 +0200
          Re: [PATCH 5/5] usb: usb251xb: Add copyrights Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 02:20 +0200
      Re: [PATCH 5/5] usb: usb251xb: Add copyrights Greg KH <gregkh@linuxfoundation.org> - 2017-09-16 01:50 +0200
      Re: [PATCH 5/5] usb: usb251xb: Add copyrights Greg KH <gregkh@linuxfoundation.org> - 2017-09-16 01:50 +0200
    [PATCH 2/5] usb: usb251xb: Fix property_u32 NULL pointer dereference Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 01:40 +0200
    [PATCH 7/9 v2] usb: usb251xb: Fix property_u32 NULL pointer dereference Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
    [PATCH 4/9 v2] usb: usb251xb: Add 5,6,7 ports boost settings Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
    [PATCH 3/9 v2] usb: usb251xb: Add 5,6,7 ports mapping def setting Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
    [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
      Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Rob Herring <robh@kernel.org> - 2017-09-20 23:00 +0200
        Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Serge Semin <fancer.lancer@gmail.com> - 2017-09-20 23:30 +0200
      Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Greg KH <gregkh@linuxfoundation.org> - 2017-09-21 10:30 +0200
        Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Serge Semin <fancer.lancer@gmail.com> - 2017-09-21 17:00 +0200
          Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Greg KH <gregkh@linuxfoundation.org> - 2017-09-21 17:10 +0200
            Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Serge Semin <fancer.lancer@gmail.com> - 2017-09-22 17:30 +0200
              Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer  interface Greg KH <gregkh@linuxfoundation.org> - 2017-09-22 18:10 +0200
    [PATCH 5/9 v2] usb: usb251xb: Add battery enable setting flag Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
    [PATCH 0/9 v2] usb: usb251xb: Add USB2517i hub support and fix some bugs Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
      [PATCH 2/9 v2] usb: usb251xb: Add USB251x specific port count setting Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
      [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
        Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property  support Rob Herring <robh@kernel.org> - 2017-09-20 23:00 +0200
          Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property  support Serge Semin <fancer.lancer@gmail.com> - 2017-09-20 23:30 +0200
            Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support Rob Herring <robh@kernel.org> - 2017-09-21 18:30 +0200
              Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property  support Serge Semin <fancer.lancer@gmail.com> - 2017-09-21 19:20 +0200
      [PATCH 6/9 v2] usb: usb251xb: Add USB2517 LED settings Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200
      [PATCH 1/9 v2] usb: usb251xb: Add USB2517i specific struct and IDs Serge Semin <fancer.lancer@gmail.com> - 2017-09-16 12:50 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1733245 — [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

The driver used to be developed with legacy GPIO API support. It's
better to use descriptor-based interface for several reasons. First
of all the legacy API doesn't support the ACTIVE_LOW/HIGH flag of dts
nodes, which is essential since different hardware may have different
GPIOs connectivity including the logical value inversion. Secondly,
by requesting the reset GPIO descriptor the driver prevent the other
applications from changing its value. And last but not least the
legacy GPIO interface should be avoided in the new code due to it
obsolescence.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt |  2 +-
 drivers/usb/misc/usb251xb.c                        | 34 +++++++++-------------
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index dd59a32e7..7c981d556 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -8,10 +8,10 @@ Required properties :
 	"microchip,usb2512b", "microchip,usb2512bi", "microchip,usb2513b",
 	"microchip,usb2513bi", "microchip,usb2514b", "microchip,usb2514bi",
 	"microchip,usb2517", "microchip,usb2517i"
- - reset-gpios : Should specify the gpio for hub reset
  - reg : I2C address on the selected bus (default is <0x2C>)
 
 Optional properties :
+ - reset-gpios : Should specify the gpio for hub reset
  - skip-config : Skip Hub configuration, but only send the USB-Attach command
  - vendor-id : Set USB Vendor ID of the hub (16 bit, default is 0x0424)
  - product-id : Set USB Product ID of the hub (16 bit, default depends on type)
diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index 71994b883..c2dd9742f 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -3,6 +3,7 @@
  * Configuration via SMBus.
  *
  * Copyright (c) 2017 SKIDATA AG
+ * Copyright (c) 2017 T-platforms
  *
  * This work is based on the USB3503 driver by Dongjin Kim and
  * a not-accepted patch by Fabien Lahoudere, see:
@@ -20,12 +21,11 @@
  */
 
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/nls.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 
 /* Internal Register Set Addresses & Default Values acc. to DS00001692C */
@@ -127,7 +127,7 @@ struct usb251xb {
 	struct device *dev;
 	struct i2c_client *i2c;
 	u8 skip_config;
-	int gpio_reset;
+	struct gpio_desc *gpio_reset;
 	u16 vendor_id;
 	u16 product_id;
 	u16 device_id;
@@ -235,13 +235,13 @@ static const struct usb251xb_data usb2517i_data = {
 
 static void usb251xb_reset(struct usb251xb *hub, int state)
 {
-	if (!gpio_is_valid(hub->gpio_reset))
+	if (!hub->gpio_reset)
 		return;
 
-	gpio_set_value_cansleep(hub->gpio_reset, state);
+	gpiod_set_value_cansleep(hub->gpio_reset, state);
 
 	/* wait for hub recovery/stabilization */
-	if (state)
+	if (!state)
 		usleep_range(500, 750);	/* >=500us at power on */
 	else
 		usleep_range(1, 10);	/* >=1us at power down */
@@ -260,7 +260,7 @@ static int usb251xb_connect(struct usb251xb *hub)
 		i2c_wb[0] = 0x01;
 		i2c_wb[1] = USB251XB_STATUS_COMMAND_ATTACH;
 
-		usb251xb_reset(hub, 1);
+		usb251xb_reset(hub, 0);
 
 		err = i2c_smbus_write_i2c_block_data(hub->i2c,
 				USB251XB_ADDR_STATUS_COMMAND, 2, i2c_wb);
@@ -310,7 +310,7 @@ static int usb251xb_connect(struct usb251xb *hub)
 	i2c_wb[USB251XB_ADDR_PORT_MAP_7]        = hub->port_map7;
 	i2c_wb[USB251XB_ADDR_STATUS_COMMAND] = USB251XB_STATUS_COMMAND_ATTACH;
 
-	usb251xb_reset(hub, 1);
+	usb251xb_reset(hub, 0);
 
 	/* write registers */
 	for (i = 0; i < (USB251XB_I2C_REG_SZ / USB251XB_I2C_WRITE_SZ); i++) {
@@ -363,19 +363,13 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 	else
 		hub->skip_config = 0;
 
-	hub->gpio_reset = of_get_named_gpio(np, "reset-gpios", 0);
-	if (hub->gpio_reset == -EPROBE_DEFER)
+	hub->gpio_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (PTR_ERR(hub->gpio_reset) == -EPROBE_DEFER) {
 		return -EPROBE_DEFER;
-	if (gpio_is_valid(hub->gpio_reset)) {
-		err = devm_gpio_request_one(dev, hub->gpio_reset,
-					    GPIOF_OUT_INIT_LOW,
-					    "usb251xb reset");
-		if (err) {
-			dev_err(dev,
-				"unable to request GPIO %d as reset pin (%d)\n",
-				hub->gpio_reset, err);
-			return err;
-		}
+	} else if (IS_ERR(hub->gpio_reset)) {
+		err = PTR_ERR(hub->gpio_reset);
+		dev_err(dev, "unable to request GPIO reset pin (%d)\n", err);
+		return err;
 	}
 
 	if (of_property_read_u16_array(np, "vendor-id", &hub->vendor_id, 1))
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1736126 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> The driver used to be developed with legacy GPIO API support. It's
> better to use descriptor-based interface for several reasons. First
> of all the legacy API doesn't support the ACTIVE_LOW/HIGH flag of dts
> nodes, which is essential since different hardware may have different
> GPIOs connectivity including the logical value inversion. Secondly,
> by requesting the reset GPIO descriptor the driver prevent the other
> applications from changing its value. And last but not least the
> legacy GPIO interface should be avoided in the new code due to it
> obsolescence.

All this has nothing to do with the binding. Please make the binding 
change separate and state why it should be optional.

> 
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  Documentation/devicetree/bindings/usb/usb251xb.txt |  2 +-
>  drivers/usb/misc/usb251xb.c                        | 34 +++++++++-------------
>  2 files changed, 15 insertions(+), 21 deletions(-)

[toc] | [prev] | [next] | [standalone]

#1736159 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Wed, Sep 20, 2017 at 03:52:57PM -0500, Rob Herring <robh@kernel.org> wrote:
> On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> > The driver used to be developed with legacy GPIO API support. It's
> > better to use descriptor-based interface for several reasons. First
> > of all the legacy API doesn't support the ACTIVE_LOW/HIGH flag of dts
> > nodes, which is essential since different hardware may have different
> > GPIOs connectivity including the logical value inversion. Secondly,
> > by requesting the reset GPIO descriptor the driver prevent the other
> > applications from changing its value. And last but not least the
> > legacy GPIO interface should be avoided in the new code due to it
> > obsolescence.
> 
> All this has nothing to do with the binding. Please make the binding 
> change separate and state why it should be optional.
> 

Ok.

> > 
> > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/usb/usb251xb.txt |  2 +-
> >  drivers/usb/misc/usb251xb.c                        | 34 +++++++++-------------
> >  2 files changed, 15 insertions(+), 21 deletions(-)

[toc] | [prev] | [next] | [standalone]

#1736455 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index 71994b883..c2dd9742f 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
> @@ -3,6 +3,7 @@
>   * Configuration via SMBus.
>   *
>   * Copyright (c) 2017 SKIDATA AG
> + * Copyright (c) 2017 T-platforms

Again, no, please consult with your corporate lawyers why this isn't ok.

greg k-h

[toc] | [prev] | [next] | [standalone]

#1736745 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Thu, Sep 21, 2017 at 10:23:38AM +0200, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> > index 71994b883..c2dd9742f 100644
> > --- a/drivers/usb/misc/usb251xb.c
> > +++ b/drivers/usb/misc/usb251xb.c
> > @@ -3,6 +3,7 @@
> >   * Configuration via SMBus.
> >   *
> >   * Copyright (c) 2017 SKIDATA AG
> > + * Copyright (c) 2017 T-platforms
> 
> Again, no, please consult with your corporate lawyers why this isn't ok.
> 
> greg k-h

I still can't see why this isn't right. We submitted the patchset. It is not
that big and still it isn't just two lines. As I've seen all over the kernel, It is
a common practice to have multiple copyrights in kernel files. We are not claiming
the copyright to the whole file, but to the contribution only. I got a consent to
contribute when I was employed by the company. What's wrong with that? Shall I
send the patchset from my corporate e-mail then?

-Sergey

[toc] | [prev] | [next] | [standalone]

#1736752 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Thu, Sep 21, 2017 at 05:51:29PM +0300, Serge Semin wrote:
> On Thu, Sep 21, 2017 at 10:23:38AM +0200, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> > > index 71994b883..c2dd9742f 100644
> > > --- a/drivers/usb/misc/usb251xb.c
> > > +++ b/drivers/usb/misc/usb251xb.c
> > > @@ -3,6 +3,7 @@
> > >   * Configuration via SMBus.
> > >   *
> > >   * Copyright (c) 2017 SKIDATA AG
> > > + * Copyright (c) 2017 T-platforms
> > 
> > Again, no, please consult with your corporate lawyers why this isn't ok.
> > 
> > greg k-h
> 
> I still can't see why this isn't right. We submitted the patchset. It is not
> that big and still it isn't just two lines. As I've seen all over the kernel, It is
> a common practice to have multiple copyrights in kernel files. We are not claiming
> the copyright to the whole file, but to the contribution only. I got a consent to
> contribute when I was employed by the company. What's wrong with that? Shall I
> send the patchset from my corporate e-mail then?

Well, yes, I need some way to properly identify that this corporation
did do the changes.  I said that before, I don't know why you ignored
that.

And yes, multiple copyrights are just fine, but again, please talk to
your corporate lawyer about why these changes don't seem to warrant that
"mark".  If they do think that they do warrant that, great, I will be
glad to discuss that with them, off-list if needed.

For even more fun, try discussing with your lawyers about why copyright
marks like this don't even mean anything anymore, and haven't for 20+
years now (can't remember the actual date...)  But that's a different
topic, and one not really relevant here.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1737584 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Thu, Sep 21, 2017 at 05:07:14PM +0200, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Sep 21, 2017 at 05:51:29PM +0300, Serge Semin wrote:
> > On Thu, Sep 21, 2017 at 10:23:38AM +0200, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> > > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> > > > index 71994b883..c2dd9742f 100644
> > > > --- a/drivers/usb/misc/usb251xb.c
> > > > +++ b/drivers/usb/misc/usb251xb.c
> > > > @@ -3,6 +3,7 @@
> > > >   * Configuration via SMBus.
> > > >   *
> > > >   * Copyright (c) 2017 SKIDATA AG
> > > > + * Copyright (c) 2017 T-platforms
> > > 
> > > Again, no, please consult with your corporate lawyers why this isn't ok.
> > > 
> > > greg k-h
> > 
> > I still can't see why this isn't right. We submitted the patchset. It is not
> > that big and still it isn't just two lines. As I've seen all over the kernel, It is
> > a common practice to have multiple copyrights in kernel files. We are not claiming
> > the copyright to the whole file, but to the contribution only. I got a consent to
> > contribute when I was employed by the company. What's wrong with that? Shall I
> > send the patchset from my corporate e-mail then?
> 
> Well, yes, I need some way to properly identify that this corporation
> did do the changes.  I said that before, I don't know why you ignored
> that.
> 
> And yes, multiple copyrights are just fine, but again, please talk to
> your corporate lawyer about why these changes don't seem to warrant that
> "mark".  If they do think that they do warrant that, great, I will be
> glad to discuss that with them, off-list if needed.
> 
> For even more fun, try discussing with your lawyers about why copyright
> marks like this don't even mean anything anymore, and haven't for 20+
> years now (can't remember the actual date...)  But that's a different
> topic, and one not really relevant here.
> 
> thanks,
> 
> greg k-h

Alright then. We'll remove the Copyright mark and I'll resend the patchset
from my corporate e-mail. Hope it will solve this issue.
Could you review the rest of the patchset?

Regards,
-Sergey

[toc] | [prev] | [next] | [standalone]

#1737600 — Re: [PATCH 9/9 v2] usb: usb251xb: Use GPIO descriptor consumer interface

On Fri, Sep 22, 2017 at 06:26:54PM +0300, Serge Semin wrote:
> On Thu, Sep 21, 2017 at 05:07:14PM +0200, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Sep 21, 2017 at 05:51:29PM +0300, Serge Semin wrote:
> > > On Thu, Sep 21, 2017 at 10:23:38AM +0200, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > On Sat, Sep 16, 2017 at 01:42:20PM +0300, Serge Semin wrote:
> > > > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> > > > > index 71994b883..c2dd9742f 100644
> > > > > --- a/drivers/usb/misc/usb251xb.c
> > > > > +++ b/drivers/usb/misc/usb251xb.c
> > > > > @@ -3,6 +3,7 @@
> > > > >   * Configuration via SMBus.
> > > > >   *
> > > > >   * Copyright (c) 2017 SKIDATA AG
> > > > > + * Copyright (c) 2017 T-platforms
> > > > 
> > > > Again, no, please consult with your corporate lawyers why this isn't ok.
> > > > 
> > > > greg k-h
> > > 
> > > I still can't see why this isn't right. We submitted the patchset. It is not
> > > that big and still it isn't just two lines. As I've seen all over the kernel, It is
> > > a common practice to have multiple copyrights in kernel files. We are not claiming
> > > the copyright to the whole file, but to the contribution only. I got a consent to
> > > contribute when I was employed by the company. What's wrong with that? Shall I
> > > send the patchset from my corporate e-mail then?
> > 
> > Well, yes, I need some way to properly identify that this corporation
> > did do the changes.  I said that before, I don't know why you ignored
> > that.
> > 
> > And yes, multiple copyrights are just fine, but again, please talk to
> > your corporate lawyer about why these changes don't seem to warrant that
> > "mark".  If they do think that they do warrant that, great, I will be
> > glad to discuss that with them, off-list if needed.
> > 
> > For even more fun, try discussing with your lawyers about why copyright
> > marks like this don't even mean anything anymore, and haven't for 20+
> > years now (can't remember the actual date...)  But that's a different
> > topic, and one not really relevant here.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Alright then. We'll remove the Copyright mark and I'll resend the patchset
> from my corporate e-mail. Hope it will solve this issue.
> Could you review the rest of the patchset?

I'll wait for the resend, as Rob already pointed out issues, so it is
long-gone from my patchqueue.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1733246 — [PATCH 5/9 v2] usb: usb251xb: Add battery enable setting flag

Battery charging settings are supported by USB251xb hubs only.
USB2517i isn't one of them. So we need to reflect it within the
device-specific data structure. The driver doesn't support dts
property to change this setting, but instead defaults it with zero.
So the flag isn't used anywhere in the driver, but still can be helpful
in future, when necessity of corresponding dts setting arises.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/usb/misc/usb251xb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index 44fa7d084..0834729d1 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -164,54 +164,63 @@ struct usb251xb {
 struct usb251xb_data {
 	u16 product_id;
 	u8 port_cnt;
+	bool bat_support;
 	char product_str[USB251XB_STRING_BUFSIZE / 2]; /* ASCII string */
 };
 
 static const struct usb251xb_data usb2512b_data = {
 	.product_id = 0x2512,
 	.port_cnt = 2,
+	.bat_support = true,
 	.product_str = "USB2512B",
 };
 
 static const struct usb251xb_data usb2512bi_data = {
 	.product_id = 0x2512,
 	.port_cnt = 2,
+	.bat_support = true,
 	.product_str = "USB2512Bi",
 };
 
 static const struct usb251xb_data usb2513b_data = {
 	.product_id = 0x2513,
 	.port_cnt = 3,
+	.bat_support = true,
 	.product_str = "USB2513B",
 };
 
 static const struct usb251xb_data usb2513bi_data = {
 	.product_id = 0x2513,
 	.port_cnt = 3,
+	.bat_support = true,
 	.product_str = "USB2513Bi",
 };
 
 static const struct usb251xb_data usb2514b_data = {
 	.product_id = 0x2514,
 	.port_cnt = 4,
+	.bat_support = true,
 	.product_str = "USB2514B",
 };
 
 static const struct usb251xb_data usb2514bi_data = {
 	.product_id = 0x2514,
 	.port_cnt = 4,
+	.bat_support = true,
 	.product_str = "USB2514Bi",
 };
 
 static const struct usb251xb_data usb2517_data = {
 	.product_id = 0x2517,
 	.port_cnt = 7,
+	.bat_support = false,
 	.product_str = "USB2517",
 };
 
 static const struct usb251xb_data usb2517i_data = {
 	.product_id = 0x2517,
 	.port_cnt = 7,
+	.bat_support = false,
 	.product_str = "USB2517i",
 };
 
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1733247 — [PATCH 0/9 v2] usb: usb251xb: Add USB2517i hub support and fix some bugs

Primarily it was intended to just add USB2517 hub support to the driver.
But after tests a bug and inconistency were discovered. So it was decided
to make the following changes:

Changelog v1:
- Add USB2517/i hub specifics support to the driver
- Fix property_u32 NULL-pointer dereference
- Add new {bus,self}-max-{power,curret} dts properties
- Replace legacy GPIO API usage with descriptor-based one

Changelog v2:
- Split first patch into smaller ones
- Fix invalid BOOST_14 register definition
- Combine copyrights adding patch into the last one

Serge Semin (9):
  usb: usb251xb: Add USB2517i specific struct and IDs
  usb: usb251xb: Add USB251x specific port count setting
  usb: usb251xb: Add 5,6,7 ports mapping def setting
  usb: usb251xb: Add 5,6,7 ports boost settings
  usb: usb251xb: Add battery enable setting flag
  usb: usb251xb: Add USB2517 LED settings
  usb: usb251xb: Fix property_u32 NULL pointer dereference
  usb: usb251xb: Add max power/current dts property support
  usb: usb251xb: Use GPIO descriptor consumer interface

 Documentation/devicetree/bindings/usb/usb251xb.txt |  12 +-
 drivers/usb/misc/usb251xb.c                        | 160 +++++++++++++++------
 2 files changed, 128 insertions(+), 44 deletions(-)

-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1733248 — [PATCH 2/9 v2] usb: usb251xb: Add USB251x specific port count setting

USB251xb as well as USB2517 datasheet states, that all these
hubs differ by number of ports declared as the last digit in the
model name. So USB2512 got two ports, USB2513 - three, and so on.
Such setting must be reflected in the device specific data
structure and corresponding dts property should be checked whether
it doesn't get out of available ports.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/usb/misc/usb251xb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index 96a8c20ac..5cb0e5570 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -154,46 +154,55 @@ struct usb251xb {
 
 struct usb251xb_data {
 	u16 product_id;
+	u8 port_cnt;
 	char product_str[USB251XB_STRING_BUFSIZE / 2]; /* ASCII string */
 };
 
 static const struct usb251xb_data usb2512b_data = {
 	.product_id = 0x2512,
+	.port_cnt = 2,
 	.product_str = "USB2512B",
 };
 
 static const struct usb251xb_data usb2512bi_data = {
 	.product_id = 0x2512,
+	.port_cnt = 2,
 	.product_str = "USB2512Bi",
 };
 
 static const struct usb251xb_data usb2513b_data = {
 	.product_id = 0x2513,
+	.port_cnt = 3,
 	.product_str = "USB2513B",
 };
 
 static const struct usb251xb_data usb2513bi_data = {
 	.product_id = 0x2513,
+	.port_cnt = 3,
 	.product_str = "USB2513Bi",
 };
 
 static const struct usb251xb_data usb2514b_data = {
 	.product_id = 0x2514,
+	.port_cnt = 4,
 	.product_str = "USB2514B",
 };
 
 static const struct usb251xb_data usb2514bi_data = {
 	.product_id = 0x2514,
+	.port_cnt = 4,
 	.product_str = "USB2514Bi",
 };
 
 static const struct usb251xb_data usb2517_data = {
 	.product_id = 0x2517,
+	.port_cnt = 7,
 	.product_str = "USB2517",
 };
 
 static const struct usb251xb_data usb2517i_data = {
 	.product_id = 0x2517,
+	.port_cnt = 7,
 	.product_str = "USB2517i",
 };
 
@@ -422,8 +431,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 		for (i = 0; i < len / sizeof(u32); i++) {
 			u32 port = be32_to_cpu(cproperty_u32[i]);
 
-			if ((port >= 1) && (port <= 4))
+			if ((port >= 1) && (port <= data->port_cnt))
 				hub->non_rem_dev |= BIT(port);
+			else
+				dev_warn(dev, "port %u doesn't exist\n", port);
 		}
 	}
 
@@ -433,8 +444,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 		for (i = 0; i < len / sizeof(u32); i++) {
 			u32 port = be32_to_cpu(cproperty_u32[i]);
 
-			if ((port >= 1) && (port <= 4))
+			if ((port >= 1) && (port <= data->port_cnt))
 				hub->port_disable_sp |= BIT(port);
+			else
+				dev_warn(dev, "port %u doesn't exist\n", port);
 		}
 	}
 
@@ -444,8 +457,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 		for (i = 0; i < len / sizeof(u32); i++) {
 			u32 port = be32_to_cpu(cproperty_u32[i]);
 
-			if ((port >= 1) && (port <= 4))
+			if ((port >= 1) && (port <= data->port_cnt))
 				hub->port_disable_bp |= BIT(port);
+			else
+				dev_warn(dev, "port %u doesn't exist\n", port);
 		}
 	}
 
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1733249 — [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

This parameters may be varied in accordance with hardware specifics.
So lets add the corresponding settings to the usb251x driver dts
specification.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt   |  6 ++++++
 drivers/usb/misc/usb251xb.c                          | 20 ++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index 3d84626d3..dd59a32e7 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -44,6 +44,12 @@ Optional properties :
 	device connected.
  - sp-disabled-ports : Specifies the ports which will be self-power disabled
  - bp-disabled-ports : Specifies the ports which will be bus-power disabled
+ - sp-max-{power,current} : Indicates the power/current consumed by hub from
+	an upstream port (VBUS) when operation as a self-powered hub. The value
+	is given in mA in a 0 - 100 range (default is 1mA).
+ - bp-max-{power,current} : Indicates the power/current consumed by hub from
+	an upstream port (VBUS) when operation as a bus-powered hub. The value
+	is given in mA in a 0 - 510 range (default is 100mA).
  - power-on-time-ms : Specifies the time it takes from the time the host
 	initiates the power-on sequence to a port until the port has adequate
 	power. The value is given in ms in a 0 - 510 range (default is 100ms).
diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index c308b0006..71994b883 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -497,6 +497,22 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 		}
 	}
 
+	hub->max_power_sp = USB251XB_DEF_MAX_POWER_SELF;
+	if (!of_property_read_u32(np, "sp-max-power", &property_u32))
+		hub->max_power_sp = min_t(u8, property_u32 / 2, 50);
+
+	hub->max_power_bp = USB251XB_DEF_MAX_POWER_BUS;
+	if (!of_property_read_u32(np, "bp-max-power", &property_u32))
+		hub->max_power_bp = min_t(u8, property_u32 / 2, 255);
+
+	hub->max_current_sp = USB251XB_DEF_MAX_CURRENT_SELF;
+	if (!of_property_read_u32(np, "sp-max-current", &property_u32))
+		hub->max_current_sp = min_t(u8, property_u32 / 2, 50);
+
+	hub->max_current_bp = USB251XB_DEF_MAX_CURRENT_BUS;
+	if (!of_property_read_u32(np, "bp-max-current", &property_u32))
+		hub->max_current_bp = min_t(u8, property_u32 / 2, 255);
+
 	hub->power_on_time = USB251XB_DEF_POWER_ON_TIME;
 	if (!of_property_read_u32(np, "power-on-time-ms", &property_u32))
 		hub->power_on_time = min_t(u8, property_u32 / 2, 255);
@@ -536,10 +552,6 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 	/* The following parameters are currently not exposed to devicetree, but
 	 * may be as soon as needed.
 	 */
-	hub->max_power_sp = USB251XB_DEF_MAX_POWER_SELF;
-	hub->max_power_bp = USB251XB_DEF_MAX_POWER_BUS;
-	hub->max_current_sp = USB251XB_DEF_MAX_CURRENT_SELF;
-	hub->max_current_bp = USB251XB_DEF_MAX_CURRENT_BUS;
 	hub->bat_charge_en = USB251XB_DEF_BATTERY_CHARGING_ENABLE;
 	hub->boost_up = USB251XB_DEF_BOOST_UP;
 	hub->boost_57 = USB251XB_DEF_BOOST_57;
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1736115 — Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

On Sat, Sep 16, 2017 at 01:42:19PM +0300, Serge Semin wrote:
> This parameters may be varied in accordance with hardware specifics.
> So lets add the corresponding settings to the usb251x driver dts
> specification.
> 
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  Documentation/devicetree/bindings/usb/usb251xb.txt   |  6 ++++++
>  drivers/usb/misc/usb251xb.c                          | 20 ++++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
> index 3d84626d3..dd59a32e7 100644
> --- a/Documentation/devicetree/bindings/usb/usb251xb.txt
> +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
> @@ -44,6 +44,12 @@ Optional properties :
>  	device connected.
>   - sp-disabled-ports : Specifies the ports which will be self-power disabled
>   - bp-disabled-ports : Specifies the ports which will be bus-power disabled
> + - sp-max-{power,current} : Indicates the power/current consumed by hub from
> +	an upstream port (VBUS) when operation as a self-powered hub. The value
> +	is given in mA in a 0 - 100 range (default is 1mA).
> + - bp-max-{power,current} : Indicates the power/current consumed by hub from
> +	an upstream port (VBUS) when operation as a bus-powered hub. The value
> +	is given in mA in a 0 - 510 range (default is 100mA).

These need units as defined in property-units.txt.

Why do you need power and current? Can't you calculate power?

>   - power-on-time-ms : Specifies the time it takes from the time the host
>  	initiates the power-on sequence to a port until the port has adequate
>  	power. The value is given in ms in a 0 - 510 range (default is 100ms).

[toc] | [prev] | [next] | [standalone]

#1736160 — Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

On Wed, Sep 20, 2017 at 03:52:55PM -0500, Rob Herring <robh@kernel.org> wrote:
> On Sat, Sep 16, 2017 at 01:42:19PM +0300, Serge Semin wrote:
> > This parameters may be varied in accordance with hardware specifics.
> > So lets add the corresponding settings to the usb251x driver dts
> > specification.
> > 
> > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/usb/usb251xb.txt   |  6 ++++++
> >  drivers/usb/misc/usb251xb.c                          | 20 ++++++++++++++++----
> >  2 files changed, 22 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
> > index 3d84626d3..dd59a32e7 100644
> > --- a/Documentation/devicetree/bindings/usb/usb251xb.txt
> > +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
> > @@ -44,6 +44,12 @@ Optional properties :
> >  	device connected.
> >   - sp-disabled-ports : Specifies the ports which will be self-power disabled
> >   - bp-disabled-ports : Specifies the ports which will be bus-power disabled
> > + - sp-max-{power,current} : Indicates the power/current consumed by hub from
> > +	an upstream port (VBUS) when operation as a self-powered hub. The value
> > +	is given in mA in a 0 - 100 range (default is 1mA).
> > + - bp-max-{power,current} : Indicates the power/current consumed by hub from
> > +	an upstream port (VBUS) when operation as a bus-powered hub. The value
> > +	is given in mA in a 0 - 510 range (default is 100mA).
> 
> These need units as defined in property-units.txt.
> 

Ok.

> Why do you need power and current? Can't you calculate power?
> 

These are different parameters of the device. They got different configuration
registers and descriptions:
max_power* - ... This value also includes the power consumption of a
permanently attached peripheral if the hub is configured as a compound
device, and the embedded peripheral reports 0mA in its descriptors.
max_current* - ... This value does NOT include the power consumption of a
permanently attached peripheral if the hub is configured as a compound
device.

Additionally as you can see, they both are measured in "mA", so it isn't
a real physical power.

> >   - power-on-time-ms : Specifies the time it takes from the time the host
> >  	initiates the power-on sequence to a port until the port has adequate
> >  	power. The value is given in ms in a 0 - 510 range (default is 100ms).

[toc] | [prev] | [next] | [standalone]

#1736834 — Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin <fancer.lancer@gmail.com> wrote:
> On Wed, Sep 20, 2017 at 03:52:55PM -0500, Rob Herring <robh@kernel.org> wrote:
>> On Sat, Sep 16, 2017 at 01:42:19PM +0300, Serge Semin wrote:
>> > This parameters may be varied in accordance with hardware specifics.
>> > So lets add the corresponding settings to the usb251x driver dts
>> > specification.
>> >
>> > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
>> > ---
>> >  Documentation/devicetree/bindings/usb/usb251xb.txt   |  6 ++++++
>> >  drivers/usb/misc/usb251xb.c                          | 20 ++++++++++++++++----
>> >  2 files changed, 22 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
>> > index 3d84626d3..dd59a32e7 100644
>> > --- a/Documentation/devicetree/bindings/usb/usb251xb.txt
>> > +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
>> > @@ -44,6 +44,12 @@ Optional properties :
>> >     device connected.
>> >   - sp-disabled-ports : Specifies the ports which will be self-power disabled
>> >   - bp-disabled-ports : Specifies the ports which will be bus-power disabled
>> > + - sp-max-{power,current} : Indicates the power/current consumed by hub from
>> > +   an upstream port (VBUS) when operation as a self-powered hub. The value
>> > +   is given in mA in a 0 - 100 range (default is 1mA).
>> > + - bp-max-{power,current} : Indicates the power/current consumed by hub from
>> > +   an upstream port (VBUS) when operation as a bus-powered hub. The value
>> > +   is given in mA in a 0 - 510 range (default is 100mA).
>>
>> These need units as defined in property-units.txt.
>>
>
> Ok.
>
>> Why do you need power and current? Can't you calculate power?
>>
>
> These are different parameters of the device. They got different configuration
> registers and descriptions:
> max_power* - ... This value also includes the power consumption of a
> permanently attached peripheral if the hub is configured as a compound
> device, and the embedded peripheral reports 0mA in its descriptors.
> max_current* - ... This value does NOT include the power consumption of a
> permanently attached peripheral if the hub is configured as a compound
> device.

Then the names here should somehow reflect the above. Perhaps
"composite-current" and "hub-current" or something like that.

>
> Additionally as you can see, they both are measured in "mA", so it isn't
> a real physical power.

Well, I can't because there's no units.

Rob

[toc] | [prev] | [next] | [standalone]

#1736929 — Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

On Thu, Sep 21, 2017 at 11:26:04AM -0500, Rob Herring <robh@kernel.org> wrote:
> On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin <fancer.lancer@gmail.com> wrote:
> > On Wed, Sep 20, 2017 at 03:52:55PM -0500, Rob Herring <robh@kernel.org> wrote:
> >> On Sat, Sep 16, 2017 at 01:42:19PM +0300, Serge Semin wrote:
> >> > This parameters may be varied in accordance with hardware specifics.
> >> > So lets add the corresponding settings to the usb251x driver dts
> >> > specification.
> >> >
> >> > Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> >> > ---
> >> >  Documentation/devicetree/bindings/usb/usb251xb.txt   |  6 ++++++
> >> >  drivers/usb/misc/usb251xb.c                          | 20 ++++++++++++++++----
> >> >  2 files changed, 22 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
> >> > index 3d84626d3..dd59a32e7 100644
> >> > --- a/Documentation/devicetree/bindings/usb/usb251xb.txt
> >> > +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
> >> > @@ -44,6 +44,12 @@ Optional properties :
> >> >     device connected.
> >> >   - sp-disabled-ports : Specifies the ports which will be self-power disabled
> >> >   - bp-disabled-ports : Specifies the ports which will be bus-power disabled
> >> > + - sp-max-{power,current} : Indicates the power/current consumed by hub from
> >> > +   an upstream port (VBUS) when operation as a self-powered hub. The value
> >> > +   is given in mA in a 0 - 100 range (default is 1mA).
> >> > + - bp-max-{power,current} : Indicates the power/current consumed by hub from
> >> > +   an upstream port (VBUS) when operation as a bus-powered hub. The value
> >> > +   is given in mA in a 0 - 510 range (default is 100mA).
> >>
> >> These need units as defined in property-units.txt.
> >>
> >
> > Ok.
> >
> >> Why do you need power and current? Can't you calculate power?
> >>
> >
> > These are different parameters of the device. They got different configuration
> > registers and descriptions:
> > max_power* - ... This value also includes the power consumption of a
> > permanently attached peripheral if the hub is configured as a compound
> > device, and the embedded peripheral reports 0mA in its descriptors.
> > max_current* - ... This value does NOT include the power consumption of a
> > permanently attached peripheral if the hub is configured as a compound
> > device.
> 
> Then the names here should somehow reflect the above. Perhaps
> "composite-current" and "hub-current" or something like that.
> 

I left the naming in accordance with the device datasheet. I thought it would be
better since the driver user would still need to consult with the device
documentation to properly set them. I don't really get how the difference is reflected
with the naming declared there though. So what naming would you prefer then? Might be
something like:
{sp,bp}-max-total-current - for so named {sp,bp}-max-power, since it includes all the
permanently attached peripherals.
{sp,bp}-max-removable-current - for so named {sp,bp}-max-current, since it doesn't
include the permanently attached peripherals.

Or is it better to leave it in compliance with the documentation naming?

> >
> > Additionally as you can see, they both are measured in "mA", so it isn't
> > a real physical power.
> 
> Well, I can't because there's no units.
> 

What this line means then?
- sp-max-{power,current} : ... The value is given in mA in a 0 - 100 range (default is 1mA).
- bp-max-{power,current} : ... The value is given in mA in a 0 - 510 range (default is 100mA).

Maybe I don't know something and the description line should state the units somehow
clearer?

> Rob

[toc] | [prev] | [next] | [standalone]

#1733250 — [PATCH 6/9 v2] usb: usb251xb: Add USB2517 LED settings

USB2517 supports two LED modes: USB mode (default) and speed indication
mode. The last one can be switched on by corresponding dts property.
Since USB251xb hubs doesn't support LEDs settings, we need to ignore
this setting.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt         |  1 +
 drivers/usb/misc/usb251xb.c                                | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index 1682d4087..3d84626d3 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -37,6 +37,7 @@ Optional properties :
 	an invalid value is given, the default is used instead.
  - compound-device : indicate the hub is part of a compound device
  - port-mapping-mode : enable port mapping mode
+ - speed-led-mode : led speed indiation mode selection (usb2517 only)
  - string-support : enable string descriptor support (required for manufacturer,
 	product and serial string configuration)
  - non-removable-ports : Should specify the ports which have a non-removable
diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index 0834729d1..51cc53ddc 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -49,7 +49,7 @@
 #define USB251XB_ADDR_CONFIG_DATA_2	0x07
 #define USB251XB_DEF_CONFIG_DATA_2	0x20
 #define USB251XB_ADDR_CONFIG_DATA_3	0x08
-#define USB251XB_DEF_CONFIG_DATA_3	0x02
+#define USB251XB_DEF_CONFIG_DATA_3	0x00
 
 #define USB251XB_ADDR_NON_REMOVABLE_DEVICES	0x09
 #define USB251XB_DEF_NON_REMOVABLE_DEVICES	0x00
@@ -164,6 +164,7 @@ struct usb251xb {
 struct usb251xb_data {
 	u16 product_id;
 	u8 port_cnt;
+	bool led_support;
 	bool bat_support;
 	char product_str[USB251XB_STRING_BUFSIZE / 2]; /* ASCII string */
 };
@@ -171,6 +172,7 @@ struct usb251xb_data {
 static const struct usb251xb_data usb2512b_data = {
 	.product_id = 0x2512,
 	.port_cnt = 2,
+	.led_support = false,
 	.bat_support = true,
 	.product_str = "USB2512B",
 };
@@ -178,6 +180,7 @@ static const struct usb251xb_data usb2512b_data = {
 static const struct usb251xb_data usb2512bi_data = {
 	.product_id = 0x2512,
 	.port_cnt = 2,
+	.led_support = false,
 	.bat_support = true,
 	.product_str = "USB2512Bi",
 };
@@ -185,6 +188,7 @@ static const struct usb251xb_data usb2512bi_data = {
 static const struct usb251xb_data usb2513b_data = {
 	.product_id = 0x2513,
 	.port_cnt = 3,
+	.led_support = false,
 	.bat_support = true,
 	.product_str = "USB2513B",
 };
@@ -192,6 +196,7 @@ static const struct usb251xb_data usb2513b_data = {
 static const struct usb251xb_data usb2513bi_data = {
 	.product_id = 0x2513,
 	.port_cnt = 3,
+	.led_support = false,
 	.bat_support = true,
 	.product_str = "USB2513Bi",
 };
@@ -199,6 +204,7 @@ static const struct usb251xb_data usb2513bi_data = {
 static const struct usb251xb_data usb2514b_data = {
 	.product_id = 0x2514,
 	.port_cnt = 4,
+	.led_support = false,
 	.bat_support = true,
 	.product_str = "USB2514B",
 };
@@ -206,6 +212,7 @@ static const struct usb251xb_data usb2514b_data = {
 static const struct usb251xb_data usb2514bi_data = {
 	.product_id = 0x2514,
 	.port_cnt = 4,
+	.led_support = false,
 	.bat_support = true,
 	.product_str = "USB2514Bi",
 };
@@ -213,6 +220,7 @@ static const struct usb251xb_data usb2514bi_data = {
 static const struct usb251xb_data usb2517_data = {
 	.product_id = 0x2517,
 	.port_cnt = 7,
+	.led_support = true,
 	.bat_support = false,
 	.product_str = "USB2517",
 };
@@ -220,6 +228,7 @@ static const struct usb251xb_data usb2517_data = {
 static const struct usb251xb_data usb2517i_data = {
 	.product_id = 0x2517,
 	.port_cnt = 7,
+	.led_support = true,
 	.bat_support = false,
 	.product_str = "USB2517i",
 };
@@ -443,6 +452,9 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 	if (of_get_property(np, "port-mapping-mode", NULL))
 		hub->conf_data3 |= BIT(3);
 
+	if (data->led_support && of_get_property(np, "speed-led-mode", NULL))
+		hub->conf_data3 |= BIT(1);
+
 	if (of_get_property(np, "string-support", NULL))
 		hub->conf_data3 |= BIT(0);
 
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1733251 — [PATCH 1/9 v2] usb: usb251xb: Add USB2517i specific struct and IDs

There are USB2517 and USB2517i hubs, which have almost the same
registers space as already supported USB251xbi series. The difference
it in DIDs and in few functions. This patch adds the USB2517/i data
structures to the driver, so it would have different setting depending
on the device discovered on i2c-bus.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt  |  3 ++-
 drivers/usb/misc/usb251xb.c                         | 21 ++++++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index 3957d4eda..1682d4087 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -6,7 +6,8 @@ Hi-Speed Controller.
 Required properties :
  - compatible : Should be "microchip,usb251xb" or one of the specific types:
 	"microchip,usb2512b", "microchip,usb2512bi", "microchip,usb2513b",
-	"microchip,usb2513bi", "microchip,usb2514b", "microchip,usb2514bi"
+	"microchip,usb2513bi", "microchip,usb2514b", "microchip,usb2514bi",
+	"microchip,usb2517", "microchip,usb2517i"
  - reset-gpios : Should specify the gpio for hub reset
  - reg : I2C address on the selected bus (default is <0x2C>)
 
diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index 91f66d68b..96a8c20ac 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -38,6 +38,7 @@
 #define USB251XB_DEF_PRODUCT_ID_12	0x2512 /* USB2512B/12Bi */
 #define USB251XB_DEF_PRODUCT_ID_13	0x2513 /* USB2513B/13Bi */
 #define USB251XB_DEF_PRODUCT_ID_14	0x2514 /* USB2514B/14Bi */
+#define USB251XB_DEF_PRODUCT_ID_17	0x2517 /* USB2517i */
 
 #define USB251XB_ADDR_DEVICE_ID_LSB	0x04
 #define USB251XB_ADDR_DEVICE_ID_MSB	0x05
@@ -82,7 +83,7 @@
 
 #define USB251XB_ADDR_PRODUCT_STRING_LEN	0x14
 #define USB251XB_ADDR_PRODUCT_STRING		0x54
-#define USB251XB_DEF_PRODUCT_STRING		"USB251xB/xBi"
+#define USB251XB_DEF_PRODUCT_STRING		"USB251xB/xBi/7i"
 
 #define USB251XB_ADDR_SERIAL_STRING_LEN		0x15
 #define USB251XB_ADDR_SERIAL_STRING		0x92
@@ -186,6 +187,16 @@ static const struct usb251xb_data usb2514bi_data = {
 	.product_str = "USB2514Bi",
 };
 
+static const struct usb251xb_data usb2517_data = {
+	.product_id = 0x2517,
+	.product_str = "USB2517",
+};
+
+static const struct usb251xb_data usb2517i_data = {
+	.product_id = 0x2517,
+	.product_str = "USB2517i",
+};
+
 static void usb251xb_reset(struct usb251xb *hub, int state)
 {
 	if (!gpio_is_valid(hub->gpio_reset))
@@ -511,6 +522,12 @@ static const struct of_device_id usb251xb_of_match[] = {
 		.compatible = "microchip,usb2514bi",
 		.data = &usb2514bi_data,
 	}, {
+		.compatible = "microchip,usb2517",
+		.data = &usb2517_data,
+	}, {
+		.compatible = "microchip,usb2517i",
+		.data = &usb2517i_data,
+	}, {
 		/* sentinel */
 	}
 };
@@ -574,6 +591,8 @@ static const struct i2c_device_id usb251xb_id[] = {
 	{ "usb2513bi", 0 },
 	{ "usb2514b", 0 },
 	{ "usb2514bi", 0 },
+	{ "usb2517", 0 },
+	{ "usb2517i", 0 },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, usb251xb_id);
-- 
2.12.0

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web