[PATCH 0/3] usb: misc: improve USB251xB/xBi dt bindings

[PATCH 0/3] usb: misc: improve USB251xB/xBi dt bindings

[Richard Leitner]

This patchset improves/fixes the USB251xB/xBi devicetree bindings as
recommended by Rob Herring.

Richard Leitner (3):
  usb: usb251xb: remove max_{power,current}_{sp,bp} properties
  usb: usb251xb: dt: add unit suffix to oc-delay and power-on-time
  doc: dt-bindings: usb251xb: mark reg as required

 Documentation/devicetree/bindings/usb/usb251xb.txt | 52 +++++++-------------
 drivers/usb/misc/usb251xb.c                        | 56 +++++++++-------------
 2 files changed, 40 insertions(+), 68 deletions(-)

-- 
2.1.4

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

[PATCH 1/3] usb: usb251xb: remove max_{power,current}_{sp,bp} properties

[Richard Leitner]

Remove the max_{power,current}_{sp,bp} properties of the usb251xb driver
from devicetree. This is done to simplify the dt bindings as requested
by Rob Herring in https://lkml.org/lkml/2017/2/15/1283. If those
properties are ever needed by somebody they can be enabled again easily.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt | 20 ------------------
 drivers/usb/misc/usb251xb.c                        | 24 ++++------------------
 2 files changed, 4 insertions(+), 40 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index 0c065f7..a5efd10 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -40,26 +40,6 @@ 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
- - max-sp-power : Specifies the maximum current the hub consumes from an
-	upstream port when operating as self-powered hub including the power
-	consumption of a permanently attached peripheral if the hub is
-	configured as a compound device. The value is given in mA in a 0 - 500
-	range (default is 2).
- - max-bp-power : Specifies the maximum current the hub consumes from an
-	upstream port when operating as bus-powered hub including the power
-	consumption of a permanently attached peripheral if the hub is
-	configured as a compound device. The value is given in mA in a 0 - 500
-	range (default is 100).
- - max-sp-current : Specifies the maximum current the hub consumes from an
-	upstream port when operating as self-powered hub EXCLUDING the power
-	consumption of a permanently attached peripheral if the hub is
-	configured as a compound device. The value is given in mA in a 0 - 500
-	range (default is 2).
- - max-bp-current : Specifies the maximum current the hub consumes from an
-	upstream port when operating as bus-powered hub EXCLUDING the power
-	consumption of a permanently attached peripheral if the hub is
-	configured as a compound device. The value is given in mA in a 0 - 500
-	range (default is 100).
  - power-on-time : 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 4e18600..3f9c306 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -432,26 +432,6 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 		}
 	}
 
-	hub->max_power_sp = USB251XB_DEF_MAX_POWER_SELF;
-	if (!of_property_read_u32(np, "max-sp-power", property_u32))
-		hub->max_power_sp = min_t(u8, be32_to_cpu(*property_u32) / 2,
-					  250);
-
-	hub->max_power_bp = USB251XB_DEF_MAX_POWER_BUS;
-	if (!of_property_read_u32(np, "max-bp-power", property_u32))
-		hub->max_power_bp = min_t(u8, be32_to_cpu(*property_u32) / 2,
-					  250);
-
-	hub->max_current_sp = USB251XB_DEF_MAX_CURRENT_SELF;
-	if (!of_property_read_u32(np, "max-sp-current", property_u32))
-		hub->max_current_sp = min_t(u8, be32_to_cpu(*property_u32) / 2,
-					    250);
-
-	hub->max_current_bp = USB251XB_DEF_MAX_CURRENT_BUS;
-	if (!of_property_read_u32(np, "max-bp-current", property_u32))
-		hub->max_current_bp = min_t(u8, be32_to_cpu(*property_u32) / 2,
-					    250);
-
 	hub->power_on_time = USB251XB_DEF_POWER_ON_TIME;
 	if (!of_property_read_u32(np, "power-on-time", property_u32))
 		hub->power_on_time = min_t(u8, be32_to_cpu(*property_u32) / 2,
@@ -492,6 +472,10 @@ 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_x = USB251XB_DEF_BOOST_X;
-- 
2.1.4

Re: [PATCH 1/3] usb: usb251xb: remove max_{power,current}_{sp,bp} properties

[Rob Herring]

On Tue, Feb 21, 2017 at 09:56:17AM +0100, Richard Leitner wrote:
> Remove the max_{power,current}_{sp,bp} properties of the usb251xb driver
> from devicetree. This is done to simplify the dt bindings as requested
> by Rob Herring in https://lkml.org/lkml/2017/2/15/1283. If those
> properties are ever needed by somebody they can be enabled again easily.
> 
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
>  Documentation/devicetree/bindings/usb/usb251xb.txt | 20 ------------------
>  drivers/usb/misc/usb251xb.c                        | 24 ++++------------------
>  2 files changed, 4 insertions(+), 40 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/3] usb: usb251xb: dt: add unit suffix to oc-delay and power-on-time

