diff mbox series

[2/5] gpio: Congatec Board Controller gpio driver

Message ID 20240503-congatec-board-controller-v1-2-fec5236270e7@bootlin.com (mailing list archive)
State New
Headers show
Series Congatec Board Controller drivers | expand

Commit Message

Thomas Richard Aug. 9, 2024, 2:52 p.m. UTC
Add gpio support for the Congatec Board Controller.

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
 drivers/gpio/Kconfig     |  10 +++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-cgbc.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 214 insertions(+)

Comments

Bartosz Golaszewski Aug. 14, 2024, 9:16 a.m. UTC | #1
On Fri, Aug 9, 2024 at 4:52 PM Thomas Richard
<thomas.richard@bootlin.com> wrote:
>
> Add gpio support for the Congatec Board Controller.
>
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
>  drivers/gpio/Kconfig     |  10 +++
>  drivers/gpio/Makefile    |   1 +
>  drivers/gpio/gpio-cgbc.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 214 insertions(+)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 58f43bcced7c..ce77bad40087 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -233,6 +233,16 @@ config GPIO_CADENCE
>         help
>           Say yes here to enable support for Cadence GPIO controller.
>
> +config GPIO_CGBC
> +       tristate "Congatec Board Controller GPIO support"
> +       depends on MFD_CGBC
> +       help
> +         Select this option to enable GPIO support for the Congatec Board
> +         Controller.
> +
> +         This driver can also be built as a module. If so, the module will be
> +         called gpio-cgbc.
> +
>  config GPIO_CLPS711X
>         tristate "CLPS711X GPIO support"
>         depends on ARCH_CLPS711X || COMPILE_TEST
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 64dd6d9d730d..3a96e3c27a2d 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_GPIO_BD9571MWV)          += gpio-bd9571mwv.o
>  obj-$(CONFIG_GPIO_BRCMSTB)             += gpio-brcmstb.o
>  obj-$(CONFIG_GPIO_BT8XX)               += gpio-bt8xx.o
>  obj-$(CONFIG_GPIO_CADENCE)             += gpio-cadence.o
> +obj-$(CONFIG_GPIO_CGBC)                        += gpio-cgbc.o
>  obj-$(CONFIG_GPIO_CLPS711X)            += gpio-clps711x.o
>  obj-$(CONFIG_GPIO_SNPS_CREG)           += gpio-creg-snps.o
>  obj-$(CONFIG_GPIO_CROS_EC)             += gpio-cros-ec.o
> diff --git a/drivers/gpio/gpio-cgbc.c b/drivers/gpio/gpio-cgbc.c
> new file mode 100644
> index 000000000000..6da50c794872
> --- /dev/null
> +++ b/drivers/gpio/gpio-cgbc.c
> @@ -0,0 +1,203 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Congatec Board Controller GPIO driver
> + *
> + * Copyright (C) 2024 Bootlin
> + * Author: Thomas Richard <thomas.richard@bootlin.com>
> + */
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/mfd/cgbc.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +
> +#define CGBC_GPIO_NGPIO        14
> +
> +#define CGBC_GPIO_CMD_GET      0x64
> +#define CGBC_GPIO_CMD_SET      0x65
> +#define CGBC_GPIO_CMD_DIR_GET  0x66
> +#define CGBC_GPIO_CMD_DIR_SET  0x67
> +
> +struct cgbc_gpio_data {
> +       struct gpio_chip        chip;
> +       struct cgbc_device_data *cgbc;
> +       struct mutex lock;
> +};
> +
> +static int cgbc_gpio_cmd(struct cgbc_device_data *cgbc,
> +                        u8 cmd0, u8 cmd1, u8 cmd2, u8 *value)
> +{
> +       u8 cmd[3] = {cmd0, cmd1, cmd2};
> +
> +       return cgbc_command(cgbc, cmd, sizeof(cmd), value, 1, NULL);
> +}
> +
> +static int cgbc_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +       struct cgbc_device_data *cgbc = gpio->cgbc;
> +       int ret;
> +       u8 val;
> +

Can you use scoped_guard() here and elsewhere?

