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: <3a779fca-5a56-44e5-2acf-12b8abdaf906@crapouillou.net>
Date:   Mon, 8 May 2017 00:05:10 +0200
From:   Paul Cercueil <paul@...pouillou.net>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Alexandre Courbot <gnurou@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ralf Baechle <ralf@...ux-mips.org>
Cc:     Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Maarten ter Huurne <maarten@...ewalker.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Paul Burton <paul.burton@...tec.com>, james.hogan@...tec.com,
        linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mips@...ux-mips.org,
        linux-mmc@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-pwm@...r.kernel.org, linux-fbdev@...r.kernel.org
Subject: Re: [PATCH v5 04/14] GPIO: Add gpio-ingenic driver

It looks like the gpio_get_value() is broken on jz4740.

I'll send a v6.

- Paul

On 28/04/2017 22:08, Paul Cercueil wrote:
> This driver handles the GPIOs of all the Ingenic JZ47xx SoCs
> currently supported by the upsteam Linux kernel.
>
> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
> ---
>   drivers/gpio/Kconfig        |  10 ++
>   drivers/gpio/Makefile       |   1 +
>   drivers/gpio/gpio-ingenic.c | 399 ++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 410 insertions(+)
>   create mode 100644 drivers/gpio/gpio-ingenic.c
>
>   v2: Consider it's a new patch. Completely rewritten from v1.
>   v3: Add missing include <linux/pinctrl/consumer.h> and drop semicolon after }
>   v4: Completely rewritten from v3.
>   v5: Get bank number from 'reg' property; drop dependency on PINCTRL_INGENIC
>       (note: selecting PINCTRL would cause cyclic Kconfig dependencies, that's
>        why I just dropped the dependency on PINCTRL_INGENIC)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 05043071fc98..7a5f9cf123c1 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -241,6 +241,16 @@ config GPIO_ICH
>   
>   	  If unsure, say N.
>   
> +config GPIO_INGENIC
> +	tristate "Ingenic JZ47xx SoCs GPIO support"
> +	depends on MACH_INGENIC || COMPILE_TEST
> +	select GPIOLIB_IRQCHIP
> +	help
> +	  Say yes here to support the GPIO functionality present on the
> +	  JZ4740 and JZ4780 SoCs from Ingenic.
> +
> +	  If unsure, say N.
> +
>   config GPIO_IOP
>   	tristate "Intel IOP GPIO"
>   	depends on ARCH_IOP32X || ARCH_IOP33X || COMPILE_TEST
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index becb96c724fe..3fceb4ab3ca7 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_GPIO_MM)	+= gpio-gpio-mm.o
>   obj-$(CONFIG_GPIO_GRGPIO)	+= gpio-grgpio.o
>   obj-$(CONFIG_HTC_EGPIO)		+= gpio-htc-egpio.o
>   obj-$(CONFIG_GPIO_ICH)		+= gpio-ich.o
> +obj-$(CONFIG_GPIO_INGENIC)	+= gpio-ingenic.o
>   obj-$(CONFIG_GPIO_IOP)		+= gpio-iop.o
>   obj-$(CONFIG_GPIO_IT87)		+= gpio-it87.o
>   obj-$(CONFIG_GPIO_JANZ_TTL)	+= gpio-janz-ttl.o
> diff --git a/drivers/gpio/gpio-ingenic.c b/drivers/gpio/gpio-ingenic.c
> new file mode 100644
> index 000000000000..34e9339bd815
> --- /dev/null
> +++ b/drivers/gpio/gpio-ingenic.c
> @@ -0,0 +1,399 @@
> +/*
> + * Ingenic JZ47xx GPIO driver
> + *
> + * Copyright (c) 2017 Paul Cercueil <paul@...pouillou.net>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/regmap.h>
> +
> +#define GPIO_PIN	0x00
> +#define GPIO_MSK	0x20
> +
> +#define JZ4740_GPIO_DATA	0x10
> +#define JZ4740_GPIO_SELECT	0x50
> +#define JZ4740_GPIO_DIR		0x60
> +#define JZ4740_GPIO_TRIG	0x70
> +#define JZ4740_GPIO_FLAG	0x80
> +
> +#define JZ4770_GPIO_INT		0x10
> +#define JZ4770_GPIO_PAT1	0x30
> +#define JZ4770_GPIO_PAT0	0x40
> +#define JZ4770_GPIO_FLAG	0x50
> +
> +#define REG_SET(x) ((x) + 0x4)
> +#define REG_CLEAR(x) ((x) + 0x8)
> +
> +enum jz_version {
> +	ID_JZ4740,
> +	ID_JZ4770,
> +	ID_JZ4780,
> +};
> +
> +struct ingenic_gpio_chip {
> +	struct regmap *map;
> +	struct gpio_chip gc;
> +	struct irq_chip irq_chip;
> +	unsigned int irq, reg_base;
> +	enum jz_version version;
> +};
> +
> +static u32 gpio_ingenic_read_reg(struct ingenic_gpio_chip *jzgc, u8 reg)
> +{
> +	unsigned int val;
> +
> +	regmap_read(jzgc->map, jzgc->reg_base + reg, &val);
> +
> +	return (u32) val;
> +}
> +
> +static void gpio_ingenic_set_bit(struct ingenic_gpio_chip *jzgc,
> +		u8 reg, u8 offset, bool set)
> +{
> +	if (set)
> +		reg = REG_SET(reg);
> +	else
> +		reg = REG_CLEAR(reg);
> +
> +	regmap_write(jzgc->map, jzgc->reg_base + reg, BIT(offset));
> +}
> +
> +static inline bool gpio_get_value(struct ingenic_gpio_chip *jzgc, u8 offset)
> +{
> +	unsigned int val;
> +
> +	if (jzgc->version >= ID_JZ4770)
> +		val = gpio_ingenic_read_reg(jzgc, GPIO_PIN);
> +	else
> +		val = gpio_ingenic_read_reg(jzgc, JZ4740_GPIO_DATA);
> +
> +	return !!(val & BIT(offset));
> +}
> +
> +static void gpio_set_value(struct ingenic_gpio_chip *jzgc, u8 offset, int value)
> +{
> +	if (jzgc->version >= ID_JZ4770)
> +		gpio_ingenic_set_bit(jzgc, JZ4770_GPIO_PAT0, offset, !!value);
> +	else
> +		gpio_ingenic_set_bit(jzgc, JZ4740_GPIO_DATA, offset, !!value);
> +}
> +
> +static void irq_set_type(struct ingenic_gpio_chip *jzgc,
> +		u8 offset, unsigned int type)
> +{
> +	u8 reg1, reg2;
> +
> +	if (jzgc->version >= ID_JZ4770) {
> +		reg1 = JZ4770_GPIO_PAT1;
> +		reg2 = JZ4770_GPIO_PAT0;
> +	} else {
> +		reg1 = JZ4740_GPIO_TRIG;
> +		reg2 = JZ4740_GPIO_DIR;
> +	}
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		gpio_ingenic_set_bit(jzgc, reg2, offset, true);
> +		gpio_ingenic_set_bit(jzgc, reg1, offset, true);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		gpio_ingenic_set_bit(jzgc, reg2, offset, false);
> +		gpio_ingenic_set_bit(jzgc, reg1, offset, true);
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		gpio_ingenic_set_bit(jzgc, reg2, offset, true);
> +		gpio_ingenic_set_bit(jzgc, reg1, offset, false);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +	default:
> +		gpio_ingenic_set_bit(jzgc, reg2, offset, false);
> +		gpio_ingenic_set_bit(jzgc, reg1, offset, false);
> +		break;
> +	}
> +}
> +
> +static void ingenic_gpio_irq_mask(struct irq_data *irqd)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +
> +	gpio_ingenic_set_bit(jzgc, GPIO_MSK, irqd->hwirq, true);
> +}
> +
> +static void ingenic_gpio_irq_unmask(struct irq_data *irqd)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +
> +	gpio_ingenic_set_bit(jzgc, GPIO_MSK, irqd->hwirq, false);
> +}
> +
> +static void ingenic_gpio_irq_enable(struct irq_data *irqd)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +	int irq = irqd->hwirq;
> +
> +	if (jzgc->version >= ID_JZ4770)
> +		gpio_ingenic_set_bit(jzgc, JZ4770_GPIO_INT, irq, true);
> +	else
> +		gpio_ingenic_set_bit(jzgc, JZ4740_GPIO_SELECT, irq, true);
> +
> +	ingenic_gpio_irq_unmask(irqd);
> +}
> +
> +static void ingenic_gpio_irq_disable(struct irq_data *irqd)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +	int irq = irqd->hwirq;
> +
> +	ingenic_gpio_irq_mask(irqd);
> +
> +	if (jzgc->version >= ID_JZ4770)
> +		gpio_ingenic_set_bit(jzgc, JZ4770_GPIO_INT, irq, false);
> +	else
> +		gpio_ingenic_set_bit(jzgc, JZ4740_GPIO_SELECT, irq, false);
> +}
> +
> +static void ingenic_gpio_irq_ack(struct irq_data *irqd)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +	int irq = irqd->hwirq;
> +	bool high;
> +
> +	if (irqd_get_trigger_type(irqd) == IRQ_TYPE_EDGE_BOTH) {
> +		/*
> +		 * Switch to an interrupt for the opposite edge to the one that
> +		 * triggered the interrupt being ACKed.
> +		 */
> +		high = gpio_get_value(jzgc, irq);
> +		if (high)
> +			irq_set_type(jzgc, irq, IRQ_TYPE_EDGE_FALLING);
> +		else
> +			irq_set_type(jzgc, irq, IRQ_TYPE_EDGE_RISING);
> +	}
> +
> +	if (jzgc->version >= ID_JZ4770)
> +		gpio_ingenic_set_bit(jzgc, JZ4770_GPIO_FLAG, irq, false);
> +	else
> +		gpio_ingenic_set_bit(jzgc, JZ4740_GPIO_DATA, irq, true);
> +}
> +
> +static int ingenic_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_BOTH:
> +	case IRQ_TYPE_EDGE_RISING:
> +	case IRQ_TYPE_EDGE_FALLING:
> +		irq_set_handler_locked(irqd, handle_edge_irq);
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +	case IRQ_TYPE_LEVEL_LOW:
> +		irq_set_handler_locked(irqd, handle_level_irq);
> +		break;
> +	default:
> +		irq_set_handler_locked(irqd, handle_bad_irq);
> +	}
> +
> +	if (type == IRQ_TYPE_EDGE_BOTH) {
> +		/*
> +		 * The hardware does not support interrupts on both edges. The
> +		 * best we can do is to set up a single-edge interrupt and then
> +		 * switch to the opposing edge when ACKing the interrupt.
> +		 */
> +		bool high = gpio_get_value(jzgc, irqd->hwirq);
> +
> +		type = high ? IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING;
> +	}
> +
> +	irq_set_type(jzgc, irqd->hwirq, type);
> +	return 0;
> +}
> +
> +static int ingenic_gpio_irq_set_wake(struct irq_data *irqd, unsigned int on)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +
> +	return irq_set_irq_wake(jzgc->irq, on);
> +}
> +
> +static void ingenic_gpio_irq_handler(struct irq_desc *desc)
> +{
> +	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +	struct irq_chip *irq_chip = irq_data_get_irq_chip(&desc->irq_data);
> +	unsigned long flag, i;
> +
> +	chained_irq_enter(irq_chip, desc);
> +
> +	if (jzgc->version >= ID_JZ4770)
> +		flag = gpio_ingenic_read_reg(jzgc, JZ4770_GPIO_FLAG);
> +	else
> +		flag = gpio_ingenic_read_reg(jzgc, JZ4740_GPIO_FLAG);
> +
> +	for_each_set_bit(i, &flag, 32)
> +		generic_handle_irq(irq_linear_revmap(gc->irqdomain, i));
> +	chained_irq_exit(irq_chip, desc);
> +}
> +
> +static void ingenic_gpio_set(struct gpio_chip *gc,
> +		unsigned int offset, int value)
> +{
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +
> +	gpio_set_value(jzgc, offset, value);
> +}
> +
> +static int ingenic_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct ingenic_gpio_chip *jzgc = gpiochip_get_data(gc);
> +
> +	return (int) gpio_get_value(jzgc, offset);
> +}
> +
> +static int ingenic_gpio_direction_input(struct gpio_chip *gc,
> +		unsigned int offset)
> +{
> +	return pinctrl_gpio_direction_input(gc->base + offset);
> +}
> +
> +static int ingenic_gpio_direction_output(struct gpio_chip *gc,
> +		unsigned int offset, int value)
> +{
> +	ingenic_gpio_set(gc, offset, value);
> +	return pinctrl_gpio_direction_output(gc->base + offset);
> +}
> +
> +static const struct of_device_id ingenic_gpio_of_match[] = {
> +	{ .compatible = "ingenic,jz4740-gpio", .data = (void *)ID_JZ4740 },
> +	{ .compatible = "ingenic,jz4770-gpio", .data = (void *)ID_JZ4770 },
> +	{ .compatible = "ingenic,jz4780-gpio", .data = (void *)ID_JZ4780 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_gpio_of_match);
> +
> +static int ingenic_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *of_id = of_match_device(
> +			ingenic_gpio_of_match, dev);
> +	struct ingenic_gpio_chip *jzgc;
> +	u32 bank;
> +	int err;
> +
> +	jzgc = devm_kzalloc(dev, sizeof(*jzgc), GFP_KERNEL);
> +	if (!jzgc)
> +		return -ENOMEM;
> +
> +	jzgc->map = dev_get_drvdata(dev->parent);
> +	if (!jzgc->map) {
> +		dev_err(dev, "Cannot get parent regmap\n");
> +		return -ENXIO;
> +	}
> +
> +	err = of_property_read_u32(dev->of_node, "reg", &bank);
> +	if (err) {
> +		dev_err(dev, "Cannot read \"reg\" property: %i\n", err);
> +		return err;
> +	}
> +
> +	jzgc->reg_base = bank * 0x100;
> +
> +	jzgc->gc.label = devm_kasprintf(dev, GFP_KERNEL, "GPIO%c", 'A' + bank);
> +	if (!jzgc->gc.label)
> +		return -ENOMEM;
> +
> +	/* DO NOT EXPAND THIS: FOR BACKWARD GPIO NUMBERSPACE COMPATIBIBILITY
> +	 * ONLY: WORK TO TRANSITION CONSUMERS TO USE THE GPIO DESCRIPTOR API IN
> +	 * <linux/gpio/consumer.h> INSTEAD.
> +	 */
> +	jzgc->gc.base = bank * 32;
> +
> +	jzgc->gc.ngpio = 32;
> +	jzgc->gc.parent = dev;
> +	jzgc->gc.of_node = dev->of_node;
> +	jzgc->gc.owner = THIS_MODULE;
> +	jzgc->version = (enum jz_version)of_id->data;
> +
> +	jzgc->gc.set = ingenic_gpio_set;
> +	jzgc->gc.get = ingenic_gpio_get;
> +	jzgc->gc.direction_input = ingenic_gpio_direction_input;
> +	jzgc->gc.direction_output = ingenic_gpio_direction_output;
> +
> +	if (of_property_read_bool(dev->of_node, "gpio-ranges")) {
> +		jzgc->gc.request = gpiochip_generic_request;
> +		jzgc->gc.free = gpiochip_generic_free;
> +	}
> +
> +	err = devm_gpiochip_add_data(dev, &jzgc->gc, jzgc);
> +	if (err)
> +		return err;
> +
> +	jzgc->irq = irq_of_parse_and_map(dev->of_node, 0);
> +	if (!jzgc->irq)
> +		return -EINVAL;
> +
> +	jzgc->irq_chip.name = jzgc->gc.label;
> +	jzgc->irq_chip.irq_enable = ingenic_gpio_irq_enable;
> +	jzgc->irq_chip.irq_disable = ingenic_gpio_irq_disable;
> +	jzgc->irq_chip.irq_unmask = ingenic_gpio_irq_unmask;
> +	jzgc->irq_chip.irq_mask = ingenic_gpio_irq_mask;
> +	jzgc->irq_chip.irq_ack = ingenic_gpio_irq_ack;
> +	jzgc->irq_chip.irq_set_type = ingenic_gpio_irq_set_type;
> +	jzgc->irq_chip.irq_set_wake = ingenic_gpio_irq_set_wake;
> +	jzgc->irq_chip.flags = IRQCHIP_MASK_ON_SUSPEND;
> +
> +	err = gpiochip_irqchip_add(&jzgc->gc, &jzgc->irq_chip, 0,
> +			handle_level_irq, IRQ_TYPE_NONE);
> +	if (err)
> +		return err;
> +
> +	gpiochip_set_chained_irqchip(&jzgc->gc, &jzgc->irq_chip,
> +			jzgc->irq, ingenic_gpio_irq_handler);
> +	return 0;
> +}
> +
> +static int ingenic_gpio_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static struct platform_driver ingenic_gpio_driver = {
> +	.driver = {
> +		.name = "gpio-ingenic",
> +		.of_match_table = of_match_ptr(ingenic_gpio_of_match),
> +	},
> +	.probe = ingenic_gpio_probe,
> +	.remove = ingenic_gpio_remove,
> +};
> +
> +static int __init ingenic_gpio_drv_register(void)
> +{
> +	return platform_driver_register(&ingenic_gpio_driver);
> +}
> +subsys_initcall(ingenic_gpio_drv_register);
> +
> +static void __exit ingenic_gpio_drv_unregister(void)
> +{
> +	platform_driver_unregister(&ingenic_gpio_driver);
> +}
> +module_exit(ingenic_gpio_drv_unregister);
> +
> +MODULE_AUTHOR("Paul Cercueil <paul@...pouillou.net>");
> +MODULE_DESCRIPTION("Ingenic JZ47xx GPIO driver");
> +MODULE_LICENSE("GPL");


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