[Richard Leitner]

Rename oc-delay-* to oc-delay-us and make it expect a time value.
Furthermore add -ms suffix to power-on-time. These changes were
suggested by Rob Herring in https://lkml.org/lkml/2017/2/15/1283.

Signed-off-by: Richard Leitner <richard.leitner-WcANXNA0UjBBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt |  9 +++---
 drivers/usb/misc/usb251xb.c                        | 32 +++++++++++++---------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index a5efd10..4d6bd73 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -31,7 +31,8 @@ Optional properties :
 	(default is individual)
  - dynamic-power-switching : enable auto-switching from self- to bus-powered
 	operation if the local power source is removed or unavailable
- - oc-delay-{100us,4ms,8ms,16ms} : set over current timer delay (default is 8ms)
+ - oc-delay-us : microseconds over current timer delay, valid values are 100,
+	4000, 8000 (default) and 16000. All other values are rounded up.
  - compound-device : indicated the hub is part of a compound device
  - port-mapping-mode : enable port mapping mode
  - string-support : enable string descriptor support (required for manufacturer,
@@ -40,9 +41,9 @@ 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
- - power-on-time : 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).
+ - 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).
 
 Examples:
 	usb2512b@2c {
diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
index 3f9c306..58b887a 100644
--- a/drivers/usb/misc/usb251xb.c
+++ b/drivers/usb/misc/usb251xb.c
@@ -375,18 +375,24 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 	if (of_get_property(np, "dynamic-power-switching", NULL))
 		hub->conf_data2 |= BIT(7);
 
-	if (of_get_property(np, "oc-delay-100us", NULL)) {
-		hub->conf_data2 &= ~BIT(5);
-		hub->conf_data2 &= ~BIT(4);
-	} else if (of_get_property(np, "oc-delay-4ms", NULL)) {
-		hub->conf_data2 &= ~BIT(5);
-		hub->conf_data2 |= BIT(4);
-	} else if (of_get_property(np, "oc-delay-8ms", NULL)) {
-		hub->conf_data2 |= BIT(5);
-		hub->conf_data2 &= ~BIT(4);
-	} else if (of_get_property(np, "oc-delay-16ms", NULL)) {
-		hub->conf_data2 |= BIT(5);
-		hub->conf_data2 |= BIT(4);
+	if (!of_property_read_u32(np, "oc-delay-us", property_u32)) {
+		if (*property_u32 < 100) {
+			/* 100 us*/
+			hub->conf_data2 &= ~BIT(5);
+			hub->conf_data2 &= ~BIT(4);
+		} else if (*property_u32 < 4000) {
+			/* 4 ms */
+			hub->conf_data2 &= ~BIT(5);
+			hub->conf_data2 |= BIT(4);
+		} else if (*property_u32 < 8000) {
+			/* 8 ms */
+			hub->conf_data2 |= BIT(5);
+			hub->conf_data2 &= ~BIT(4);
+		} else {
+			/* 16 ms */
+			hub->conf_data2 |= BIT(5);
+			hub->conf_data2 |= BIT(4);
+		}
 	}
 
 	if (of_get_property(np, "compound-device", NULL))
@@ -433,7 +439,7 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
 	}
 
 	hub->power_on_time = USB251XB_DEF_POWER_ON_TIME;
-	if (!of_property_read_u32(np, "power-on-time", property_u32))
+	if (!of_property_read_u32(np, "power-on-time-ms", property_u32))
 		hub->power_on_time = min_t(u8, be32_to_cpu(*property_u32) / 2,
 					   255);
 
-- 
2.1.4

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

Re: [PATCH 2/3] usb: usb251xb: dt: add unit suffix to oc-delay and power-on-time

[Rob Herring]

On Tue, Feb 21, 2017 at 09:56:18AM +0100, Richard Leitner wrote:
> Rename oc-delay-* to oc-delay-us and make it expect a time value.
> Furthermore add -ms suffix to power-on-time. These changes were
> suggested by Rob Herring in https://lkml.org/lkml/2017/2/15/1283.
> 
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
>  Documentation/devicetree/bindings/usb/usb251xb.txt |  9 +++---
>  drivers/usb/misc/usb251xb.c                        | 32 +++++++++++++---------
>  2 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
> index a5efd10..4d6bd73 100644
> --- a/Documentation/devicetree/bindings/usb/usb251xb.txt
> +++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
> @@ -31,7 +31,8 @@ Optional properties :
>  	(default is individual)
>   - dynamic-power-switching : enable auto-switching from self- to bus-powered
>  	operation if the local power source is removed or unavailable
> - - oc-delay-{100us,4ms,8ms,16ms} : set over current timer delay (default is 8ms)
> + - oc-delay-us : microseconds over current timer delay, valid values are 100,
> +	4000, 8000 (default) and 16000. All other values are rounded up.
>   - compound-device : indicated the hub is part of a compound device
>   - port-mapping-mode : enable port mapping mode
>   - string-support : enable string descriptor support (required for manufacturer,
> @@ -40,9 +41,9 @@ 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
> - - power-on-time : 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).
> + - 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).

