| 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
| ||
|
Message-ID: <CAMRc=MeC4q3BGxycxnOZCC8nD7p=8AO9rQasb5Gd4T1E+aKvHA@mail.gmail.com>
Date: Wed, 14 Aug 2024 11:16:39 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Thomas Richard <thomas.richard@...tlin.com>
Cc: Lee Jones <lee@...nel.org>, Linus Walleij <linus.walleij@...aro.org>,
Andi Shyti <andi.shyti@...nel.org>, Wim Van Sebroeck <wim@...ux-watchdog.org>,
Guenter Roeck <linux@...ck-us.net>, linux-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-i2c@...r.kernel.org,
linux-watchdog@...r.kernel.org, thomas.petazzoni@...tlin.com,
blake.vermeer@...sight.com
Subject: Re: [PATCH 2/5] gpio: Congatec Board Controller gpio driver
On Fri, Aug 9, 2024 at 4:52 PM Thomas Richard
<thomas.richard@...tlin.com> wrote:
>
> Add gpio support for the Congatec Board Controller.
>
> Signed-off-by: Thomas Richard <thomas.richard@...tlin.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@...tlin.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@...tlin.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:cgbc-gpio");
>
> --
> 2.39.2
>
Bart
Powered by blists - more mailing lists