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]
Message-ID: <CAMRc=MdJszmZ8d1MGo=bfJ8TwqOYBPLe2Jfc9MfbErDUCMQktg@mail.gmail.com>
Date: Thu, 20 Feb 2025 14:34:54 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Yixun Lan <dlan@...too.org>
Cc: Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Conor Dooley <conor@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Yangyu Chen <cyy@...self.name>, 
	Jisheng Zhang <jszhang@...nel.org>, Jesse Taube <mr.bossman075@...il.com>, 
	Inochi Amaoto <inochiama@...look.com>, Icenowy Zheng <uwu@...nowy.me>, 
	Meng Zhang <zhangmeng.kevin@...ux.spacemit.com>, linux-gpio@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev
Subject: Re: [PATCH v5 3/5] gpio: spacemit: add support for K1 SoC

On Mon, Feb 17, 2025 at 1:58 PM Yixun Lan <dlan@...too.org> wrote:
>
> Implement GPIO functionality which capable of setting pin as
> input, output. Also, each pin can be used as interrupt which
> support rising, failing, or both edge type trigger.
>
> Signed-off-by: Yixun Lan <dlan@...too.org>
> ---
>  drivers/gpio/Kconfig            |   8 +
>  drivers/gpio/Makefile           |   1 +
>  drivers/gpio/gpio-spacemit-k1.c | 376 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 385 insertions(+)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index add5ad29a673c09082a913cb2404073b2034af48..eaae729eec00a3d6d2b83769aed3e2b0ca9927e5 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -655,6 +655,14 @@ config GPIO_SNPS_CREG
>           where only several fields in register belong to GPIO lines and
>           each GPIO line owns a field with different length and on/off value.
>
> +config GPIO_SPACEMIT_K1
> +       bool "SPACEMIT K1 GPIO support"
> +       depends on ARCH_SPACEMIT || COMPILE_TEST
> +       depends on OF_GPIO
> +       select GPIOLIB_IRQCHIP
> +       help
> +         Say yes here to support the SpacemiT's K1 GPIO device.
> +
>  config GPIO_SPEAR_SPICS
>         bool "ST SPEAr13xx SPI Chip Select as GPIO support"
>         depends on PLAT_SPEAR
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index af3ba4d81b583842893ea69e677fbe2abf31bc7b..6709ce511a0cf10310a94521c85a2d382dcfa696 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_GPIO_SIOX)                     += gpio-siox.o
>  obj-$(CONFIG_GPIO_SL28CPLD)            += gpio-sl28cpld.o
>  obj-$(CONFIG_GPIO_SLOPPY_LOGIC_ANALYZER) += gpio-sloppy-logic-analyzer.o
>  obj-$(CONFIG_GPIO_SODAVILLE)           += gpio-sodaville.o
> +obj-$(CONFIG_GPIO_SPACEMIT_K1)         += gpio-spacemit-k1.o
>  obj-$(CONFIG_GPIO_SPEAR_SPICS)         += gpio-spear-spics.o
>  obj-$(CONFIG_GPIO_SPRD)                        += gpio-sprd.o
>  obj-$(CONFIG_GPIO_STMPE)               += gpio-stmpe.o
> diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f72511b5ab8f8f0b1d1c9e89d2f9ca07b623a866
> --- /dev/null
> +++ b/drivers/gpio/gpio-spacemit-k1.c
> @@ -0,0 +1,376 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2023-2025 SpacemiT (Hangzhou) Technology Co. Ltd
> + * Copyright (C) 2025 Yixun Lan <dlan@...too.org>
> + */
> +
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/property.h>
> +#include <linux/seq_file.h>
> +#include <linux/module.h>
> +

Please order includes alphabetically.

