[V1,6/6] USB: serial: f81232: Add gpiolib to GPIO device
diff mbox series

Message ID 1559789656-15847-7-git-send-email-hpeter+linux_kernel@gmail.com
State New
Headers show
Series
  • USB: serial: f81232: Add F81534A support
Related show

Commit Message

Ji-Ze Hong (Peter Hong) June 6, 2019, 2:54 a.m. UTC
The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
is 12x3 = 36 GPIOs.

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/usb/serial/f81232.c | 210 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

Comments

Johan Hovold Aug. 28, 2019, 3:37 p.m. UTC | #1
On Thu, Jun 06, 2019 at 10:54:16AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The Fintek F81534A series contains 3 GPIOs per UART and The max GPIOs
> is 12x3 = 36 GPIOs.

How does this relate to the GPIOs used for transceiver setup? Are these
really general purpose?

Side note: Please explain the relationship to f81534 which you already
have a driver for. Is f81534a all that different that it belongs with
f81232? Confusing...

> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
> ---
>  drivers/usb/serial/f81232.c | 210 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 210 insertions(+)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 708d85c7d822..a53240bc164a 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -18,6 +18,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/mutex.h>
>  #include <linux/uaccess.h>
> +#include <linux/gpio.h>
>  #include <linux/usb.h>
>  #include <linux/usb/serial.h>
>  #include <linux/serial_reg.h>
> @@ -132,6 +133,7 @@ struct f81232_private {
>  
>  struct f81534a_ctrl_private {
>  	struct usb_interface *intf;
> +	struct gpio_chip chip;
>  	struct mutex lock;
>  	int device_idx;
>  };
> @@ -1007,6 +1009,204 @@ static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
>  	return status;
>  }
>  
> +static int f81534a_ctrl_set_mask_register(struct usb_device *dev, u16 reg,
> +		u8 mask, u8 val)
> +{
> +	int status;
> +	u8 tmp;
> +
> +	status = f81534a_ctrl_get_register(dev, reg, 1, &tmp);
> +	if (status)
> +		return status;
> +
> +
> +	tmp = (tmp & ~mask) | (val & mask);
> +
> +	status = f81534a_ctrl_set_register(dev, reg, 1, &tmp);
> +	if (status)
> +		return status;
> +
> +	return 0;
> +}

You'll get a warning about this one being unused with !GPIOLIB.

> +#ifdef CONFIG_GPIOLIB
> +static int f81534a_gpio_get(struct gpio_chip *chip, unsigned int gpio_num)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	u8 tmp[2];
> +	int set;
> +	int idx;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +
> +	status = mutex_lock_interruptible(&priv->lock);

Why _interruptible?

> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
> +							sizeof(tmp), tmp);
> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;
> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return !!(tmp[1] & BIT(idx));
> +}
> +
> +static int f81534a_gpio_direction_in(struct gpio_chip *chip,
> +					unsigned int gpio_num)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	int set;
> +	int idx;
> +	u8 mask;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = F81534A_GPIO_MODE0_DIR << idx;
> +
> +	status = mutex_lock_interruptible(&priv->lock);
> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
> +				set, mask, mask);
> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;

Just return status below instead.

> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static int f81534a_gpio_direction_out(struct gpio_chip *chip,
> +				     unsigned int gpio_num, int val)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	int set;
> +	int idx;
> +	u8 mask;
> +	u8 data;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = (F81534A_GPIO_MODE0_DIR << idx) | BIT(idx);
> +	data = val ? BIT(idx) : 0;
> +
> +	status = mutex_lock_interruptible(&priv->lock);
> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
> +				set, mask, data);

Please keep set on the same line as the reg define, but why not
calculate a reg temporary above instead?

> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;

As above.

> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static void f81534a_gpio_set(struct gpio_chip *chip, unsigned int gpio_num,
> +				int val)
> +{
> +	f81534a_gpio_direction_out(chip, gpio_num, val);
> +}
> +
> +static int f81534a_gpio_get_direction(struct gpio_chip *chip,
> +				unsigned int gpio_num)
> +{
> +	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
> +	struct usb_interface *intf = priv->intf;
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	int status;
> +	u8 tmp[2];
> +	int set;
> +	int idx;
> +	u8 mask;
> +
> +	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
> +	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
> +	mask = F81534A_GPIO_MODE0_DIR << idx;
> +
> +	status = mutex_lock_interruptible(&priv->lock);
> +	if (status)
> +		return -EINTR;
> +
> +	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
> +							sizeof(tmp), tmp);
> +	if (status) {
> +		mutex_unlock(&priv->lock);
> +		return status;
> +	}
> +
> +	mutex_unlock(&priv->lock);
> +
> +	if (tmp[0] & mask)
> +		return GPIOF_DIR_IN;
> +
> +	return GPIOF_DIR_OUT;
> +}
> +
> +static int f81534a_prepare_gpio(struct usb_interface *intf)
> +{
> +	struct f81534a_ctrl_private *priv = usb_get_intfdata(intf);
> +	int status;
> +
> +	priv->chip.parent = &intf->dev;
> +	priv->chip.owner = THIS_MODULE;
> +	priv->chip.get_direction = f81534a_gpio_get_direction,
> +	priv->chip.get = f81534a_gpio_get;
> +	priv->chip.direction_input = f81534a_gpio_direction_in;
> +	priv->chip.set = f81534a_gpio_set;
> +	priv->chip.direction_output = f81534a_gpio_direction_out;
> +	priv->chip.label = "f81534a";
> +	/* M0(SD)/M1/M2 */
> +	priv->chip.ngpio = F81534A_CTRL_GPIO_MAX_PIN * F81534A_MAX_PORT;

It looks like you should have one gpiochip per port.

> +	priv->chip.base = -1;

You need to set the can_sleep flag.

> +
> +	priv->intf = intf;
> +	mutex_init(&priv->lock);
> +
> +	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
> +	if (status) {
> +		dev_err(&intf->dev, "%s: failed: %d\n", __func__, status);

No need for __func__. Spell what went wrong (e.g. "failed to register
gpiochip: %d\n").

> +		return status;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static int f81534a_prepare_gpio(struct usb_interface *intf)
> +{
> +	dev_warn(&intf->dev, "CONFIG_GPIOLIB is not enabled\n");
> +	dev_warn(&intf->dev, "The GPIOLIB interface will not register\n");

Please remove this.

> +
> +	return 0;
> +}
> +#endif
> +
> +static int f81534a_release_gpio(struct usb_interface *intf)
> +{
> +	return 0;
> +}

Remove.

> +
>  static int f81534a_ctrl_generate_ports(struct usb_interface *intf,
>  					struct f81534a_device *device)
>  {
> @@ -1118,6 +1318,7 @@ static int f81534a_ctrl_probe(struct usb_interface *intf,
>  	struct usb_device *dev = interface_to_usbdev(intf);
>  	struct f81534a_ctrl_private *priv;
>  	struct f81534a_device *device;
> +	int status;
>  
>  	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -1130,6 +1331,10 @@ static int f81534a_ctrl_probe(struct usb_interface *intf,
>  	mutex_init(&priv->lock);
>  	usb_set_intfdata(intf, priv);
>  
> +	status = f81534a_prepare_gpio(intf);
> +	if (status)
> +		return status;
> +
>  	INIT_LIST_HEAD(&device->list);
>  	device->intf = intf;
>  
> @@ -1158,6 +1363,11 @@ static void f81534a_ctrl_disconnect(struct usb_interface *intf)
>  
>  			priv = usb_get_intfdata(intf);
>  			dev = interface_to_usbdev(intf);
> +
> +			mutex_lock(&priv->lock);
> +			f81534a_release_gpio(intf);
> +			mutex_unlock(&priv->lock);
> +
>  			usb_put_dev(dev);
>  			break;
>  		}

Johan

Patch
diff mbox series

diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 708d85c7d822..a53240bc164a 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -18,6 +18,7 @@ 
 #include <linux/moduleparam.h>
 #include <linux/mutex.h>
 #include <linux/uaccess.h>
+#include <linux/gpio.h>
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 #include <linux/serial_reg.h>
@@ -132,6 +133,7 @@  struct f81232_private {
 
 struct f81534a_ctrl_private {
 	struct usb_interface *intf;
+	struct gpio_chip chip;
 	struct mutex lock;
 	int device_idx;
 };
@@ -1007,6 +1009,204 @@  static int f81534a_ctrl_set_register(struct usb_device *dev, u16 reg, u16 size,
 	return status;
 }
 
+static int f81534a_ctrl_set_mask_register(struct usb_device *dev, u16 reg,
+		u8 mask, u8 val)
+{
+	int status;
+	u8 tmp;
+
+	status = f81534a_ctrl_get_register(dev, reg, 1, &tmp);
+	if (status)
+		return status;
+
+
+	tmp = (tmp & ~mask) | (val & mask);
+
+	status = f81534a_ctrl_set_register(dev, reg, 1, &tmp);
+	if (status)
+		return status;
+
+	return 0;
+}
+
+#ifdef CONFIG_GPIOLIB
+static int f81534a_gpio_get(struct gpio_chip *chip, unsigned int gpio_num)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	u8 tmp[2];
+	int set;
+	int idx;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
+							sizeof(tmp), tmp);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return !!(tmp[1] & BIT(idx));
+}
+
+static int f81534a_gpio_direction_in(struct gpio_chip *chip,
+					unsigned int gpio_num)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	int set;
+	int idx;
+	u8 mask;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = F81534A_GPIO_MODE0_DIR << idx;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
+				set, mask, mask);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int f81534a_gpio_direction_out(struct gpio_chip *chip,
+				     unsigned int gpio_num, int val)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	int set;
+	int idx;
+	u8 mask;
+	u8 data;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = (F81534A_GPIO_MODE0_DIR << idx) | BIT(idx);
+	data = val ? BIT(idx) : 0;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_set_mask_register(dev, F81534A_CTRL_GPIO_REG +
+				set, mask, data);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static void f81534a_gpio_set(struct gpio_chip *chip, unsigned int gpio_num,
+				int val)
+{
+	f81534a_gpio_direction_out(chip, gpio_num, val);
+}
+
+static int f81534a_gpio_get_direction(struct gpio_chip *chip,
+				unsigned int gpio_num)
+{
+	struct f81534a_ctrl_private *priv = gpiochip_get_data(chip);
+	struct usb_interface *intf = priv->intf;
+	struct usb_device *dev = interface_to_usbdev(intf);
+	int status;
+	u8 tmp[2];
+	int set;
+	int idx;
+	u8 mask;
+
+	set = gpio_num / F81534A_CTRL_GPIO_MAX_PIN;
+	idx = gpio_num % F81534A_CTRL_GPIO_MAX_PIN;
+	mask = F81534A_GPIO_MODE0_DIR << idx;
+
+	status = mutex_lock_interruptible(&priv->lock);
+	if (status)
+		return -EINTR;
+
+	status = f81534a_ctrl_get_register(dev, F81534A_CTRL_GPIO_REG + set,
+							sizeof(tmp), tmp);
+	if (status) {
+		mutex_unlock(&priv->lock);
+		return status;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	if (tmp[0] & mask)
+		return GPIOF_DIR_IN;
+
+	return GPIOF_DIR_OUT;
+}
+
+static int f81534a_prepare_gpio(struct usb_interface *intf)
+{
+	struct f81534a_ctrl_private *priv = usb_get_intfdata(intf);
+	int status;
+
+	priv->chip.parent = &intf->dev;
+	priv->chip.owner = THIS_MODULE;
+	priv->chip.get_direction = f81534a_gpio_get_direction,
+	priv->chip.get = f81534a_gpio_get;
+	priv->chip.direction_input = f81534a_gpio_direction_in;
+	priv->chip.set = f81534a_gpio_set;
+	priv->chip.direction_output = f81534a_gpio_direction_out;
+	priv->chip.label = "f81534a";
+	/* M0(SD)/M1/M2 */
+	priv->chip.ngpio = F81534A_CTRL_GPIO_MAX_PIN * F81534A_MAX_PORT;
+	priv->chip.base = -1;
+
+	priv->intf = intf;
+	mutex_init(&priv->lock);
+
+	status = devm_gpiochip_add_data(&intf->dev, &priv->chip, priv);
+	if (status) {
+		dev_err(&intf->dev, "%s: failed: %d\n", __func__, status);
+		return status;
+	}
+
+	return 0;
+}
+#else
+static int f81534a_prepare_gpio(struct usb_interface *intf)
+{
+	dev_warn(&intf->dev, "CONFIG_GPIOLIB is not enabled\n");
+	dev_warn(&intf->dev, "The GPIOLIB interface will not register\n");
+
+	return 0;
+}
+#endif
+
+static int f81534a_release_gpio(struct usb_interface *intf)
+{
+	return 0;
+}
+
 static int f81534a_ctrl_generate_ports(struct usb_interface *intf,
 					struct f81534a_device *device)
 {
@@ -1118,6 +1318,7 @@  static int f81534a_ctrl_probe(struct usb_interface *intf,
 	struct usb_device *dev = interface_to_usbdev(intf);
 	struct f81534a_ctrl_private *priv;
 	struct f81534a_device *device;
+	int status;
 
 	priv = devm_kzalloc(&intf->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1130,6 +1331,10 @@  static int f81534a_ctrl_probe(struct usb_interface *intf,
 	mutex_init(&priv->lock);
 	usb_set_intfdata(intf, priv);
 
+	status = f81534a_prepare_gpio(intf);
+	if (status)
+		return status;
+
 	INIT_LIST_HEAD(&device->list);
 	device->intf = intf;
 
@@ -1158,6 +1363,11 @@  static void f81534a_ctrl_disconnect(struct usb_interface *intf)
 
 			priv = usb_get_intfdata(intf);
 			dev = interface_to_usbdev(intf);
+
+			mutex_lock(&priv->lock);
+			f81534a_release_gpio(intf);
+			mutex_unlock(&priv->lock);
+
 			usb_put_dev(dev);
 			break;
 		}