> +       mutex_lock(&gpio->lock);
> +
> +       ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_GET, (offset > 7) ? 1 : 0, 0, &val);
> +
> +       mutex_unlock(&gpio->lock);
> +
> +       offset %= 8;
> +
> +       if (ret)
> +               return ret;
> +       else
> +               return (int)(val & (u8)BIT(offset));
> +}
> +
> +static void __cgbc_gpio_set(struct gpio_chip *chip,
> +                           unsigned int offset, int value)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +       struct cgbc_device_data *cgbc = gpio->cgbc;
> +       u8 val;
> +       int ret;
> +
> +       ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_GET, (offset > 7) ? 1 : 0, 0, &val);
> +       if (ret)
> +               return;
> +
> +       if (value)
> +               val |= BIT(offset % 8);
> +       else
> +               val &= ~((u8)BIT(offset % 8));
> +
> +       cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_SET, (offset > 7) ? 1 : 0, val, &val);
> +}
> +
> +static void cgbc_gpio_set(struct gpio_chip *chip,
> +                         unsigned int offset, int value)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +
> +       mutex_lock(&gpio->lock);
> +       __cgbc_gpio_set(chip, offset, value);
> +       mutex_unlock(&gpio->lock);
> +}
> +
> +static int cgbc_gpio_direction_set(struct gpio_chip *chip,
> +                                  unsigned int offset, int direction)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +       struct cgbc_device_data *cgbc = gpio->cgbc;
> +       int ret;
> +       u8 val;
> +
> +       ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_DIR_GET, (offset > 7) ? 1 : 0, 0, &val);
> +       if (ret)
> +               goto end;
> +
> +       if (direction == GPIO_LINE_DIRECTION_IN)
> +               val &= ~((u8)BIT(offset % 8));
> +       else
> +               val |= BIT(offset % 8);
> +
> +       ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_DIR_SET, (offset > 7) ? 1 : 0, val, &val);
> +
> +end:
> +       return ret;
> +}
> +
> +static int cgbc_gpio_direction_input(struct gpio_chip *chip,
> +                                    unsigned int offset)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +       int ret;
> +
> +       mutex_lock(&gpio->lock);
> +       ret = cgbc_gpio_direction_set(chip, offset, GPIO_LINE_DIRECTION_IN);
> +       mutex_unlock(&gpio->lock);
> +
> +       return ret;
> +}
> +
> +static int cgbc_gpio_direction_output(struct gpio_chip *chip,
> +                                     unsigned int offset, int value)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +       int ret;
> +
> +       mutex_lock(&gpio->lock);
> +       __cgbc_gpio_set(chip, offset, value);
> +       ret = cgbc_gpio_direction_set(chip, offset, GPIO_LINE_DIRECTION_OUT);
> +       mutex_unlock(&gpio->lock);
> +
> +       return ret;
> +}
> +
> +static int cgbc_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
> +       struct cgbc_device_data *cgbc = gpio->cgbc;
> +       int ret;
> +       u8 val;
> +
> +       ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_DIR_GET, (offset > 7) ? 1 : 0, 0, &val);
> +       if (ret)
> +               return ret;
> +
> +       if (val & BIT(offset % 8))
> +               return GPIO_LINE_DIRECTION_OUT;
> +       else
> +               return GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int cgbc_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct cgbc_device_data *cgbc = dev_get_drvdata(dev->parent);
> +       struct cgbc_gpio_data *gpio;
> +       struct gpio_chip *chip;
> +       int ret;
> +
> +       gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
> +       if (!gpio)
> +               return -ENOMEM;
> +
> +       gpio->cgbc = cgbc;
> +
> +       platform_set_drvdata(pdev, gpio);
> +
> +       chip = &gpio->chip;
> +       chip->label = dev_name(&pdev->dev);
> +       chip->owner = THIS_MODULE;
> +       chip->parent = dev;
> +       chip->base = -1;
> +       chip->direction_input = cgbc_gpio_direction_input;
> +       chip->direction_output = cgbc_gpio_direction_output;
> +       chip->get_direction = cgbc_gpio_get_direction;
> +       chip->get = cgbc_gpio_get;
> +       chip->set = cgbc_gpio_set;
> +       chip->ngpio = CGBC_GPIO_NGPIO;
> +
> +       mutex_init(&gpio->lock);

Please use devm_mutex_init() so that it gets cleaned up at exit. It's
not strictly necessary but helps with lock debugging.

> +
> +       ret = devm_gpiochip_add_data(dev, chip, gpio);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "Could not register GPIO chip\n");
> +
> +       return 0;
> +}
> +
> +static struct platform_driver cgbc_gpio_driver = {
> +       .driver = {
> +               .name = "cgbc-gpio",
> +       },
> +       .probe  = cgbc_gpio_probe,
> +};
> +
> +module_platform_driver(cgbc_gpio_driver);
> +
> +MODULE_DESCRIPTION("Congatec Board Controller GPIO Driver");
> +MODULE_AUTHOR("Thomas Richard <thomas.richard@bootlin.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:cgbc-gpio");
>
> --
> 2.39.2
>