> +#include "gpiolib.h"
> +
> +/* register offset */
> +/* GPIO port level register */
> +#define GPLR           0x00
> +/* GPIO port direction register - R/W */
> +#define GPDR           0x0c
> +/* GPIO port set register - W */
> +#define GPSR           0x18
> +/* GPIO port clear register - W */
> +#define GPCR           0x24
> +/* GPIO port rising edge register R/W */
> +#define GRER           0x30
> +/* GPIO port falling edge register R/W */
> +#define GFER           0x3c
> +/* GPIO edge detect status register - R/W1C */
> +#define GEDR           0x48
> +/*  GPIO (set) direction register - W */
> +#define GSDR           0x54
> +/* GPIO (clear) direction register - W */
> +#define GCDR           0x60
> +/* GPIO (set) rising edge detect enable register - W */
> +#define GSRER          0x6c
> +/* GPIO (clear) rising edge detect enable register - W */
> +#define GCRER          0x78
> +/* GPIO (set) falling edge detect enable register - W */
> +#define GSFER          0x84
> +/* GPIO (clear) falling edge detect enable register - W */
> +#define GCFER          0x90
> +/* GPIO interrupt mask register, 0 disable, 1 enable - R/W */
> +#define GAPMASK                0x9c
> +
> +#define NR_BANKS               4
> +#define NR_GPIOS_PER_BANK      32
> +

Please use a common prefix for all symbols, even for defines. Use
SPACEMIT_GCFER, etc.

> +#define to_spacemit_gpio_bank(x) container_of((x), struct spacemit_gpio_bank, gc)
> +
> +struct spacemit_gpio;
> +
> +struct spacemit_gpio_bank {
> +       struct gpio_chip                gc;
> +       struct spacemit_gpio            *sg;
> +       void __iomem                    *base;
> +       u32                             index;
> +       u32                             irq_mask;
> +       u32                             irq_rising_edge;
> +       u32                             irq_falling_edge;
> +};
> +
> +struct spacemit_gpio {
> +       struct  device                  *dev;
> +       struct  spacemit_gpio_bank      sgb[NR_BANKS];
> +};

Please don't use tabs in struct definitions.