Okay for the binding, but...

>  
>  Examples:
>  	usb2512b@2c {
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index 3f9c306..58b887a 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
> @@ -375,18 +375,24 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
>  	if (of_get_property(np, "dynamic-power-switching", NULL))
>  		hub->conf_data2 |= BIT(7);
>  
> -	if (of_get_property(np, "oc-delay-100us", NULL)) {
> -		hub->conf_data2 &= ~BIT(5);
> -		hub->conf_data2 &= ~BIT(4);
> -	} else if (of_get_property(np, "oc-delay-4ms", NULL)) {
> -		hub->conf_data2 &= ~BIT(5);
> -		hub->conf_data2 |= BIT(4);
> -	} else if (of_get_property(np, "oc-delay-8ms", NULL)) {
> -		hub->conf_data2 |= BIT(5);
> -		hub->conf_data2 &= ~BIT(4);
> -	} else if (of_get_property(np, "oc-delay-16ms", NULL)) {
> -		hub->conf_data2 |= BIT(5);
> -		hub->conf_data2 |= BIT(4);
> +	if (!of_property_read_u32(np, "oc-delay-us", property_u32)) {
> +		if (*property_u32 < 100) {

What does the time control? A protection mechanism or just to filter 
notifications? For the former, it seems like you would want the time 
specified to be a maximum. So for example if the user says 150us, then 
you want to make sure the OC condition doesn't exceed that time and 
therefore set it to 100us.

Or you could require exact values and set a default if the value doesn't 
match.

> +			/* 100 us*/
> +			hub->conf_data2 &= ~BIT(5);
> +			hub->conf_data2 &= ~BIT(4);
> +		} else if (*property_u32 < 4000) {
> +			/* 4 ms */
> +			hub->conf_data2 &= ~BIT(5);
> +			hub->conf_data2 |= BIT(4);
> +		} else if (*property_u32 < 8000) {
> +			/* 8 ms */
> +			hub->conf_data2 |= BIT(5);
> +			hub->conf_data2 &= ~BIT(4);
> +		} else {
> +			/* 16 ms */
> +			hub->conf_data2 |= BIT(5);
> +			hub->conf_data2 |= BIT(4);
> +		}
>  	}
>  
>  	if (of_get_property(np, "compound-device", NULL))
> @@ -433,7 +439,7 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
>  	}
>  
>  	hub->power_on_time = USB251XB_DEF_POWER_ON_TIME;
> -	if (!of_property_read_u32(np, "power-on-time", property_u32))
> +	if (!of_property_read_u32(np, "power-on-time-ms", property_u32))
>  		hub->power_on_time = min_t(u8, be32_to_cpu(*property_u32) / 2,

This is a bug. property_u32 is already in cpu endianness.

>  					   255);
>  
> -- 
> 2.1.4
> 

[PATCH 3/3] doc: dt-bindings: usb251xb: mark reg as required

[Richard Leitner]

Mark the reg property as required and furthermore fix some typos and
spellings in the documentation.

Signed-off-by: Richard Leitner <richard.leitner-WcANXNA0UjBBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/usb251xb.txt | 23 +++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb251xb.txt b/Documentation/devicetree/bindings/usb/usb251xb.txt
index 4d6bd73..0408d9e 100644
--- a/Documentation/devicetree/bindings/usb/usb251xb.txt
+++ b/Documentation/devicetree/bindings/usb/usb251xb.txt
@@ -7,18 +7,18 @@ 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"
- - hub-reset-gpios : Should specify the gpio for hub reset
+ - reset-gpios : Should specify the gpio for hub reset
+ - reg : I2C address on the selected bus (default is <0x2C>)
 
 Optional properties :
- - reg : I2C address on the selected bus (default is <0x2C>)
  - skip-config : Skip Hub configuration, but only send the USB-Attach command
- - vendor-id : USB Vendor ID of the hub (16 bit, default is 0x0424)
- - product-id : USB Product ID of the hub (16 bit, default depends on type)
- - device-id : USB Device ID of the hub (16 bit, default is 0x0bb3)
- - language-id : USB Language ID (16 bit, default is 0x0000)
- - manufacturer : USB Manufacturer string (max 31 characters long)
- - product : USB Product string (max 31 characters long)
- - serial : USB Serial string (max 31 characters long)
+ - 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)
+ - device-id : Set USB Device ID of the hub (16 bit, default is 0x0bb3)
+ - language-id : Set USB Language ID (16 bit, default is 0x0000)
+ - manufacturer : Set USB Manufacturer string (max 31 characters long)
+ - product : Set USB Product string (max 31 characters long)
+ - serial : Set USB Serial string (max 31 characters long)
  - {bus,self}-powered : selects between self- and bus-powered operation (default
 	is self-powered)
  - disable-hi-speed : disable USB Hi-Speed support
@@ -33,7 +33,7 @@ Optional properties :
 	operation if the local power source is removed or unavailable
  - oc-delay-us : microseconds over current timer delay, valid values are 100,
 	4000, 8000 (default) and 16000. All other values are rounded up.
- - compound-device : indicated the hub is part of a compound device
+ - compound-device : indicate the hub is part of a compound device
  - port-mapping-mode : enable port mapping mode
  - string-support : enable string descriptor support (required for manufacturer,
 	product and serial string configuration)
@@ -48,7 +48,8 @@ Optional properties :
 Examples:
 	usb2512b@2c {
 		compatible = "microchip,usb2512b";
-		hub-reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
+		reg = <0x2c>;
+		reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
 	};
 
 	usb2514b@2c {
-- 
2.1.4

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