Bart
Thomas Richard Aug. 19, 2024, 4:11 p.m. UTC | #2
>> +static int cgbc_gpio_get(struct gpio_chip *chip, unsigned int offset)
>> +{
>> +       struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
>> +       struct cgbc_device_data *cgbc = gpio->cgbc;
>> +       int ret;
>> +       u8 val;
>> +
> 
> Can you use scoped_guard() here and elsewhere?

Hi Bartosz,

Thanks for the review.

For the next iteration I added scoped_guard() in cgbc_gpio_get(), and
guard() in cgbc_gpio_set(), cgbc_gpio_direction_input(), and
cgbc_gpio_direction_output().

> 
>> +       mutex_lock(&gpio->lock);
>> +
>> +       ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_GET, (offset > 7) ? 1 : 0, 0, &val);
>> +
>> +       mutex_unlock(&gpio->lock);
>> +
>> +       offset %= 8;
>> +
>> +       if (ret)
>> +               return ret;

...

>> +static int cgbc_gpio_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct cgbc_device_data *cgbc = dev_get_drvdata(dev->parent);
>> +       struct cgbc_gpio_data *gpio;
>> +       struct gpio_chip *chip;
>> +       int ret;
>> +
>> +       gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
>> +       if (!gpio)
>> +               return -ENOMEM;
>> +
>> +       gpio->cgbc = cgbc;
>> +
>> +       platform_set_drvdata(pdev, gpio);
>> +
>> +       chip = &gpio->chip;
>> +       chip->label = dev_name(&pdev->dev);
>> +       chip->owner = THIS_MODULE;
>> +       chip->parent = dev;
>> +       chip->base = -1;
>> +       chip->direction_input = cgbc_gpio_direction_input;
>> +       chip->direction_output = cgbc_gpio_direction_output;
>> +       chip->get_direction = cgbc_gpio_get_direction;
>> +       chip->get = cgbc_gpio_get;
>> +       chip->set = cgbc_gpio_set;
>> +       chip->ngpio = CGBC_GPIO_NGPIO;
>> +
>> +       mutex_init(&gpio->lock);
> 
> Please use devm_mutex_init() so that it gets cleaned up at exit. It's
> not strictly necessary but helps with lock debugging.

Fixed in the next iteration.

Regards,

Thomas
diff mbox series

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 58f43bcced7c..ce77bad40087 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -233,6 +233,16 @@  config GPIO_CADENCE
 	help
 	  Say yes here to enable support for Cadence GPIO controller.
 
+config GPIO_CGBC
+	tristate "Congatec Board Controller GPIO support"
+	depends on MFD_CGBC
+	help
+	  Select this option to enable GPIO support for the Congatec Board
+	  Controller.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called gpio-cgbc.
+
 config GPIO_CLPS711X
 	tristate "CLPS711X GPIO support"
 	depends on ARCH_CLPS711X || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 64dd6d9d730d..3a96e3c27a2d 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -44,6 +44,7 @@  obj-$(CONFIG_GPIO_BD9571MWV)		+= gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BRCMSTB)		+= gpio-brcmstb.o
 obj-$(CONFIG_GPIO_BT8XX)		+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CADENCE)		+= gpio-cadence.o
+obj-$(CONFIG_GPIO_CGBC)			+= gpio-cgbc.o
 obj-$(CONFIG_GPIO_CLPS711X)		+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_SNPS_CREG)		+= gpio-creg-snps.o
 obj-$(CONFIG_GPIO_CROS_EC)		+= gpio-cros-ec.o
