lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 16 Sep 2017 13:42:20 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     richard.leitner@...data.com, gregkh@...uxfoundation.org,
        robh+dt@...nel.org, mark.rutland@....com
Cc:     Sergey.Semin@...latforms.ru, linux-usb@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Serge Semin <fancer.lancer@...il.com>
Subject: [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@...il.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

Powered by blists - more mailing lists