> +
> +static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
> +{
> +       struct spacemit_gpio_bank *gb = dev_id;
> +       unsigned long pending;
> +       u32 n, gedr;
> +
> +       gedr = readl(gb->base + GEDR);
> +       if (!gedr)
> +               return IRQ_NONE;
> +       writel(gedr, gb->base + GEDR);
> +
> +       gedr = gedr & gb->irq_mask;
> +       if (!gedr)
> +               return IRQ_NONE;
> +
> +       pending = gedr;
> +       for_each_set_bit(n, &pending, BITS_PER_LONG)
> +               handle_nested_irq(irq_find_mapping(gb->gc.irq.domain, n));
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void spacemit_gpio_irq_ack(struct irq_data *d)
> +{
> +       struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
> +
> +       writel(BIT(irqd_to_hwirq(d)), gb->base + GEDR);
> +}
> +
> +static void spacemit_gpio_irq_mask(struct irq_data *d)
> +{
> +       struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
> +       u32 bit = BIT(irqd_to_hwirq(d));
> +
> +       gb->irq_mask &= ~bit;
> +
> +       if (bit & gb->irq_rising_edge)
> +               writel(bit, gb->base + GCRER);
> +
> +       if (bit & gb->irq_falling_edge)
> +               writel(bit, gb->base + GCFER);
> +}
> +
> +static void spacemit_gpio_irq_unmask(struct irq_data *d)
> +{
> +       struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
> +       u32 bit = BIT(irqd_to_hwirq(d));
> +
> +       gb->irq_mask |= bit;
> +
> +       if (bit & gb->irq_rising_edge)
> +               writel(bit,  gb->base + GSRER);
> +
> +       if (bit & gb->irq_falling_edge)
> +               writel(bit, gb->base + GSFER);
> +}
> +
> +static int spacemit_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
> +       u32 bit = BIT(irqd_to_hwirq(d));
> +
> +       if (type & IRQ_TYPE_EDGE_RISING) {
> +               gb->irq_rising_edge |= bit;
> +               writel(bit, gb->base + GSRER);
> +       } else {
> +               gb->irq_rising_edge &= ~bit;
> +               writel(bit, gb->base + GCRER);
> +       }
> +
> +       if (type & IRQ_TYPE_EDGE_FALLING) {
> +               gb->irq_falling_edge |= bit;
> +               writel(bit, gb->base + GSFER);
> +       } else {
> +               gb->irq_falling_edge &= ~bit;
> +               writel(bit, gb->base + GCFER);
> +       }
> +
> +       return 0;
> +}
> +
> +static void spacemit_gpio_irq_print_chip(struct irq_data *data, struct seq_file *p)
> +{
> +       struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(data);
> +
> +       seq_printf(p, "%s-%d", dev_name(gb->gc.parent), gb->index);
> +}
> +
> +static struct irq_chip spacemit_gpio_chip = {
> +       .name           = "k1-gpio-irqchip",
> +       .irq_ack        = spacemit_gpio_irq_ack,
> +       .irq_mask       = spacemit_gpio_irq_mask,
> +       .irq_unmask     = spacemit_gpio_irq_unmask,
> +       .irq_set_type   = spacemit_gpio_irq_set_type,
> +       .irq_print_chip = spacemit_gpio_irq_print_chip,
> +       .flags          = IRQCHIP_IMMUTABLE,
> +       GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +static int spacemit_gpio_xlate(struct gpio_chip *gc,
> +                              const struct of_phandle_args *gpiospec, u32 *flags)
> +{
> +       struct spacemit_gpio_bank *gb = gpiochip_get_data(gc);
> +       struct spacemit_gpio *sg = gb->sg;
> +

Stray newline.

> +       int i;
> +
> +       if (gc->of_gpio_n_cells != 3)
> +               return -EINVAL;
> +
> +       if (gpiospec->args_count < gc->of_gpio_n_cells)
> +               return -EINVAL;
> +
> +       i = gpiospec->args[0];
> +       if (i >= NR_BANKS)
> +               return -EINVAL;
> +
> +       if (gc != &sg->sgb[i].gc)
> +               return -EINVAL;
> +
> +       if (gpiospec->args[1] >= gc->ngpio)
> +               return -EINVAL;
> +
> +       if (flags)
> +               *flags = gpiospec->args[2];
> +
> +       return gpiospec->args[1];
> +}
> +
> +static int spacemit_add_pin_range(struct gpio_chip *gc)
> +{
> +       struct spacemit_gpio_bank *gb;
> +       struct of_phandle_args pinspec;
> +       struct pinctrl_dev *pctldev;
> +       struct device_node *np;
> +       int ret, trim;
> +
> +       np = dev_of_node(&gc->gpiodev->dev);
> +       if (!np)
> +               return 0;
> +
> +       gb = to_spacemit_gpio_bank(gc);
> +
> +       ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
> +                                              gb->index, &pinspec);
> +       if (ret)
> +               return ret;
> +
> +       pctldev = of_pinctrl_get(pinspec.np);
> +       of_node_put(pinspec.np);
> +       if (!pctldev)
> +               return -EPROBE_DEFER;
> +
> +       /* Ignore ranges outside of this GPIO chip */
> +       if (pinspec.args[0] >= (gc->offset + gc->ngpio))
> +               return -EINVAL;
> +
> +       if (pinspec.args[0] + pinspec.args[2] <= gc->offset)
> +               return -EINVAL;
> +
> +       if (!pinspec.args[2])
> +               return -EINVAL;
> +
> +       /* Trim the range to fit this GPIO chip */
> +       if (gc->offset > pinspec.args[0]) {
> +               trim = gc->offset - pinspec.args[0];
> +               pinspec.args[2] -= trim;
> +               pinspec.args[1] += trim;
> +               pinspec.args[0] = 0;
> +       } else {
> +               pinspec.args[0] -= gc->offset;
> +       }
> +       if ((pinspec.args[0] + pinspec.args[2]) > gc->ngpio)
> +               pinspec.args[2] = gc->ngpio - pinspec.args[0];
> +
> +       ret = gpiochip_add_pin_range(gc,
> +                                    pinctrl_dev_get_devname(pctldev),
> +                                    pinspec.args[0],
> +                                    pinspec.args[1],
> +                                    pinspec.args[2]);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int spacemit_gpio_add_bank(struct spacemit_gpio *sg,
> +                                 void __iomem *regs,
> +                                 int index, int irq)
> +{
> +       struct spacemit_gpio_bank *gb = &sg->sgb[index];
> +       struct gpio_chip *gc = &gb->gc;
> +       struct device *dev = sg->dev;
> +       struct gpio_irq_chip *girq;
> +       void __iomem *dat, *set, *clr, *dirin, *dirout;
> +       int ret, bank_base[] = { 0x0, 0x4, 0x8, 0x100 };
> +
> +       gb->index = index;
> +       gb->base = regs + bank_base[index];
> +
> +       dat     = gb->base + GPLR;
> +       set     = gb->base + GPSR;
> +       clr     = gb->base + GPCR;
> +       dirin   = gb->base + GCDR;
> +       dirout  = gb->base + GSDR;
> +
> +       /* This registers 32 GPIO lines per bank */
> +       ret = bgpio_init(gc, dev, 4, dat, set, clr, dirout, dirin,
> +                        BGPIOF_UNREADABLE_REG_SET | BGPIOF_UNREADABLE_REG_DIR);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to init gpio chip\n");
> +
> +       gb->sg = sg;
> +
> +       gc->label               = dev_name(dev);
> +       gc->request             = gpiochip_generic_request;
> +       gc->free                = gpiochip_generic_free;
> +       gc->ngpio               = NR_GPIOS_PER_BANK;
> +       gc->base                = -1;
> +
> +#ifdef CONFIG_OF_GPIO
> +       gc->of_xlate            = spacemit_gpio_xlate;
> +       gc->of_add_pin_range    = spacemit_add_pin_range;
> +       gc->of_gpio_n_cells     = 3;
> +#endif
> +
> +       girq                    = &gc->irq;
> +       girq->threaded          = true;
> +       girq->handler           = handle_simple_irq;
> +
> +       gpio_irq_chip_set_chip(girq, &spacemit_gpio_chip);
> +
> +       /* Clear Edge Detection Settings */
> +       writel(0x0, gb->base + GRER);
> +       writel(0x0, gb->base + GFER);
> +       /* Clear and Disable Interrupt */
> +       writel(0xffffffff, gb->base + GCFER);
> +       writel(0xffffffff, gb->base + GCRER);
> +       writel(0, gb->base + GAPMASK);
> +
> +       ret = devm_request_threaded_irq(dev, irq, NULL,
> +                                       spacemit_gpio_irq_handler,
> +                                       IRQF_ONESHOT | IRQF_SHARED,
> +                                       gb->gc.label, gb);
> +       if (ret < 0)
> +               return dev_err_probe(dev, ret, "failed to register IRQ\n");
> +
> +       ret = devm_gpiochip_add_data(dev, gc, gb);
> +       if (ret < 0)
> +               return dev_err_probe(dev, ret, "failed to add gpio chip\n");
> +
> +       /* Eable Interrupt */

Enable.

> +       writel(0xffffffff, gb->base + GAPMASK);
> +
> +       return 0;
> +}
> +
> +static int spacemit_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct spacemit_gpio *sg;
> +       struct resource *res;
> +       void __iomem *regs;
> +       int i, irq, ret;
> +
> +       sg = devm_kzalloc(dev, sizeof(*sg), GFP_KERNEL);
> +       if (!sg)
> +               return -ENOMEM;
> +
> +       regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> +       if (IS_ERR(regs))
> +               return PTR_ERR(regs);
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0)
> +               return irq;
> +
> +       sg->dev = dev;
> +
> +       for (i = 0; i < NR_BANKS; i++) {
> +               ret = spacemit_gpio_add_bank(sg, regs, i, irq);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id spacemit_gpio_dt_ids[] = {
> +       { .compatible = "spacemit,k1-gpio" },
> +       { /* sentinel */ }
> +};
> +
> +static struct platform_driver spacemit_gpio_driver = {
> +       .probe          = spacemit_gpio_probe,
> +       .driver         = {
> +               .name   = "k1-gpio",
> +               .of_match_table = spacemit_gpio_dt_ids,
> +       },
> +};
> +module_platform_driver(spacemit_gpio_driver);
> +
> +MODULE_AUTHOR("Yixun Lan <dlan@...too.org>");
> +MODULE_DESCRIPTION("GPIO driver for SpacemiT K1 SoC");
> +MODULE_LICENSE("GPL");
>
> --
> 2.48.1
>

Bart

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