diff --git a/drivers/gpio/gpio-cgbc.c b/drivers/gpio/gpio-cgbc.c
new file mode 100644
index 000000000000..6da50c794872
--- /dev/null
+++ b/drivers/gpio/gpio-cgbc.c
@@ -0,0 +1,203 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Congatec Board Controller GPIO driver
+ *
+ * Copyright (C) 2024 Bootlin
+ * Author: Thomas Richard <thomas.richard@bootlin.com>
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/cgbc.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+#define CGBC_GPIO_NGPIO	14
+
+#define CGBC_GPIO_CMD_GET	0x64
+#define CGBC_GPIO_CMD_SET	0x65
+#define CGBC_GPIO_CMD_DIR_GET	0x66
+#define CGBC_GPIO_CMD_DIR_SET	0x67
+
+struct cgbc_gpio_data {
+	struct gpio_chip	chip;
+	struct cgbc_device_data	*cgbc;
+	struct mutex lock;
+};
+
+static int cgbc_gpio_cmd(struct cgbc_device_data *cgbc,
+			 u8 cmd0, u8 cmd1, u8 cmd2, u8 *value)
+{
+	u8 cmd[3] = {cmd0, cmd1, cmd2};
+
+	return cgbc_command(cgbc, cmd, sizeof(cmd), value, 1, NULL);
+}
+
+static int cgbc_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	struct cgbc_device_data *cgbc = gpio->cgbc;
+	int ret;
+	u8 val;
+
+	mutex_lock(&gpio->lock);
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_GET, (offset > 7) ? 1 : 0, 0, &val);
+
+	mutex_unlock(&gpio->lock);
+
+	offset %= 8;
+
+	if (ret)
+		return ret;
+	else
+		return (int)(val & (u8)BIT(offset));
+}
+
+static void __cgbc_gpio_set(struct gpio_chip *chip,
+			    unsigned int offset, int value)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	struct cgbc_device_data *cgbc = gpio->cgbc;
+	u8 val;
+	int ret;
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_GET, (offset > 7) ? 1 : 0, 0, &val);
+	if (ret)
+		return;
+
+	if (value)
+		val |= BIT(offset % 8);
+	else
+		val &= ~((u8)BIT(offset % 8));
+
+	cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_SET, (offset > 7) ? 1 : 0, val, &val);
+}
+
+static void cgbc_gpio_set(struct gpio_chip *chip,
+			  unsigned int offset, int value)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+
+	mutex_lock(&gpio->lock);
+	__cgbc_gpio_set(chip, offset, value);
+	mutex_unlock(&gpio->lock);
+}
+
+static int cgbc_gpio_direction_set(struct gpio_chip *chip,
+				   unsigned int offset, int direction)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	struct cgbc_device_data *cgbc = gpio->cgbc;
+	int ret;
+	u8 val;
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_DIR_GET, (offset > 7) ? 1 : 0, 0, &val);
+	if (ret)
+		goto end;
+
+	if (direction == GPIO_LINE_DIRECTION_IN)
+		val &= ~((u8)BIT(offset % 8));
+	else
+		val |= BIT(offset % 8);
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_DIR_SET, (offset > 7) ? 1 : 0, val, &val);
+
+end:
+	return ret;
+}
+
+static int cgbc_gpio_direction_input(struct gpio_chip *chip,
+				     unsigned int offset)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	int ret;
+
+	mutex_lock(&gpio->lock);
+	ret = cgbc_gpio_direction_set(chip, offset, GPIO_LINE_DIRECTION_IN);
+	mutex_unlock(&gpio->lock);
+
+	return ret;
+}
+
+static int cgbc_gpio_direction_output(struct gpio_chip *chip,
+				      unsigned int offset, int value)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	int ret;
+
+	mutex_lock(&gpio->lock);
+	__cgbc_gpio_set(chip, offset, value);
+	ret = cgbc_gpio_direction_set(chip, offset, GPIO_LINE_DIRECTION_OUT);
+	mutex_unlock(&gpio->lock);
+
+	return ret;
+}
+
+static int cgbc_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct cgbc_gpio_data *gpio = gpiochip_get_data(chip);
+	struct cgbc_device_data *cgbc = gpio->cgbc;
+	int ret;
+	u8 val;
+
+	ret = cgbc_gpio_cmd(cgbc, CGBC_GPIO_CMD_DIR_GET, (offset > 7) ? 1 : 0, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val & BIT(offset % 8))
+		return GPIO_LINE_DIRECTION_OUT;
+	else
+		return GPIO_LINE_DIRECTION_IN;
+}
+
+static int cgbc_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cgbc_device_data *cgbc = dev_get_drvdata(dev->parent);
+	struct cgbc_gpio_data *gpio;
+	struct gpio_chip *chip;
+	int ret;
+
+	gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
+	if (!gpio)
+		return -ENOMEM;
+
+	gpio->cgbc = cgbc;
+
+	platform_set_drvdata(pdev, gpio);
+
+	chip = &gpio->chip;
+	chip->label = dev_name(&pdev->dev);
+	chip->owner = THIS_MODULE;
+	chip->parent = dev;
+	chip->base = -1;
+	chip->direction_input = cgbc_gpio_direction_input;
+	chip->direction_output = cgbc_gpio_direction_output;
+	chip->get_direction = cgbc_gpio_get_direction;
+	chip->get = cgbc_gpio_get;
+	chip->set = cgbc_gpio_set;
+	chip->ngpio = CGBC_GPIO_NGPIO;
+
+	mutex_init(&gpio->lock);
+
+	ret = devm_gpiochip_add_data(dev, chip, gpio);
+	if (ret)
+		return dev_err_probe(dev, ret, "Could not register GPIO chip\n");
+
+	return 0;
+}
+
+static struct platform_driver cgbc_gpio_driver = {
+	.driver = {
+		.name = "cgbc-gpio",
+	},
+	.probe	= cgbc_gpio_probe,
+};
+
+module_platform_driver(cgbc_gpio_driver);
+
+MODULE_DESCRIPTION("Congatec Board Controller GPIO Driver");
+MODULE_AUTHOR("Thomas Richard <thomas.richard@bootlin.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:cgbc-gpio");