Linux GPIO subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] gpio: tegra: fix debugfs compile error
From: Arnd Bergmann @ 2019-07-08 13:19 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Linus Walleij, Bartosz Golaszewski, Thierry Reding,
	Jonathan Hunter, open list:GPIO SUBSYSTEM,
	open list:TEGRA ARCHITECTURE SUPPORT, Linux Kernel Mailing List
In-Reply-To: <d30c122c-297e-3370-86d6-039388b48cea@gmail.com>

On Mon, Jul 8, 2019 at 3:06 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> 08.07.2019 15:38, Arnd Bergmann пишет:
>
> The offending patch should be reverted already, please see [1].
>
> [1] https://patchwork.ozlabs.org/patch/1128007/#2210871

Ok,  sounds good. With my patch on top, the other version is a little
nicer, but the revert is correct as well, and probably the safe choice
during the merge window.

         Arnd

^ permalink raw reply

* Re: [PATCH] gpio: tegra: fix debugfs compile error
From: Dmitry Osipenko @ 2019-07-08 13:06 UTC (permalink / raw)
  To: Arnd Bergmann, Linus Walleij, Bartosz Golaszewski, Thierry Reding,
	Jonathan Hunter
  Cc: linux-gpio, linux-tegra, linux-kernel
In-Reply-To: <20190708123843.3302581-1-arnd@arndb.de>

08.07.2019 15:38, Arnd Bergmann пишет:
> Without this header, some configurations now run into a build failure:
> 
> drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function 'debugfs_create_file'
>       [-Werror,-Wimplicit-function-declaration]
>         debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
>         ^
> drivers/gpio/gpio-tegra.c:665:2: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
> drivers/gpio/gpio-tegra.c:666:9: error: use of undeclared identifier 'tegra_dbg_gpio_fops'
> 
> Remove the #ifdef here and let the compiler drop the unused
> functions itself when debugfs_create_file() is an empty inline
> function.
> 
> Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpio/gpio-tegra.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index 59b99d8c3647..dbcecbe5f52f 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -9,6 +9,7 @@
>   *	Erik Gilling <konkers@google.com>
>   */
>  
> +#include <linux/debugfs.h>
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
> @@ -22,6 +23,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/pm.h>
> +#include <linux/seq_file.h>
>  
>  #define GPIO_BANK(x)		((x) >> 5)
>  #define GPIO_PORT(x)		(((x) >> 3) & 0x3)
> @@ -508,10 +510,6 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
>  }
>  #endif
>  
> -#ifdef CONFIG_DEBUG_FS
> -
> -#include <linux/debugfs.h>
> -#include <linux/seq_file.h>
>  
>  static int tegra_dbg_gpio_show(struct seq_file *s, void *unused)
>  {
> @@ -538,7 +536,6 @@ static int tegra_dbg_gpio_show(struct seq_file *s, void *unused)
>  }
>  
>  DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
> -#endif
>  
>  static const struct dev_pm_ops tegra_gpio_pm_ops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(tegra_gpio_suspend, tegra_gpio_resume)
> 

The offending patch should be reverted already, please see [1].

[1] https://patchwork.ozlabs.org/patch/1128007/#2210871

^ permalink raw reply

* Re: [PATCH] gpio: gpiolib-of.c: Add of_node_put() before break
From: Bartosz Golaszewski @ 2019-07-08 12:40 UTC (permalink / raw)
  To: Nishka Dasgupta; +Cc: Linus Walleij, linux-gpio
In-Reply-To: <20190706133422.3372-1-nishkadg.linux@gmail.com>

sob., 6 lip 2019 o 15:34 Nishka Dasgupta <nishkadg.linux@gmail.com> napisał(a):
>
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a break from the middle of the loop, there is no put, thus
> causing a memory leak. Hence add an of_node_put before the break.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
>  drivers/gpio/gpiolib-of.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index aec7bd86ae7e..c9325efc1783 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -154,6 +154,7 @@ static void of_gpio_flags_quirks(struct device_node *np,
>                                                         of_node_full_name(child));
>                                         *flags |= OF_GPIO_ACTIVE_LOW;
>                                 }
> +                               of_node_put(child);
>                                 break;
>                         }
>                 }
> --
> 2.19.1
>

Applied for fixes with a slightly changed commit message. Thanks!

Bartosz

^ permalink raw reply

* [PATCH] gpio: tegra: fix debugfs compile error
From: Arnd Bergmann @ 2019-07-08 12:38 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Thierry Reding,
	Jonathan Hunter
  Cc: Arnd Bergmann, Dmitry Osipenko, linux-gpio, linux-tegra,
	linux-kernel

Without this header, some configurations now run into a build failure:

drivers/gpio/gpio-tegra.c:665:2: error: implicit declaration of function 'debugfs_create_file'
      [-Werror,-Wimplicit-function-declaration]
        debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
        ^
drivers/gpio/gpio-tegra.c:665:2: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
drivers/gpio/gpio-tegra.c:666:9: error: use of undeclared identifier 'tegra_dbg_gpio_fops'

Remove the #ifdef here and let the compiler drop the unused
functions itself when debugfs_create_file() is an empty inline
function.

Fixes: a4de43049a1d ("gpio: tegra: Clean-up debugfs initialisation")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/gpio-tegra.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 59b99d8c3647..dbcecbe5f52f 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -9,6 +9,7 @@
  *	Erik Gilling <konkers@google.com>
  */
 
+#include <linux/debugfs.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/irq.h>
@@ -22,6 +23,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/pm.h>
+#include <linux/seq_file.h>
 
 #define GPIO_BANK(x)		((x) >> 5)
 #define GPIO_PORT(x)		(((x) >> 3) & 0x3)
@@ -508,10 +510,6 @@ static int tegra_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
 }
 #endif
 
-#ifdef CONFIG_DEBUG_FS
-
-#include <linux/debugfs.h>
-#include <linux/seq_file.h>
 
 static int tegra_dbg_gpio_show(struct seq_file *s, void *unused)
 {
@@ -538,7 +536,6 @@ static int tegra_dbg_gpio_show(struct seq_file *s, void *unused)
 }
 
 DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
-#endif
 
 static const struct dev_pm_ops tegra_gpio_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(tegra_gpio_suspend, tegra_gpio_resume)
-- 
2.20.0


^ permalink raw reply related

* Re: [PATCH] gpio: davinci: silence error prints in case of EPROBE_DEFER
From: Bartosz Golaszewski @ 2019-07-08 12:36 UTC (permalink / raw)
  To: Keerthy; +Cc: Linus Walleij, LKML, linux-gpio, Tero Kristo
In-Reply-To: <20190708084904.18607-1-j-keerthy@ti.com>

pon., 8 lip 2019 o 10:48 Keerthy <j-keerthy@ti.com> napisał(a):
>
> Silence error prints in case of EPROBE_DEFER. This avoids
> multiple/duplicate defer prints during boot.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/gpio/gpio-davinci.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index fc494a84a29d..e0b025689625 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -238,8 +238,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>         for (i = 0; i < nirq; i++) {
>                 chips->irqs[i] = platform_get_irq(pdev, i);
>                 if (chips->irqs[i] < 0) {
> -                       dev_info(dev, "IRQ not populated, err = %d\n",
> -                                chips->irqs[i]);
> +                       if (chips->irqs[i] != -EPROBE_DEFER)
> +                               dev_info(dev, "IRQ not populated, err = %d\n",
> +                                        chips->irqs[i]);
>                         return chips->irqs[i];
>                 }
>         }
> --
> 2.17.1
>

Applied to fixes, thanks.

Bart

^ permalink raw reply

* [RFC] SW connection between DVB Transport Stream demuxer and I2C-based frontend
From: Marc Gonzalez @ 2019-07-08 11:08 UTC (permalink / raw)
  To: I2C, linux-media, GPIO
  Cc: Mauro Carvalho Chehab, Jonathan Neuschäfer, Brad Love,
	Antti Palosaari, Olli Salonen, Bjorn Andersson, Linus Walleij,
	Jeffrey Hugo, Wolfram Sang, Simon Horman, Peter Korsgaard,
	Linux ARM

Hello everyone,

My first message(*) might have been too vague. Let me try explaining
the goal in a different way, using code this time. (Provided at the
end of this message.)

(*) https://www.spinics.net/lists/arm-kernel/msg739657.html

Background: I'm working with a SoC that provides a TSIF HW block,
and I'm trying to write a driver for it.

The purpose of this "Transport Stream Interface" is to merge incoming
bits together into a 188-byte TS packet, add a timestamp, then raise
an interrupt to notify the system that a new packet is ready.
(Driver for this HW block is tsif.c)

Pretty basic stuff so far...

While the TSIF block is embedded in the SoC, the block that feeds the
TSIF (a demodulator) is not, therefore the demod is provided on the
board. (On the board I have, demod=si2168, tuner=2141). Different boards
may have different demods/tuners.

So, I have a DT node for the TSIF in the SoC DTSI, and a DT node for
the si2168 in the board DTSI; and the TSIF node has a phandle to the
demod node. The system communicates with si2168 over an I2C bus.
The si2168 is connected to the TSIF using 3 GPIO pins.


PROBLEM #1

The media framework requires that the TSIF and demod be "tied" together,
by calling dvb_register_frontend(). If I do that in tsif.c, then I need to
get the frontend pointer from the demod at some point. There is no such
callback presently. Since si2168 lives on an I2C bus, I can get a
struct i2c_client pointer, through the DT phandle. But some kind of
abstraction is missing to query the i2c_client object to make sure it
is a demodulator and request its frontend pointer.

For the time being, I have added a very generic pointer to struct i2c_client
but I feel this is not quite right... (though it gets the job done)


PROBLEM #2

The tuner (si2157) is not on the i2c5 bus, instead it is on a private
i2c bus *behind* si2168, which routes requests to the proper client.
For the time being, I don't know how to model this relationship in DT.
(TODO: check i2c_slave_cb_t slave_cb in struct i2c_client)
I have initialized si2157 in the si2168 driver, but this doesn't feel
right. (Though it seems all(?) users pair 2168 with 2157.)


It would really help if I could get some guidance from media and i2c
people for these two issues. (I'd like to upstream the driver in time
for 5.3)

Regards.



diff --git a/arch/arm64/boot/dts/qcom/apq8098-batfish.dts b/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
index 29d59ecad138..9353e62375a7 100644
--- a/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
+++ b/arch/arm64/boot/dts/qcom/apq8098-batfish.dts
@@ -30,6 +30,28 @@
 	status = "ok";
 };
 
+&blsp1_i2c5 {
+	status = "ok";
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c5_default>;
+
+	dvb_demod: si2168@64 {
+		compatible = "silabs,si2168";
+		reg = <0x64>;
+		reset-gpios = <&tlmm 84 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&tsif {
+	demod = <&dvb_demod>;
+};
+
+&i2c5_default {
+	drive-strength = <2>;
+	bias-disable;
+};
+
 &qusb2phy {
 	status = "ok";
 	vdda-pll-supply = <&vreg_l12a_1p8>;
diff --git a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi
index 6db70acd38ee..22ef4dfddb15 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-pins.dtsi
@@ -2,6 +2,18 @@
 /* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
 
 &tlmm {
+	i2c5_default: i2c5-default {
+		pins = "gpio87", "gpio88";
+		function = "blsp_i2c5";
+	};
+
+	tsif0_default: tsif0-default {
+		pins = "gpio89", "gpio90", "gpio91";
+		function = "tsif0";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
 	sdc2_clk_on: sdc2_clk_on {
 		config {
 			pins = "sdc2_clk";
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index f8671a46392d..72b5d3e094c0 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1206,6 +1206,21 @@
 			status = "disabled";
 		};
 
+		tsif: tsif@c1e7000 {
+			compatible = "qcom,msm8998-tsif";
+			reg = <0x0c1e7000 0x200>;
+			reg-names = "MSM_TSIF0_PHYS";
+
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "TSIF0_IRQ";
+
+			clocks = <&gcc GCC_TSIF_AHB_CLK>;
+			clock-names = "iface_clk";
+
+			pinctrl-0 = <&tsif0_default>;
+			pinctrl-names = "default";
+		};
+
 		timer@17920000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 48e8a376766e..726bb6759315 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -6,7 +6,9 @@
  */
 
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 
+#include "si2157.h"
 #include "si2168_priv.h"
 
 static const struct dvb_frontend_ops si2168_ops;
@@ -660,9 +662,20 @@ static const struct dvb_frontend_ops si2168_ops = {
 	.read_status = si2168_read_status,
 };
 
+struct si2168_config si2168_config;
+struct si2157_config si2157_config;
+struct i2c_client *tuner;
+
+static void *get_fe(struct i2c_client *client)
+{
+	struct si2168_dev *dev = i2c_get_clientdata(client);
+	return &dev->fe;
+}
+
 static int si2168_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
+	struct device *cdev = &client->dev;
 	struct si2168_config *config = client->dev.platform_data;
 	struct si2168_dev *dev;
 	int ret;
@@ -670,12 +683,24 @@ static int si2168_probe(struct i2c_client *client,
 
 	dev_dbg(&client->dev, "\n");
 
+	if (!config) {
+		config = &si2168_config;
+		config->ts_mode = SI2168_TS_SERIAL;
+	}
+
+	if (cdev->of_node) {
+		struct gpio_desc *desc;
+		desc = devm_gpiod_get_optional(cdev, "reset", GPIOD_OUT_LOW);
+		if (IS_ERR(desc)) return PTR_ERR(desc);
+	}
+
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
+	client->get_something = get_fe;
 	i2c_set_clientdata(client, dev);
 	mutex_init(&dev->i2c_mutex);
 
@@ -739,8 +764,10 @@ static int si2168_probe(struct i2c_client *client,
 	/* create dvb_frontend */
 	memcpy(&dev->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
 	dev->fe.demodulator_priv = client;
-	*config->i2c_adapter = dev->muxc->adapter[0];
-	*config->fe = &dev->fe;
+	if (config->i2c_adapter)
+		*config->i2c_adapter = dev->muxc->adapter[0];
+	if (config->fe)
+		*config->fe = &dev->fe;
 	dev->ts_mode = config->ts_mode;
 	dev->ts_clock_inv = config->ts_clock_inv;
 	dev->ts_clock_gapped = config->ts_clock_gapped;
@@ -753,6 +780,12 @@ static int si2168_probe(struct i2c_client *client,
 		 dev->version >> 24 & 0xff, dev->version >> 16 & 0xff,
 		 dev->version >> 8 & 0xff, dev->version >> 0 & 0xff);
 
+	if (cdev->of_node) {
+		struct i2c_adapter *si2168_bus = dev->muxc->adapter[0];
+		si2157_config.fe = &dev->fe;
+		tuner = dvb_module_probe("si2157", "si2141", si2168_bus, 0x60, &si2157_config);
+	}
+
 	return 0;
 err_kfree:
 	kfree(dev);
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 7cbbd925124c..b24fc530e875 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -101,3 +101,8 @@ obj-y					+= meson/
 obj-y					+= cros-ec-cec/
 
 obj-$(CONFIG_VIDEO_SUN6I_CSI)		+= sunxi/sun6i-csi/
+
+obj-y += tsif.o
+ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
+ccflags-y += -I$(srctree)/drivers/media/tuners
+ccflags-y += -I$(srctree)/drivers/media/common
diff --git a/drivers/media/platform/tsif.c b/drivers/media/platform/tsif.c
new file mode 100644
index 000000000000..a0118c2ee870
--- /dev/null
+++ b/drivers/media/platform/tsif.c
@@ -0,0 +1,294 @@
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kthread.h>
+#include <linux/vmalloc.h>
+
+#include <media/dvb_demux.h>
+#include <media/dmxdev.h>
+
+#include "si2168.h"
+#include "si2157.h"
+
+#include <linux/module.h>        /* Just for modules */
+#include <linux/kernel.h>        /* Only for KERN_INFO */
+#include <linux/err.h>           /* Error macros */
+#include <linux/cdev.h>
+#include <linux/init.h>          /* Needed for the macros */
+#include <linux/io.h>            /* IO macros */
+#include <linux/device.h>        /* Device drivers need this */
+#include <linux/sched.h>         /* Externally defined globals */
+#include <linux/fs.h>
+#include <linux/uaccess.h>       /* copy_to_user */
+#include <linux/slab.h>          /* kfree, kzalloc */
+#include <linux/ioport.h>        /* XXX_ mem_region */
+#include <linux/dma-mapping.h>   /* dma_XXX */
+#include <linux/delay.h>         /* msleep */
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/poll.h>          /* poll() file op */
+#include <linux/wait.h>          /* wait() macros, sleeping */
+#include <linux/bitops.h>        /* BIT() macro */
+#include <linux/regulator/consumer.h>
+#include <linux/timer.h>         /* Timer services */
+#include <linux/jiffies.h>       /* Jiffies counter */
+#include <linux/i2c.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/string.h>
+#include <linux/interrupt.h>	/* tasklet */
+
+/* Max number of section filters */
+#define TSPP_MAX_SECTION_FILTER_NUM	128
+
+/*
+ * General defines
+ */
+#define TSPP_TSIF_INSTANCES            1
+
+/*
+ * TSIF register offsets
+ */
+#define TSIF_STS_CTL	0x0
+#define TSIF_DATA_PORT	0x100
+
+/* bits for TSIF_STS_CTL register */
+#define TSIF_STS_CTL_EN_IRQ       BIT(28)
+#define TSIF_STS_CTL_PACK_AVAIL   BIT(27)
+#define TSIF_STS_CTL_1ST_PACKET   BIT(26)
+#define TSIF_STS_CTL_OVERFLOW     BIT(25)
+#define TSIF_STS_CTL_LOST_SYNC    BIT(24)
+#define TSIF_STS_CTL_TIMEOUT      BIT(23)
+#define TSIF_STS_CTL_INV_SYNC     BIT(21)
+#define TSIF_STS_CTL_INV_NULL     BIT(20)
+#define TSIF_STS_CTL_INV_ERROR    BIT(19)
+#define TSIF_STS_CTL_INV_ENABLE   BIT(18)
+#define TSIF_STS_CTL_INV_DATA     BIT(17)
+#define TSIF_STS_CTL_INV_CLOCK    BIT(16)
+#define TSIF_STS_CTL_SPARE        BIT(15)
+#define TSIF_STS_CTL_EN_NULL      BIT(11)
+#define TSIF_STS_CTL_EN_ERROR     BIT(10)
+#define TSIF_STS_CTL_LAST_BIT     BIT(9)
+#define TSIF_STS_CTL_EN_TIME_LIM  BIT(8)
+#define TSIF_STS_CTL_EN_TCR       BIT(7)
+#define TSIF_STS_CTL_TEST_MODE    BIT(6)
+#define TSIF_STS_CTL_MODE_2       BIT(5)
+#define TSIF_STS_CTL_EN_DM        BIT(4)
+#define TSIF_STS_CTL_STOP         BIT(3)
+#define TSIF_STS_CTL_START        BIT(0)
+
+/* this represents the actual hardware device */
+struct tspp_device {
+	struct platform_device *pdev;
+	struct clk *tsif_pclk;
+};
+
+static struct batfish_dvb_adapter {
+	struct device *dev;
+} batfish_dvb_adapter;
+
+struct tsif_device {
+	void __iomem *base;
+	u32 ref_count;
+};
+
+/*** GLOBALS ***/
+/*** TODO: move them all to a properly kalloc'ed struct ***/
+static struct dvb_frontend *my_dvb_frontend;
+static struct dmx_frontend my_dmx_frontend;
+static struct dvb_adapter my_dvb_adapter;
+static struct dvb_demux my_dvb_demux;
+static struct dmxdev my_dmxdev;
+static struct tsif_device my_tsif_device;
+
+static int start_tsif(struct dvb_demux_feed *feed)
+{
+	printk("%s: feed PID=%u\n", __func__, feed->pid);
+
+	if (my_tsif_device.ref_count++ == 0) {
+		u32 val = TSIF_STS_CTL_EN_IRQ + TSIF_STS_CTL_START;
+		writel_relaxed(val, my_tsif_device.base + TSIF_STS_CTL);
+	}
+
+	return 0;
+}
+
+static int stop_tsif(struct dvb_demux_feed *feed)
+{
+	printk("%s: feed PID=%u\n", __func__, feed->pid);
+
+	if (my_tsif_device.ref_count == 0) panic(__func__);
+
+	if (--my_tsif_device.ref_count == 0)
+		writel_relaxed(TSIF_STS_CTL_STOP, my_tsif_device.base + TSIF_STS_CTL);
+
+	return 0;
+}
+
+static void batfish_init(struct batfish_dvb_adapter *adapter)
+{
+	int ret;
+	short int ids[] = { -1 };
+
+	ret = dvb_register_adapter(&my_dvb_adapter, "MSM TSPP", THIS_MODULE, adapter->dev, ids);
+	if (ret < 0) panic("dvb_register_adapter");
+
+	my_dvb_demux.dmx.capabilities = DMX_TS_FILTERING | DMX_SECTION_FILTERING;
+
+	my_dvb_demux.priv = (void *)0xdeadbeef;
+	my_dvb_demux.filternum = TSPP_MAX_SECTION_FILTER_NUM;
+	my_dvb_demux.feednum = 128;
+	my_dvb_demux.start_feed = start_tsif;
+	my_dvb_demux.stop_feed = stop_tsif;
+
+	ret = dvb_dmx_init(&my_dvb_demux);
+	if (ret < 0) panic("dvb_dmx_init");
+
+	my_dmxdev.filternum = 128;
+	my_dmxdev.demux = &my_dvb_demux.dmx;
+	my_dmxdev.capabilities = 0;
+
+	ret = dvb_dmxdev_init(&my_dmxdev, &my_dvb_adapter);
+	if (ret < 0) panic("dvb_dmxdev_init");
+
+	my_dmx_frontend.source = DMX_FRONTEND_0;
+
+	ret = my_dvb_demux.dmx.add_frontend(&my_dvb_demux.dmx, &my_dmx_frontend);
+	if (ret < 0) panic("add_frontend");
+
+	ret = my_dvb_demux.dmx.connect_frontend(&my_dvb_demux.dmx, &my_dmx_frontend);
+	if (ret < 0) panic("connect_frontend");
+
+	ret = dvb_register_frontend(&my_dvb_adapter, my_dvb_frontend);
+	if (ret < 0) panic("dvb_register_frontend");
+}
+
+static void batfish_deinit(struct batfish_dvb_adapter *adapter)
+{
+	dvb_unregister_frontend(my_dvb_frontend);
+	my_dvb_demux.dmx.remove_frontend(&my_dvb_demux.dmx, &my_dmx_frontend);
+	dvb_dmxdev_release(&my_dmxdev);
+	dvb_dmx_release(&my_dvb_demux);
+	dvb_unregister_adapter(&my_dvb_adapter);
+}
+
+static void tspp_plugin_init(struct device *dev)
+{
+	batfish_dvb_adapter.dev = dev;
+	batfish_init(&batfish_dvb_adapter);
+}
+
+static void tspp_plugin_exit(void)
+{
+	batfish_deinit(&batfish_dvb_adapter);
+}
+
+void dvb_dmx_swfilter_packets(struct dvb_demux *demux, const u8 *buf, size_t count);
+
+static irqreturn_t tsif_isr(int irq, void *dev)
+{
+	int i;
+	void __iomem *tsif = my_tsif_device.base;
+	unsigned int status = readl_relaxed(tsif + TSIF_STS_CTL);
+	u32 buf[48];
+
+	status = readl_relaxed(tsif + 0);
+	writel_relaxed(status, tsif + 0);
+
+	for (i = 0; i < 48; ++i)
+		buf[i] = readl_relaxed(tsif + TSIF_DATA_PORT);
+
+	dvb_dmx_swfilter_packets(&my_dvb_demux, (void *)buf, 1);
+
+	return IRQ_HANDLED;
+}
+
+static int msm_tspp_probe(struct platform_device *pdev)
+{
+	int rc = -ENODEV;
+	struct tspp_device *device;
+	struct resource *mem_tsif0;
+	int tsif_irq;
+
+	/*** TODO: Use devm versions everywhere ***/
+
+	/* OK, we will use this device */
+	device = kzalloc(sizeof(struct tspp_device), GFP_KERNEL);
+	if (!device) panic("kzalloc");
+
+	/* set up references */
+	device->pdev = pdev;
+	platform_set_drvdata(pdev, device);
+
+	/* map clocks */
+	device->tsif_pclk = clk_get(&pdev->dev, "iface_clk");
+	if (IS_ERR(device->tsif_pclk)) panic("clk_get iface_clk");
+
+	/* map I/O memory */
+	mem_tsif0 = platform_get_resource_byname(pdev, IORESOURCE_MEM, "MSM_TSIF0_PHYS");
+	if (!mem_tsif0) panic("get mem_tsif0");
+	my_tsif_device.base = ioremap(mem_tsif0->start, resource_size(mem_tsif0));
+	if (!my_tsif_device.base) panic("ioremap mem_tsif0");
+
+	tsif_irq = platform_get_irq_byname(pdev, "TSIF0_IRQ");
+	if (tsif_irq <= 0) panic("platform_get_irq_byname");
+	rc = devm_request_irq(&pdev->dev, tsif_irq, tsif_isr, IRQF_SHARED, "toto", device);
+	if (rc) panic("devm_request_irq");
+
+	{
+		struct device_node *tsif_node, *demod_node;
+		struct i2c_client *demod;
+
+		tsif_node = pdev->dev.of_node;
+		demod_node = of_parse_phandle(tsif_node, "demod", 0);
+
+		demod = of_find_i2c_device_by_node(demod_node);
+		if (!demod) panic("of_find_i2c_device_by_node");
+
+		/*** TODO: Improve callback naming & handling ***/
+		if (!demod->get_something)
+			panic("Wrong i2c_client");
+		my_dvb_frontend = demod->get_something(demod);
+		of_node_put(demod_node);
+	}
+
+	clk_prepare_enable(device->tsif_pclk);
+
+	tspp_plugin_init(&pdev->dev);
+
+	return 0;
+}
+
+static int msm_tspp_remove(struct platform_device *pdev)
+{
+	struct tspp_device *device = platform_get_drvdata(pdev);
+
+	tspp_plugin_exit();
+
+	iounmap(my_tsif_device.base);
+
+	if (device->tsif_pclk)
+		clk_put(device->tsif_pclk);
+
+	kfree(device);
+
+	return 0;
+}
+
+static const struct of_device_id msm_match_table[] = {
+	{ .compatible = "qcom,msm8998-tsif" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver msm_tspp_driver = {
+	.probe          = msm_tspp_probe,
+	.remove         = msm_tspp_remove,
+	.driver         = {
+		.name   = "msm_tspp",
+		.of_match_table = msm_match_table,
+	},
+};
+
+module_platform_driver(msm_tspp_driver);
+
+MODULE_DESCRIPTION("MSM TSPP DVB Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e982b8913b73..5fca596e0dd0 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -295,6 +295,8 @@ struct i2c_driver {
 };
 #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)
 
+typedef void *generic_func(struct i2c_client *this);
+
 /**
  * struct i2c_client - represent an I2C slave device
  * @flags: I2C_CLIENT_TEN indicates the device uses a ten bit chip address;
@@ -328,6 +330,7 @@ struct i2c_client {
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
 #endif
+	generic_func *get_something;
 };
 #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
 

^ permalink raw reply related

* [PATCH 0/4] gpio: hierarchical IRQ improvements
From: Brian Masney @ 2019-07-08 11:01 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, tglx, marc.zyngier, ilina, jonathanh,
	skomatineni, bbiswas, linux-tegra, david.daney, yamada.masahiro,
	treding, bjorn.andersson, agross, linux-arm-msm, linux-kernel

This builds on top of Linus Walleij's existing patches that adds
hierarchical IRQ support to the GPIO core [1] so that Qualcomm's
spmi-gpio and ssbi-gpio can be converted to use these new helpers.

Linus: Feel free to squash these into your existing patches if you'd
like to use any of this code. Just give me some kind of mention in the
commit description.

[1] https://lore.kernel.org/linux-gpio/20190624132531.6184-1-linus.walleij@linaro.org/

Brian Masney (4):
  gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell
    functions
  gpio: allow customizing hierarchical IRQ chips
  gpio: use handler in gpio_irq_chip instead of handle_bad_irq
  qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core

 drivers/gpio/gpiolib.c                   | 78 ++++++++++++++++----
 drivers/pinctrl/qcom/Kconfig             |  1 +
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 92 +++++++-----------------
 include/linux/gpio/driver.h              | 65 +++++++++++++++++
 4 files changed, 154 insertions(+), 82 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH 3/4] gpio: use handler in gpio_irq_chip instead of handle_bad_irq
From: Brian Masney @ 2019-07-08 11:01 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, tglx, marc.zyngier, ilina, jonathanh,
	skomatineni, bbiswas, linux-tegra, david.daney, yamada.masahiro,
	treding, bjorn.andersson, agross, linux-arm-msm, linux-kernel
In-Reply-To: <20190708110138.24657-1-masneyb@onstation.org>

Use the IRQ handler field that's available in the struct gpio_irq_chip
when allocating an IRQ rather than hardcoding the handler to
handle_bad_irq(). The kernel reboots without any messages when testing
this using spmi-gpio on the Nexus 5.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
I didn't have time to dig into more detail about why this is happening.
I suspect the issue is that __irq_do_set_handler() has a special check
for handle_bad_irq:

https://elixir.bootlin.com/linux/latest/source/kernel/irq/chip.c#L974

My post about this:
https://lore.kernel.org/linux-gpio/20190707014620.GA9690@onstation.org/

 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5423242deb81..bc68ebb8f40e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1872,7 +1872,7 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 				    hwirq + i,
 				    gc->irq.chip,
 				    gc,
-				    handle_bad_irq,
+				    girq->handler,
 				    NULL, NULL);
 		irq_set_probe(irq + i);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/4] gpio: allow customizing hierarchical IRQ chips
From: Brian Masney @ 2019-07-08 11:01 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, tglx, marc.zyngier, ilina, jonathanh,
	skomatineni, bbiswas, linux-tegra, david.daney, yamada.masahiro,
	treding, bjorn.andersson, agross, linux-arm-msm, linux-kernel
In-Reply-To: <20190708110138.24657-1-masneyb@onstation.org>

Now that the GPIO core has support for hierarchical IRQ chips, let's add
support for three new callbacks in struct gpio_irq_chip:

populate_parent_fwspec:
    This optional callback populates the struct irq_fwspec for the
    parent's IRQ domain. If this is not specified, then
    gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
    variant named &gpiochip_populate_parent_fwspec_twocell is also
    available.

child_pin_to_irq:
    This optional callback is used to translate the child's GPIO pin
    number to an IRQ number for the GPIO to_irq() callback. If this is
    not specified, then a default callback will be provided that
    returns the pin number.

child_irq_domain_ops:
    The IRQ domain operations that will be used for this GPIO IRQ
    chip. If no operations are provided, then default callbacks will
    be populated to setup the IRQ hierarchy. Some drivers need to
    supply their own translate function.

These will be initially used by Qualcomm's spmi-gpio and ssbi-gpio.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
Note: checkpatch doesn't like that child_irq_domain_ops is not const.

 drivers/gpio/gpiolib.c      | 52 +++++++++++++++++++++++++++----------
 include/linux/gpio/driver.h | 35 +++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 06c9cf714c99..5423242deb81 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1778,7 +1778,7 @@ static void gpiochip_set_hierarchical_irqchip(struct gpio_chip *gc,
 
 			fwspec.fwnode = gc->irq.fwnode;
 			/* This is the hwirq for the GPIO line side of things */
-			fwspec.param[0] = i;
+			fwspec.param[0] = girq->child_pin_to_irq(gc, i);
 			/* Just pick something */
 			fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
 			fwspec.param_count = 2;
@@ -1841,7 +1841,7 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 
 	chip_info(gc, "called %s\n", __func__);
 
-	ret = gpiochip_hierarchy_irq_domain_translate(d, fwspec, &hwirq, &type);
+	ret = gc->irq.child_irq_domain_ops.translate(d, fwspec, &hwirq, &type);
 	if (ret)
 		return ret;
 
@@ -1882,10 +1882,9 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 		 * all together up the chain.
 		 */
 		parent_fwspec.fwnode = d->parent->fwnode;
-		parent_fwspec.param_count = 2;
-		parent_fwspec.param[0] = parent_hwirq;
 		/* This parent only handles asserted level IRQs */
-		parent_fwspec.param[1] = parent_type;
+		girq->populate_parent_fwspec(gc, &parent_fwspec, parent_hwirq,
+					     parent_type);
 		chip_info(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
 			  irq + i, parent_hwirq);
 		ret = irq_domain_alloc_irqs_parent(d, irq + i, 1,
@@ -1899,13 +1898,29 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 	return 0;
 }
 
-static const struct irq_domain_ops gpiochip_hierarchy_domain_ops = {
-	.activate = gpiochip_irq_domain_activate,
-	.deactivate = gpiochip_irq_domain_deactivate,
-	.translate = gpiochip_hierarchy_irq_domain_translate,
-	.alloc = gpiochip_hierarchy_irq_domain_alloc,
-	.free = irq_domain_free_irqs_common,
-};
+static unsigned int gpiochip_child_pin_to_irq_noop(struct gpio_chip *chip,
+						   unsigned int pin)
+{
+	return pin;
+}
+
+static void gpiochip_add_default_irq_domain_ops(struct irq_domain_ops *ops)
+{
+	if (!ops->activate)
+		ops->activate = gpiochip_irq_domain_activate;
+
+	if (!ops->deactivate)
+		ops->deactivate = gpiochip_irq_domain_deactivate;
+
+	if (!ops->translate)
+		ops->translate = gpiochip_hierarchy_irq_domain_translate;
+
+	if (!ops->alloc)
+		ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
+
+	if (!ops->free)
+		ops->free = irq_domain_free_irqs_common;
+}
 
 static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
 {
@@ -1921,12 +1936,21 @@ static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
 		return -EINVAL;
 	}
 
+	if (!gc->irq.child_pin_to_irq)
+		gc->irq.child_pin_to_irq = gpiochip_child_pin_to_irq_noop;
+
+	if (!gc->irq.populate_parent_fwspec)
+		gc->irq.populate_parent_fwspec =
+			gpiochip_populate_parent_fwspec_twocell;
+
+	gpiochip_add_default_irq_domain_ops(&gc->irq.child_irq_domain_ops);
+
 	gc->irq.domain = irq_domain_create_hierarchy(
 		gc->irq.parent_domain,
 		IRQ_DOMAIN_FLAG_HIERARCHY,
 		gc->ngpio,
 		gc->irq.fwnode,
-		&gpiochip_hierarchy_domain_ops,
+		&gc->irq.child_irq_domain_ops,
 		gc);
 
 	if (!gc->irq.domain) {
@@ -2106,7 +2130,7 @@ static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
 
 		spec.fwnode = domain->fwnode;
 		spec.param_count = 2;
-		spec.param[0] = offset;
+		spec.param[0] = chip->irq.child_pin_to_irq(chip, offset);
 		spec.param[1] = IRQ_TYPE_NONE;
 
 		return irq_create_fwspec_mapping(&spec);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 6b6bca20c8f9..fb9c126dd8f0 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -93,6 +93,41 @@ struct gpio_irq_chip {
 				     unsigned int child_type,
 				     unsigned int *parent_hwirq,
 				     unsigned int *parent_type);
+
+	/**
+	 * @populate_parent_fwspec:
+	 *
+	 * This optional callback populates the &struct irq_fwspec for the
+	 * parent's IRQ domain. If this is not specified, then
+	 * &gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
+	 * variant named &gpiochip_populate_parent_fwspec_twocell is also
+	 * available.
+	 */
+	void (*populate_parent_fwspec)(struct gpio_chip *chip,
+				       struct irq_fwspec *fwspec,
+				       unsigned int parent_hwirq,
+				       unsigned int parent_type);
+
+	/**
+	 * @child_pin_to_irq:
+	 *
+	 * This optional callback is used to translate the child's GPIO pin
+	 * number to an IRQ number for the GPIO to_irq() callback. If this is
+	 * not specified, then a default callback will be provided that
+	 * returns the pin number.
+	 */
+	unsigned int (*child_pin_to_irq)(struct gpio_chip *chip,
+					 unsigned int pin);
+
+	/**
+	 * @child_irq_domain_ops:
+	 *
+	 * The IRQ domain operations that will be used for this GPIO IRQ
+	 * chip. If no operations are provided, then default callbacks will
+	 * be populated to setup the IRQ hierarchy. Some drivers need to
+	 * supply their own translate function.
+	 */
+	struct irq_domain_ops child_irq_domain_ops;
 #endif
 
 	/**
-- 
2.20.1


^ permalink raw reply related

* [PATCH 4/4] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core
From: Brian Masney @ 2019-07-08 11:01 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, tglx, marc.zyngier, ilina, jonathanh,
	skomatineni, bbiswas, linux-tegra, david.daney, yamada.masahiro,
	treding, bjorn.andersson, agross, linux-arm-msm, linux-kernel
In-Reply-To: <20190708110138.24657-1-masneyb@onstation.org>

Now that the GPIO core has support for hierarchical IRQ chips, convert
Qualcomm's spmi-gpio over to use these new helpers to reduce duplicated
code across drivers.

This change was tested on a LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/pinctrl/qcom/Kconfig             |  1 +
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 92 +++++++-----------------
 2 files changed, 26 insertions(+), 67 deletions(-)

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 27ab585a639c..9b36da702925 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -138,6 +138,7 @@ config PINCTRL_QCOM_SPMI_PMIC
        select PINMUX
        select PINCONF
        select GENERIC_PINCONF
+       select GPIOLIB_IRQCHIP
        select IRQ_DOMAIN_HIERARCHY
        help
          This is the pinctrl, pinmux, pinconf and gpiolib driver for the
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f39da87ea185..0bef149c2f16 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -170,8 +170,6 @@ struct pmic_gpio_state {
 	struct regmap	*map;
 	struct pinctrl_dev *ctrl;
 	struct gpio_chip chip;
-	struct fwnode_handle *fwnode;
-	struct irq_domain *domain;
 };
 
 static const struct pinconf_generic_params pmic_gpio_bindings[] = {
@@ -751,23 +749,6 @@ static int pmic_gpio_of_xlate(struct gpio_chip *chip,
 	return gpio_desc->args[0] - PMIC_GPIO_PHYSICAL_OFFSET;
 }
 
-static int pmic_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
-{
-	struct pmic_gpio_state *state = gpiochip_get_data(chip);
-	struct irq_fwspec fwspec;
-
-	fwspec.fwnode = state->fwnode;
-	fwspec.param_count = 2;
-	fwspec.param[0] = pin + PMIC_GPIO_PHYSICAL_OFFSET;
-	/*
-	 * Set the type to a safe value temporarily. This will be overwritten
-	 * later with the proper value by irq_set_type.
-	 */
-	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
-
-	return irq_create_fwspec_mapping(&fwspec);
-}
-
 static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 {
 	struct pmic_gpio_state *state = gpiochip_get_data(chip);
@@ -787,7 +768,6 @@ static const struct gpio_chip pmic_gpio_gpio_template = {
 	.request		= gpiochip_generic_request,
 	.free			= gpiochip_generic_free,
 	.of_xlate		= pmic_gpio_of_xlate,
-	.to_irq			= pmic_gpio_to_irq,
 	.dbg_show		= pmic_gpio_dbg_show,
 };
 
@@ -964,46 +944,24 @@ static int pmic_gpio_domain_translate(struct irq_domain *domain,
 	return 0;
 }
 
-static int pmic_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
-				  unsigned int nr_irqs, void *data)
+static unsigned int pmic_gpio_child_pin_to_irq(struct gpio_chip *chip,
+					       unsigned int pin)
 {
-	struct pmic_gpio_state *state = container_of(domain->host_data,
-						     struct pmic_gpio_state,
-						     chip);
-	struct irq_fwspec *fwspec = data;
-	struct irq_fwspec parent_fwspec;
-	irq_hw_number_t hwirq;
-	unsigned int type;
-	int ret, i;
-
-	ret = pmic_gpio_domain_translate(domain, fwspec, &hwirq, &type);
-	if (ret)
-		return ret;
-
-	for (i = 0; i < nr_irqs; i++)
-		irq_domain_set_info(domain, virq + i, hwirq + i,
-				    &pmic_gpio_irq_chip, state,
-				    handle_level_irq, NULL, NULL);
+	return pin + PMIC_GPIO_PHYSICAL_OFFSET;
+}
 
-	parent_fwspec.fwnode = domain->parent->fwnode;
-	parent_fwspec.param_count = 4;
-	parent_fwspec.param[0] = 0;
-	parent_fwspec.param[1] = hwirq + 0xc0;
-	parent_fwspec.param[2] = 0;
-	parent_fwspec.param[3] = fwspec->param[1];
+static int pmic_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
+					   unsigned int child_hwirq,
+					   unsigned int child_type,
+					   unsigned int *parent_hwirq,
+					   unsigned int *parent_type)
+{
+	*parent_hwirq = child_hwirq + 0xc0;
+	*parent_type = child_type;
 
-	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
-					    &parent_fwspec);
+	return 0;
 }
 
-static const struct irq_domain_ops pmic_gpio_domain_ops = {
-	.activate = gpiochip_irq_domain_activate,
-	.alloc = pmic_gpio_domain_alloc,
-	.deactivate = gpiochip_irq_domain_deactivate,
-	.free = irq_domain_free_irqs_common,
-	.translate = pmic_gpio_domain_translate,
-};
-
 static int pmic_gpio_probe(struct platform_device *pdev)
 {
 	struct irq_domain *parent_domain;
@@ -1013,6 +971,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	struct pinctrl_desc *pctrldesc;
 	struct pmic_gpio_pad *pad, *pads;
 	struct pmic_gpio_state *state;
+	struct gpio_irq_chip *girq;
 	int ret, npins, i;
 	u32 reg;
 
@@ -1092,19 +1051,21 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	if (!parent_domain)
 		return -ENXIO;
 
-	state->fwnode = of_node_to_fwnode(state->dev->of_node);
-	state->domain = irq_domain_create_hierarchy(parent_domain, 0,
-						    state->chip.ngpio,
-						    state->fwnode,
-						    &pmic_gpio_domain_ops,
-						    &state->chip);
-	if (!state->domain)
-		return -ENODEV;
+	girq = &state->chip.irq;
+	girq->chip = &pmic_gpio_irq_chip;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_level_irq;
+	girq->fwnode = of_node_to_fwnode(state->dev->of_node);
+	girq->parent_domain = parent_domain;
+	girq->child_to_parent_hwirq = pmic_gpio_child_to_parent_hwirq;
+	girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
+	girq->child_pin_to_irq = pmic_gpio_child_pin_to_irq;
+	girq->child_irq_domain_ops.translate = pmic_gpio_domain_translate;
 
 	ret = gpiochip_add_data(&state->chip, state);
 	if (ret) {
 		dev_err(state->dev, "can't add gpio chip\n");
-		goto err_chip_add_data;
+		return ret;
 	}
 
 	/*
@@ -1130,8 +1091,6 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 
 err_range:
 	gpiochip_remove(&state->chip);
-err_chip_add_data:
-	irq_domain_remove(state->domain);
 	return ret;
 }
 
@@ -1140,7 +1099,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
 	struct pmic_gpio_state *state = platform_get_drvdata(pdev);
 
 	gpiochip_remove(&state->chip);
-	irq_domain_remove(state->domain);
 	return 0;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/4] gpio: introduce gpiochip_populate_parent_fwspec_{two,four}cell functions
From: Brian Masney @ 2019-07-08 11:01 UTC (permalink / raw)
  To: linus.walleij
  Cc: linux-gpio, bgolaszewski, tglx, marc.zyngier, ilina, jonathanh,
	skomatineni, bbiswas, linux-tegra, david.daney, yamada.masahiro,
	treding, bjorn.andersson, agross, linux-arm-msm, linux-kernel
In-Reply-To: <20190708110138.24657-1-masneyb@onstation.org>

Introduce the functions gpiochip_populate_parent_fwspec_{two,four}cell
that will be used for hierarchical IRQ support in GPIO drivers.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/gpio/gpiolib.c      | 24 ++++++++++++++++++++++++
 include/linux/gpio/driver.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d767e111694d..06c9cf714c99 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1946,6 +1946,30 @@ static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
 	return !!gc->irq.parent_domain;
 }
 
+void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+					     struct irq_fwspec *fwspec,
+					     unsigned int parent_hwirq,
+					     unsigned int parent_type)
+{
+	fwspec->param_count = 2;
+	fwspec->param[0] = parent_hwirq;
+	fwspec->param[1] = parent_type;
+}
+EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_twocell);
+
+void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+					      struct irq_fwspec *fwspec,
+					      unsigned int parent_hwirq,
+					      unsigned int parent_type)
+{
+	fwspec->param_count = 4;
+	fwspec->param[0] = 0;
+	fwspec->param[1] = parent_hwirq;
+	fwspec->param[2] = 0;
+	fwspec->param[3] = parent_type;
+}
+EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_fourcell);
+
 #else
 
 static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 682ad09e29f4..6b6bca20c8f9 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -493,6 +493,36 @@ struct bgpio_pdata {
 	int ngpio;
 };
 
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+
+void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+					     struct irq_fwspec *fwspec,
+					     unsigned int parent_hwirq,
+					     unsigned int parent_type);
+void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+					      struct irq_fwspec *fwspec,
+					      unsigned int parent_hwirq,
+					      unsigned int parent_type);
+
+#else
+
+static void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
+						    struct irq_fwspec *fwspec,
+						    unsigned int parent_hwirq,
+						    unsigned int parent_type)
+{
+}
+
+static void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
+						     struct irq_fwspec *fwspec,
+						     unsigned int parent_hwirq,
+						     unsigned int parent_type)
+{
+}
+
+#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
+
+
 #if IS_ENABLED(CONFIG_GPIO_GENERIC)
 
 int bgpio_init(struct gpio_chip *gc, struct device *dev,
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] platform/x86: Fix PCENGINES_APU2 Kconfig warning
From: Enrico Weigelt, metux IT consult @ 2019-07-08 10:43 UTC (permalink / raw)
  To: YueHaibing, dvhart, andy, linus.walleij, rdunlap, info
  Cc: linux-kernel, platform-driver-x86, linux-gpio
In-Reply-To: <20190704062725.50400-1-yuehaibing@huawei.com>

On 04.07.19 08:27, YueHaibing wrote:
> Fix Kconfig warning for PCENGINES_APU2 symbol:
> 
> WARNING: unmet direct dependencies detected for GPIO_AMD_FCH
>   Depends on [n]: GPIOLIB [=n] && HAS_IOMEM [=y]
>   Selected by [y]:
>   - PCENGINES_APU2 [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && INPUT [=y] && INPUT_KEYBOARD [=y] && LEDS_CLASS [=y]
> 
> WARNING: unmet direct dependencies detected for KEYBOARD_GPIO_POLLED
>   Depends on [n]: !UML && INPUT [=y] && INPUT_KEYBOARD [=y] && GPIOLIB [=n]
>   Selected by [y]:
>   - PCENGINES_APU2 [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && INPUT [=y] && INPUT_KEYBOARD [=y] && LEDS_CLASS [=y]
> 
> Add GPIOLIB dependency to fix it.

hmm, I'm not really happy w/ the kernel config system at that point:

If the select on the gpio driver would just subsequently enable gpiolib,
everything would be fine. But that contradicts how subsystems are
currently handled - you first have to enable gpio subsystem before
choosing anything that depends on it :(

Could it make sense to refactor gpiolib in a way that pieces directly
needed by gpio consumers or drivers (hmm, perhaps have separate
dependency symbols for consumer vs driver) can be selected directly,
even if the big gpio subsystem knob is disabled ? (but the other things
like userland interfaces would remain disabled) ?

OTOH, for this particular patch:

Ack-By: Enrico Weigelt <info@metux.net>


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply

* Re: [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Geert Uytterhoeven @ 2019-07-08 10:24 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Geert Uytterhoeven, Linus Walleij, Alexander Graf, Peter Maydell,
	Paolo Bonzini, Magnus Damm, linux-gpio, QEMU Developers,
	Linux-Renesas, LKML
In-Reply-To: <CAMpxmJXOrDLdw6ZPBHxzsDRYiLmhRNCb-s_Z=Gu=Ecg1XA5ONQ@mail.gmail.com>

Hi Bartosz,

On Mon, Jul 8, 2019 at 11:45 AM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> pt., 5 lip 2019 o 18:05 Geert Uytterhoeven <geert+renesas@glider.be> napisał(a):
> > GPIO controllers are exported to userspace using /dev/gpiochip*
> > character devices.  Access control to these devices is provided by
> > standard UNIX file system permissions, on an all-or-nothing basis:
> > either a GPIO controller is accessible for a user, or it is not.
> > Currently no mechanism exists to control access to individual GPIOs.
> >
> > Hence add a virtual GPIO driver to aggregate existing GPIOs (up to 32),
> > and expose them as a new gpiochip.  This is useful for implementing
> > access control, and assigning a set of GPIOs to a specific user.
> > Furthermore, it would simplify and harden exporting GPIOs to a virtual
> > machine, as the VM can just grab the full virtual GPIO controller, and
> > no longer needs to care about which GPIOs to grab and which not,
> > reducing the attack surface.
> >
> > Virtual GPIO controllers are instantiated by writing to the "new_device"
> > attribute file in sysfs:
> >
> >     $ echo "<gpiochipA> <gpioA1> [<gpioA2> ...]"
> >            "[, <gpiochipB> <gpioB1> [<gpioB2> ...]] ...]"
> >             > /sys/bus/platform/drivers/gpio-virt-agg/new_device
> >
> > Likewise, virtual GPIO controllers can be destroyed after use:
> >
> >     $ echo gpio-virt-agg.<N> \
> >             > /sys/bus/platform/drivers/gpio-virt-agg/delete_device
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> I like the general idea and the interface looks mostly fine. Since
> this is new ABI I think it needs to be documented as well.

Sure.

> I'm having trouble building this module:
>
>   CALL    scripts/atomic/check-atomics.sh
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   Kernel: arch/arm/boot/Image is ready
>   Building modules, stage 2.
>   MODPOST 235 modules
> ERROR: "gpiod_request" [drivers/gpio/gpio-virt-agg.ko] undefined!
> ERROR: "gpiochip_get_desc" [drivers/gpio/gpio-virt-agg.ko] undefined!
> ERROR: "gpiod_free" [drivers/gpio/gpio-virt-agg.ko] undefined!
> scripts/Makefile.modpost:91: recipe for target '__modpost' failed
> make[1]: *** [__modpost] Error 1
> Makefile:1287: recipe for target 'modules' failed
> make: *** [modules] Error 2
> make: *** Waiting for unfinished jobs....
>
> I'm not sure what the problem is.

Oops. As this is an RFC, I didn't bother trying to build this driver as
a module, only builtin.  Apparently the 3 symbols above are not yet
exported using EXPORT_SYMBOL_GPL().

> > --- /dev/null
> > +++ b/drivers/gpio/gpio-virt-agg.c
> > @@ -0,0 +1,390 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// GPIO Virtual Aggregator
> > +//
> > +// Copyright (C) 2019 Glider bvba
> > +
> > +#include <linux/gpio/driver.h>
> > +#include <linux/idr.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "gpiolib.h"
> > +
> > +#define DRV_NAME       "gpio-virt-agg"
> > +#define MAX_GPIOS      32
>
> Do we really need this limit? I see it simplifies the code, but maybe
> we can allocate the relevant arrays dynamically and not limit users?

Sure. That limit can be lifted.

> > +static int gpio_virt_agg_set_config(struct gpio_chip *chip,
> > +                                   unsigned int offset, unsigned long config)
> > +{
> > +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> > +
> > +       chip = priv->desc[offset]->gdev->chip;
> > +       if (chip->set_config)
> > +               return chip->set_config(chip, offset, config);
> > +
> > +       // FIXME gpiod_set_transitory() expects success if not implemented

BTW, do you have a comment about this FIXME?

> > +       return -ENOTSUPP;
> > +}

> > +static int gpio_virt_agg_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       const char *param = dev_get_platdata(dev);
> > +       struct gpio_virt_agg_priv *priv;
> > +       const char *label = NULL;
> > +       struct gpio_chip *chip;
> > +       struct gpio_desc *desc;
> > +       unsigned int offset;
> > +       int error, i;
>
> This 'i' here is reported as possibly not initialized:
>
> drivers/gpio/gpio-virt-agg.c: In function ‘gpio_virt_agg_probe’:
> drivers/gpio/gpio-virt-agg.c:230:13: warning: ‘i’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>   int error, i;
>              ^

Oops, should be preinitialized to zero. WIll fix.

> > +static int gpio_virt_agg_remove(struct platform_device *pdev)
> > +{
> > +       struct gpio_virt_agg_priv *priv = platform_get_drvdata(pdev);
> > +       unsigned int i;
> > +
> > +       gpiochip_remove(&priv->chip);
> > +
> > +       for (i = 0; i < priv->chip.ngpio; i++)
> > +               gpiod_free(priv->desc[i]);

Perhaps I should use gpiod_put() instead, which is exported to modules?

> > +
> > +       return 0;
> > +}
>
> You shouldn't need this function at all. It's up to users to free descriptors.

This frees the upstream descriptors, not the descriptors used by users
of the virtual gpiochip. Shouldn't they be freed, as they are no longer
in use?

Note that .probe() doesn't use devm_gpiochip_add_data(), as the upstream
descriptors need to be freed after the call to gpiochip_remove().

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] gpio: don't WARN() on NULL descs if gpiolib is disabled
From: Claus H. Stovgaard @ 2019-07-08 10:18 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski
In-Reply-To: <20190708082343.30726-1-brgl@bgdev.pl>

On Mon, 2019-07-08 at 10:23 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> If gpiolib is disabled, we use the inline stubs from gpio/consumer.h
> instead of regular definitions of GPIO API. The stubs for 'optional'
> variants of gpiod_get routines return NULL in this case as if the
> relevant GPIO wasn't found. This is correct so far.
> 
> Calling other (non-gpio_get) stubs from this header triggers a
> warning
> because the GPIO descriptor couldn't have been requested. The warning
> however is unconditional (WARN_ON(1)) and is emitted even if the
> passed
> descriptor pointer is NULL.
> 
> We don't want to force the users of 'optional' gpio_get to check the
> returned pointer before calling e.g. gpiod_set_value() so let's only
> WARN on non-NULL descriptors.
> 
> Reported-by: Claus H. Stovgaard <cst@phaseone.com>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  include/linux/gpio/consumer.h | 64 +++++++++++++++++--------------
> ----
>  1 file changed, 32 insertions(+), 32 deletions(-)

Have tested it on my setup (ZynqMP with AT24 EEPROM), where it works
great. This have removed the warnings in the log regarding settting
wp_gpio for AT24.

Thanks
Claus

^ permalink raw reply

* Re: [PATCH RFC] gpio: Add Virtual Aggregator GPIO Driver
From: Bartosz Golaszewski @ 2019-07-08  9:44 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Walleij, Alexander Graf, Peter Maydell, Paolo Bonzini,
	Magnus Damm, linux-gpio, qemu-devel, Linux-Renesas, LKML
In-Reply-To: <20190705160536.12047-1-geert+renesas@glider.be>

pt., 5 lip 2019 o 18:05 Geert Uytterhoeven <geert+renesas@glider.be> napisał(a):
>
> GPIO controllers are exported to userspace using /dev/gpiochip*
> character devices.  Access control to these devices is provided by
> standard UNIX file system permissions, on an all-or-nothing basis:
> either a GPIO controller is accessible for a user, or it is not.
> Currently no mechanism exists to control access to individual GPIOs.
>
> Hence add a virtual GPIO driver to aggregate existing GPIOs (up to 32),
> and expose them as a new gpiochip.  This is useful for implementing
> access control, and assigning a set of GPIOs to a specific user.
> Furthermore, it would simplify and harden exporting GPIOs to a virtual
> machine, as the VM can just grab the full virtual GPIO controller, and
> no longer needs to care about which GPIOs to grab and which not,
> reducing the attack surface.
>
> Virtual GPIO controllers are instantiated by writing to the "new_device"
> attribute file in sysfs:
>
>     $ echo "<gpiochipA> <gpioA1> [<gpioA2> ...]"
>            "[, <gpiochipB> <gpioB1> [<gpioB2> ...]] ...]"
>             > /sys/bus/platform/drivers/gpio-virt-agg/new_device
>
> Likewise, virtual GPIO controllers can be destroyed after use:
>
>     $ echo gpio-virt-agg.<N> \
>             > /sys/bus/platform/drivers/gpio-virt-agg/delete_device
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Aggregating GPIOs and exposing them as a new gpiochip was suggested in
> response to my proof-of-concept for GPIO virtualization with QEMU[1][2].
>
> Sample session on r8a7791/koelsch:
>
>   - Disable the leds node in arch/arm/boot/dts/r8a7791-koelsch.dts
>
>   - Create virtual aggregators:
>
>     $ echo "e6052000.gpio 19 20" \
>             > /sys/bus/platform/drivers/gpio-virt-agg/new_device
>
>     gpio-virt-agg gpio-virt-agg.0: GPIO 0 => e6052000.gpio/19
>     gpio-virt-agg gpio-virt-agg.0: GPIO 1 => e6052000.gpio/20
>     gpiochip_find_base: found new base at 778
>     gpio gpiochip8: (gpio-virt-agg.0): added GPIO chardev (254:8)
>     gpiochip_setup_dev: registered GPIOs 778 to 779 on device: gpiochip8 (gpio-virt-agg.0)
>
>     $ echo "e6052000.gpio 21, e6050000.gpio 20 21 22" \
>             > /sys/bus/platform/drivers/gpio-virt-agg/new_device
>
>     gpio-virt-agg gpio-virt-agg.1: GPIO 0 => e6052000.gpio/21
>     gpio-virt-agg gpio-virt-agg.1: GPIO 1 => e6050000.gpio/20
>     gpio-virt-agg gpio-virt-agg.1: GPIO 2 => e6050000.gpio/21
>     gpio-virt-agg gpio-virt-agg.1: GPIO 3 => e6050000.gpio/22
>     gpiochip_find_base: found new base at 774
>     gpio gpiochip9: (gpio-virt-agg.1): added GPIO chardev (254:9)
>     gpiochip_setup_dev: registered GPIOs 774 to 777 on device: gpiochip9 (gpio-virt-agg.1)
>
>   - Adjust permissions on /dev/gpiochip[89] (optional)
>
>   - Control LEDs:
>
>     $ gpioset gpiochip8 0=0 1=1 # LED6 OFF, LED7 ON
>     $ gpioset gpiochip8 0=1 1=0 # LED6 ON, LED7 OFF
>     $ gpioset gpiochip9 0=0     # LED8 OFF
>     $ gpioset gpiochip9 0=1     # LED8 ON
>
>   - Destroy virtual aggregators:
>
>     $ echo gpio-virt-agg.0 \
>             > /sys/bus/platform/drivers/gpio-virt-agg/delete_device
>     $ echo gpio-virt-agg.1 \
>             > /sys/bus/platform/drivers/gpio-virt-agg/delete_device
>
> Thanks for your comments!
>

Hi Geert,

I like the general idea and the interface looks mostly fine. Since
this is new ABI I think it needs to be documented as well.

I'm having trouble building this module:

  CALL    scripts/atomic/check-atomics.sh
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  Kernel: arch/arm/boot/Image is ready
  Building modules, stage 2.
  MODPOST 235 modules
ERROR: "gpiod_request" [drivers/gpio/gpio-virt-agg.ko] undefined!
ERROR: "gpiochip_get_desc" [drivers/gpio/gpio-virt-agg.ko] undefined!
ERROR: "gpiod_free" [drivers/gpio/gpio-virt-agg.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....

I'm not sure what the problem is.

> References:
>   - [1] "[PATCH QEMU POC] Add a GPIO backend"
>         (https://lore.kernel.org/linux-renesas-soc/20181003152521.23144-1-geert+renesas@glider.be/)
>   - [2] "Getting To Blinky: Virt Edition / Making device pass-through
>          work on embedded ARM"
>         (https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)
> ---
>  drivers/gpio/Kconfig         |   9 +
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/gpio-virt-agg.c | 390 +++++++++++++++++++++++++++++++++++
>  3 files changed, 400 insertions(+)
>  create mode 100644 drivers/gpio/gpio-virt-agg.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index f1f02dac324e52b6..8aff4d9626dee110 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1475,3 +1475,12 @@ config GPIO_MOCKUP
>           it.
>
>  endif
> +
> +config GPIO_VIRT_AGG
> +       tristate "GPIO Virtual Aggregator"
> +       depends on GPIOLIB
> +       help
> +         This enabled the GPIO Virtual Aggregator, which provides a way to
> +         aggregate existing GPIOs into a new virtual GPIO device.
> +         This is useful for assigning a collection of GPIOs to a user, or
> +         exported them to a virtual machine.
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 0a494052c1e845ee..32e885b7f3aa4eee 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -152,6 +152,7 @@ obj-$(CONFIG_GPIO_UCB1400)          += gpio-ucb1400.o
>  obj-$(CONFIG_GPIO_UNIPHIER)            += gpio-uniphier.o
>  obj-$(CONFIG_GPIO_VF610)               += gpio-vf610.o
>  obj-$(CONFIG_GPIO_VIPERBOARD)          += gpio-viperboard.o
> +obj-$(CONFIG_GPIO_VIRT_AGG)            += gpio-virt-agg.o
>  obj-$(CONFIG_GPIO_VR41XX)              += gpio-vr41xx.o
>  obj-$(CONFIG_GPIO_VX855)               += gpio-vx855.o
>  obj-$(CONFIG_GPIO_WHISKEY_COVE)                += gpio-wcove.o
> diff --git a/drivers/gpio/gpio-virt-agg.c b/drivers/gpio/gpio-virt-agg.c
> new file mode 100644
> index 0000000000000000..20e5f22beed9d385
> --- /dev/null
> +++ b/drivers/gpio/gpio-virt-agg.c
> @@ -0,0 +1,390 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// GPIO Virtual Aggregator
> +//
> +// Copyright (C) 2019 Glider bvba
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/idr.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +
> +#include "gpiolib.h"
> +
> +#define DRV_NAME       "gpio-virt-agg"
> +#define MAX_GPIOS      32

Do we really need this limit? I see it simplifies the code, but maybe
we can allocate the relevant arrays dynamically and not limit users?

> +
> +struct gpio_virt_agg_entry {
> +       struct platform_device *pdev;
> +};
> +
> +struct gpio_virt_agg_priv {
> +       struct gpio_chip chip;
> +       struct gpio_desc *desc[MAX_GPIOS];
> +};
> +
> +static DEFINE_MUTEX(gpio_virt_agg_lock);       /* protects idr */
> +static DEFINE_IDR(gpio_virt_agg_idr);
> +
> +static int gpio_virt_agg_get_direction(struct gpio_chip *chip,
> +                                      unsigned int offset)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +
> +       return gpiod_get_direction(priv->desc[offset]);
> +}
> +
> +static int gpio_virt_agg_direction_input(struct gpio_chip *chip,
> +                                        unsigned int offset)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +
> +       return gpiod_direction_input(priv->desc[offset]);
> +}
> +
> +static int gpio_virt_agg_direction_output(struct gpio_chip *chip,
> +                                         unsigned int offset, int value)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +
> +       return gpiod_direction_output(priv->desc[offset], value);
> +}
> +
> +static int gpio_virt_agg_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +
> +       return gpiod_get_value(priv->desc[offset]);
> +}
> +
> +static int gpio_virt_agg_get_multiple(struct gpio_chip *chip,
> +                                     unsigned long *mask, unsigned long *bits)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +       DECLARE_BITMAP(values, MAX_GPIOS) = { 0 };
> +       struct gpio_desc *desc[MAX_GPIOS];
> +       unsigned int i, j = 0;
> +       int ret;
> +
> +       for_each_set_bit(i, mask, priv->chip.ngpio)
> +               desc[j++] = priv->desc[i];
> +
> +       ret = gpiod_get_array_value(j, desc, NULL, values);
> +       if (ret)
> +               return ret;
> +
> +       for_each_set_bit(i, mask, priv->chip.ngpio)
> +               __assign_bit(i, bits, test_bit(j++, values));
> +
> +       return 0;
> +}
> +
> +static void gpio_virt_agg_set(struct gpio_chip *chip, unsigned int offset,
> +                             int value)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +
> +       gpiod_set_value(priv->desc[offset], value);
> +}
> +
> +static void gpio_virt_agg_set_multiple(struct gpio_chip *chip,
> +                                      unsigned long *mask,
> +                                      unsigned long *bits)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +       DECLARE_BITMAP(values, MAX_GPIOS);
> +       struct gpio_desc *desc[MAX_GPIOS];
> +       unsigned int i, j = 0;
> +
> +       for_each_set_bit(i, mask, priv->chip.ngpio) {
> +               __assign_bit(j, values, test_bit(i, bits));
> +               desc[j++] = priv->desc[i];
> +       }
> +
> +       gpiod_set_array_value(j, desc, NULL, values);
> +}
> +
> +static int gpio_virt_agg_set_config(struct gpio_chip *chip,
> +                                   unsigned int offset, unsigned long config)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +
> +       chip = priv->desc[offset]->gdev->chip;
> +       if (chip->set_config)
> +               return chip->set_config(chip, offset, config);
> +
> +       // FIXME gpiod_set_transitory() expects success if not implemented
> +       return -ENOTSUPP;
> +}
> +
> +static int gpio_virt_agg_init_valid_mask(struct gpio_chip *chip)
> +{
> +       struct gpio_virt_agg_priv *priv = gpiochip_get_data(chip);
> +       unsigned int i;
> +
> +       for (i = 0; i < priv->chip.ngpio; i++) {
> +               if (gpiochip_line_is_valid(priv->desc[i]->gdev->chip,
> +                                          gpio_chip_hwgpio(priv->desc[i])))
> +                       set_bit(i, chip->valid_mask);
> +       }
> +
> +       return 0;
> +}
> +
> +static int gpiochip_match_label(struct gpio_chip *chip, void *data)
> +{
> +       return !strcmp(chip->label, data);
> +}
> +
> +static struct gpio_chip *gpiochip_find_by_label(const char *label)
> +{
> +       return gpiochip_find((void *)label, gpiochip_match_label);
> +}
> +
> +static ssize_t new_device_store(struct device_driver *driver, const char *buf,
> +                               size_t count)
> +{
> +       struct gpio_virt_agg_entry *gva;
> +       struct platform_device *pdev;
> +       int res, id;
> +
> +       gva = kzalloc(sizeof(*gva), GFP_KERNEL);
> +       if (!gva)
> +               return -ENOMEM;
> +
> +       mutex_lock(&gpio_virt_agg_lock);
> +       id = idr_alloc(&gpio_virt_agg_idr, gva, 0, 0, GFP_KERNEL);
> +       mutex_unlock(&gpio_virt_agg_lock);
> +
> +       if (id < 0) {
> +               res = id;
> +               goto free_gva;
> +       }
> +
> +       /* kernfs guarantees string termination, so count + 1 is safe */
> +       pdev = platform_device_register_data(NULL, DRV_NAME, id, buf,
> +                                            count + 1);
> +       if (IS_ERR(pdev)) {
> +               res = PTR_ERR(pdev);
> +               goto remove_idr;
> +       }
> +
> +       gva->pdev = pdev;
> +       return count;
> +
> +remove_idr:
> +       mutex_lock(&gpio_virt_agg_lock);
> +       idr_remove(&gpio_virt_agg_idr, id);
> +       mutex_unlock(&gpio_virt_agg_lock);
> +free_gva:
> +       kfree(gva);
> +       return res;
> +}
> +
> +static DRIVER_ATTR_WO(new_device);
> +
> +static ssize_t delete_device_store(struct device_driver *driver,
> +                                  const char *buf, size_t count)
> +{
> +       struct gpio_virt_agg_entry *gva;
> +       int id;
> +
> +       if (strncmp(buf, DRV_NAME ".", strlen(DRV_NAME ".")))
> +               return -EINVAL;
> +
> +       id = simple_strtoul(buf + strlen(DRV_NAME "."), NULL, 10);
> +
> +       mutex_lock(&gpio_virt_agg_lock);
> +       gva = idr_remove(&gpio_virt_agg_idr, id);
> +       mutex_unlock(&gpio_virt_agg_lock);
> +
> +       if (!gva) {
> +               pr_info("Cannot find %s.%d\n", DRV_NAME, id);
> +               return -ENOENT;
> +       }
> +
> +       platform_device_unregister(gva->pdev);
> +       kfree(gva);
> +       return count;
> +}
> +static DRIVER_ATTR_WO(delete_device);
> +
> +static struct attribute *gpio_virt_agg_attrs[] = {
> +       &driver_attr_new_device.attr,
> +       &driver_attr_delete_device.attr,
> +       NULL,
> +};
> +ATTRIBUTE_GROUPS(gpio_virt_agg);
> +
> +static int gpio_virt_agg_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       const char *param = dev_get_platdata(dev);
> +       struct gpio_virt_agg_priv *priv;
> +       const char *label = NULL;
> +       struct gpio_chip *chip;
> +       struct gpio_desc *desc;
> +       unsigned int offset;
> +       int error, i;

This 'i' here is reported as possibly not initialized:

drivers/gpio/gpio-virt-agg.c: In function ‘gpio_virt_agg_probe’:
drivers/gpio/gpio-virt-agg.c:230:13: warning: ‘i’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
  int error, i;
             ^

> +       char *s;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv) {
> +               error = -ENOMEM;
> +               goto fail;
> +       }
> +
> +       for (i = 0; i < MAX_GPIOS; i++) {
> +               if (*param == '\0' || *param == '\n')
> +                       break;
> +
> +               if (*param == ',') {
> +                       if (label) {
> +                               devm_kfree(dev, label);
> +                               label = NULL;
> +                       }
> +                       for (param++; *param == ' '; param++) ;
> +               }
> +
> +               if (!label) {
> +                       s = strchr(param, ' ');
> +                       if (!s) {
> +                               dev_info(dev, "Missing gpiochip\n");
> +                               error = -EINVAL;
> +                               goto fail;
> +                       }
> +                       label = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> +                                              (int)(s - param), param);
> +                       if (!label) {
> +                               error = -ENOMEM;
> +                               goto fail;
> +                       }
> +
> +                       chip = gpiochip_find_by_label(label);
> +                       if (!chip) {
> +                               dev_info(dev, "Cannot find gpiochip %s\n",
> +                                        label);
> +                               error = -ENODEV;
> +                               goto fail;
> +                       }
> +
> +                       for (param = s + 1; *param == ' '; param++) ;
> +               }
> +
> +               offset = simple_strtoul(param, &s, 10);
> +
> +               desc = gpiochip_get_desc(chip, offset);
> +               if (IS_ERR(desc)) {
> +                       error = PTR_ERR(desc);
> +                       dev_info(dev, "Cannot get GPIO %s/%u: %d\n", label,
> +                                offset, error);
> +                       goto fail;
> +               }
> +
> +               error = gpiod_request(desc, dev_name(dev));
> +               if (error) {
> +                       dev_info(dev, "Cannot request GPIO %s/%u: %d\n", label,
> +                                offset, error);
> +                       goto fail;
> +               }
> +
> +               dev_dbg(dev, "GPIO %u => %s/%u\n", i, label, offset);
> +               priv->desc[i] = desc;
> +
> +               if (gpiod_cansleep(desc))
> +                       priv->chip.can_sleep = true;
> +               if (desc->gdev->chip->set_config)
> +                       priv->chip.set_config = gpio_virt_agg_set_config;
> +               if (desc->gdev->chip->need_valid_mask) {
> +                       priv->chip.need_valid_mask = true;
> +                       priv->chip.init_valid_mask =
> +                               gpio_virt_agg_init_valid_mask;
> +               }
> +
> +               for (param = s; *param == ' '; param++) ;
> +       }
> +       if (i == MAX_GPIOS)
> +               dev_warn(&pdev->dev,
> +                        "Too many gpios specified, truncating to %u\n",
> +                        MAX_GPIOS);
> +
> +       priv->chip.label = dev_name(dev);
> +       priv->chip.parent = dev;
> +       priv->chip.owner = THIS_MODULE;
> +       priv->chip.get_direction = gpio_virt_agg_get_direction;
> +       priv->chip.direction_input = gpio_virt_agg_direction_input;
> +       priv->chip.direction_output = gpio_virt_agg_direction_output;
> +       priv->chip.get = gpio_virt_agg_get;
> +       priv->chip.get_multiple = gpio_virt_agg_get_multiple;
> +       priv->chip.set = gpio_virt_agg_set;
> +       priv->chip.set_multiple = gpio_virt_agg_set_multiple;
> +       priv->chip.base = -1;
> +       priv->chip.ngpio = i;
> +       platform_set_drvdata(pdev, priv);
> +
> +       error = gpiochip_add_data(&priv->chip, priv);
> +       if (error)
> +               goto fail;
> +
> +       return 0;
> +
> +fail:
> +       while (i-- > 0)
> +               gpiod_free(priv->desc[i]);
> +
> +       return error;
> +}
> +
> +static int gpio_virt_agg_remove(struct platform_device *pdev)
> +{
> +       struct gpio_virt_agg_priv *priv = platform_get_drvdata(pdev);
> +       unsigned int i;
> +
> +       gpiochip_remove(&priv->chip);
> +
> +       for (i = 0; i < priv->chip.ngpio; i++)
> +               gpiod_free(priv->desc[i]);
> +
> +       return 0;
> +}

You shouldn't need this function at all. It's up to users to free descriptors.

Best regards,
Bartosz Golaszewski

> +
> +static struct platform_driver gpio_virt_agg_driver = {
> +       .probe = gpio_virt_agg_probe,
> +       .remove = gpio_virt_agg_remove,
> +       .driver = {
> +               .name = DRV_NAME,
> +               .groups = gpio_virt_agg_groups,
> +       },
> +};
> +
> +static int __init gpio_virt_agg_init(void)
> +{
> +       return platform_driver_register(&gpio_virt_agg_driver);
> +}
> +module_init(gpio_virt_agg_init);
> +
> +static int __exit gpio_virt_agg_idr_remove(int id, void *p, void *data)
> +{
> +       struct gpio_virt_agg_entry *gva = p;
> +
> +       platform_device_unregister(gva->pdev);
> +       kfree(gva);
> +       return 0;
> +}
> +
> +static void __exit gpio_virt_agg_exit(void)
> +{
> +       mutex_lock(&gpio_virt_agg_lock);
> +       idr_for_each(&gpio_virt_agg_idr, gpio_virt_agg_idr_remove, NULL);
> +       idr_destroy(&gpio_virt_agg_idr);
> +       mutex_unlock(&gpio_virt_agg_lock);
> +
> +       platform_driver_unregister(&gpio_virt_agg_driver);
> +}
> +module_exit(gpio_virt_agg_exit);
> +
> +MODULE_AUTHOR("Geert Uytterhoeven <geert+renesas@glider.be>");
> +MODULE_DESCRIPTION("GPIO Virtual Aggregator");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v2] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Bartosz Golaszewski @ 2019-07-08  8:54 UTC (permalink / raw)
  To: Michael Wu
  Cc: Linus Walleij, linux-gpio, LKML, morgan.chang, Stable # 4 . 20+
In-Reply-To: <20190708052308.27802-1-michael.wu@vatics.com>

pon., 8 lip 2019 o 07:23 Michael Wu <michael.wu@vatics.com> napisał(a):
>
> When a pin is active-low, logical trigger edge should be inverted to match
> the same interrupt opportunity.
>
> For example, a button pushed triggers falling edge in ACTIVE_HIGH case; in
> ACTIVE_LOW case, the button pushed triggers rising edge. For user space the
> IRQ requesting doesn't need to do any modification except to configuring
> GPIOHANDLE_REQUEST_ACTIVE_LOW.
>
> For example, we want to catch the event when the button is pushed. The
> button on the original board drives level to be low when it is pushed, and
> drives level to be high when it is released.
>
> In user space we can do:
>
>         req.handleflags = GPIOHANDLE_REQUEST_INPUT;
>         req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
>
>         while (1) {
>                 read(fd, &dat, sizeof(dat));
>                 if (dat.id == GPIOEVENT_EVENT_FALLING_EDGE)
>                         printf("button pushed\n");
>         }
>
> Run the same logic on another board which the polarity of the button is
> inverted; it drives level to be high when pushed, and level to be low when
> released. For this inversion we add flag GPIOHANDLE_REQUEST_ACTIVE_LOW:
>
>         req.handleflags = GPIOHANDLE_REQUEST_INPUT |
>                 GPIOHANDLE_REQUEST_ACTIVE_LOW;
>         req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;
>
> At the result, there are no any events caught when the button is pushed.
> By the way, button releasing will emit a "falling" event. The timing of
> "falling" catching is not expected.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Wu <michael.wu@vatics.com>
> ---
> Changes from v1:
> - Correct undeclared 'IRQ_TRIGGER_RISING'
> - Add an example to descibe the issue
> ---
>  drivers/gpio/gpiolib.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e013d417a936..9c9597f929d7 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
>         }
>
>         if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
> -               irqflags |= IRQF_TRIGGER_RISING;
> +               irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
> +                       IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
>         if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
> -               irqflags |= IRQF_TRIGGER_FALLING;
> +               irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
> +                       IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>         irqflags |= IRQF_ONESHOT;
>
>         INIT_KFIFO(le->events);
> --
> 2.17.1
>

Tested-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

^ permalink raw reply

* [PATCH] gpio: davinci: silence error prints in case of EPROBE_DEFER
From: Keerthy @ 2019-07-08  8:49 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski; +Cc: linux-kernel, linux-gpio, t-kristo, j-keerthy

Silence error prints in case of EPROBE_DEFER. This avoids
multiple/duplicate defer prints during boot.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/gpio/gpio-davinci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index fc494a84a29d..e0b025689625 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -238,8 +238,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 	for (i = 0; i < nirq; i++) {
 		chips->irqs[i] = platform_get_irq(pdev, i);
 		if (chips->irqs[i] < 0) {
-			dev_info(dev, "IRQ not populated, err = %d\n",
-				 chips->irqs[i]);
+			if (chips->irqs[i] != -EPROBE_DEFER)
+				dev_info(dev, "IRQ not populated, err = %d\n",
+					 chips->irqs[i]);
 			return chips->irqs[i];
 		}
 	}
-- 
2.17.1


^ permalink raw reply related

* [PATCH] gpio: don't WARN() on NULL descs if gpiolib is disabled
From: Bartosz Golaszewski @ 2019-07-08  8:23 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski,
	Claus H . Stovgaard

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

If gpiolib is disabled, we use the inline stubs from gpio/consumer.h
instead of regular definitions of GPIO API. The stubs for 'optional'
variants of gpiod_get routines return NULL in this case as if the
relevant GPIO wasn't found. This is correct so far.

Calling other (non-gpio_get) stubs from this header triggers a warning
because the GPIO descriptor couldn't have been requested. The warning
however is unconditional (WARN_ON(1)) and is emitted even if the passed
descriptor pointer is NULL.

We don't want to force the users of 'optional' gpio_get to check the
returned pointer before calling e.g. gpiod_set_value() so let's only
WARN on non-NULL descriptors.

Reported-by: Claus H. Stovgaard <cst@phaseone.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 include/linux/gpio/consumer.h | 64 +++++++++++++++++------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 9ddcf50a3c59..a7f08fb0f865 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -247,7 +247,7 @@ static inline void gpiod_put(struct gpio_desc *desc)
 	might_sleep();
 
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 
 static inline void devm_gpiod_unhinge(struct device *dev,
@@ -256,7 +256,7 @@ static inline void devm_gpiod_unhinge(struct device *dev,
 	might_sleep();
 
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 
 static inline void gpiod_put_array(struct gpio_descs *descs)
@@ -264,7 +264,7 @@ static inline void gpiod_put_array(struct gpio_descs *descs)
 	might_sleep();
 
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(descs);
 }
 
 static inline struct gpio_desc *__must_check
@@ -317,7 +317,7 @@ static inline void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
 	might_sleep();
 
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 
 static inline void devm_gpiod_put_array(struct device *dev,
@@ -326,32 +326,32 @@ static inline void devm_gpiod_put_array(struct device *dev,
 	might_sleep();
 
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(descs);
 }
 
 
 static inline int gpiod_get_direction(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -ENOSYS;
 }
 static inline int gpiod_direction_input(struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -ENOSYS;
 }
 static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -ENOSYS;
 }
 static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -ENOSYS;
 }
 
@@ -359,7 +359,7 @@ static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
 static inline int gpiod_get_value(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return 0;
 }
 static inline int gpiod_get_array_value(unsigned int array_size,
@@ -368,13 +368,13 @@ static inline int gpiod_get_array_value(unsigned int array_size,
 					unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 static inline void gpiod_set_value(struct gpio_desc *desc, int value)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 static inline int gpiod_set_array_value(unsigned int array_size,
 					struct gpio_desc **desc_array,
@@ -382,13 +382,13 @@ static inline int gpiod_set_array_value(unsigned int array_size,
 					unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 static inline int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return 0;
 }
 static inline int gpiod_get_raw_array_value(unsigned int array_size,
@@ -397,13 +397,13 @@ static inline int gpiod_get_raw_array_value(unsigned int array_size,
 					    unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 static inline int gpiod_set_raw_array_value(unsigned int array_size,
 					    struct gpio_desc **desc_array,
@@ -411,14 +411,14 @@ static inline int gpiod_set_raw_array_value(unsigned int array_size,
 					    unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 
 static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return 0;
 }
 static inline int gpiod_get_array_value_cansleep(unsigned int array_size,
@@ -427,13 +427,13 @@ static inline int gpiod_get_array_value_cansleep(unsigned int array_size,
 				     unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 static inline int gpiod_set_array_value_cansleep(unsigned int array_size,
 					    struct gpio_desc **desc_array,
@@ -441,13 +441,13 @@ static inline int gpiod_set_array_value_cansleep(unsigned int array_size,
 					    unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return 0;
 }
 static inline int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
@@ -456,14 +456,14 @@ static inline int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 					       unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
 						int value)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 }
 static inline int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 						struct gpio_desc **desc_array,
@@ -471,41 +471,41 @@ static inline int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 						unsigned long *value_bitmap)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc_array);
 	return 0;
 }
 
 static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -ENOSYS;
 }
 
 static inline int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -ENOSYS;
 }
 
 static inline int gpiod_is_active_low(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return 0;
 }
 static inline int gpiod_cansleep(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return 0;
 }
 
 static inline int gpiod_to_irq(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -EINVAL;
 }
 
@@ -513,7 +513,7 @@ static inline int gpiod_set_consumer_name(struct gpio_desc *desc,
 					  const char *name)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -EINVAL;
 }
 
@@ -525,7 +525,7 @@ static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
 static inline int desc_to_gpio(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-	WARN_ON(1);
+	WARN_ON(desc);
 	return -EINVAL;
 }
 
-- 
2.21.0


^ permalink raw reply related

* [GIT PULL] bulk GPIO changes for v5.3
From: Linus Walleij @ 2019-07-08  7:26 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: open list:GPIO SUBSYSTEM, linux-kernel, Bartosz Golaszewski

Hi Linus,

this is the big slew of GPIO changes for the v5.3 kernel cycle.
This is mostly incremental work this time.

Three important things:

- The FMC subsystem is deleted through my tree. This happens
  through GPIO as its demise was discussed in relation to a patch
  decoupling its GPIO implementation from the standard way of
  handling GPIO. As it turns out, that is not the only subsystem it
  reimplements and the authors think it is better do scratch it and
  start over using the proper kernel subsystems than try to polish
  the rust shiny. See the commit (ACKed by the maintainers) for
  details.

- Arnd made a small devres patch that was ACKed by Greg and
  goes into the device core.

- SPDX header change colissions may happen, because at times
  I've seen that quite a lot changed during the -rc:s in regards to
  SPDX. (It is good stuff, tglx has me convinced, and it is worth
  the occasional pain.)

Apart from this is is nothing controversial or problematic. I have
started to add Link: tags to lore using Kees script, it seems to
work for any message passing through the lore lists and this
seems pretty helpful (triggered by a discussion on the maintainer
summit list).

Please pull it in!

Yours,
Linus Walleij

The following changes since commit cd6c84d8f0cdc911df435bb075ba22ce3c605b07:

  Linux 5.2-rc2 (2019-05-26 16:49:19 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
tags/gpio-v5.3-1

for you to fetch changes up to 9b3b623804a67d2274ee372c1587926ab0275833:

  Revert "gpio: tegra: Clean-up debugfs initialisation" (2019-07-06
20:16:17 +0200)

----------------------------------------------------------------
Bulk GPIO changes for the v5.3 kernel cycle:

Core:

- When a gpio_chip request GPIOs from itself, it can now fully
  control the line characteristics, both machine and consumer
  flags. This makes a lot of sense, but took some time before I
  figured out that this is how it has to work.

- Several smallish documentation fixes.

New drivers:

- The PCA953x driver now supports the TI TCA9539.

- The DaVinci driver now supports the K3 AM654 SoCs.

Driver improvements:

- Major overhaul and hardening of the OMAP driver by Russell
  King.

- Starting to move some drivers to the new API passing irq_chip
  along with the gpio_chip when adding the gpio_chip instead
  of adding it separately.

Unrelated:

- Delete the FMC subsystem.

----------------------------------------------------------------
Andrey Smirnov (1):
      gpio: vf610: Use PTR_ERR_OR_ZERO() in vf610_gpio_probe()

Arnd Bergmann (1):
      devres: allow const resource arguments

Bartosz Golaszewski (3):
      gpio: em: use the managed version of gpiochip_add_data()
      gpio: max732x: use i2c_new_dummy_device()
      gpio: max732x: use devm_gpiochip_add_data()

Charles Keepax (1):
      gpio: madera: Fixup SPDX headers

Enrico Weigelt (6):
      gpio: amd: Drop unused pdev pointer in privata data
      gpio: Add comments on #if/#else/#endif
      siox: Add helper macro to simplify driver registration
      gpio: siox: Use module_siox_driver()
      drivers: gpio: amd-fch: make resource struct const
      gpio: pl061: drop duplicate printing of device name

Enrico Weigelt, metux IT consult (7):
      gpio: rcar: Pedantic formatting
      gpio: amdpt: Drop unneeded deref of &pdev->dev
      gpio: ep93xx: Use devm_platform_ioremap_resource()
      gpio: grgpio: Use devm_platform_ioremap_resource()
      gpio: janz-ttl: Drop unneccessary temp variable dev
      gpio: vr41xx: Use devm_platform_ioremap_resource()
      gpio: eic-sprd: Use devm_platform_ioremap_resource()

Geert Uytterhoeven (7):
      gpio: em: Remove error messages on out-of-memory conditions
      gpio: em: Return early on error in em_gio_probe()
      gpio: Sort GPIO drivers in Makefile
      Documentation: gpio: Fix reference to gpiod_get_array()
      gpiolib: Document new gpio_chip.init_valid_mask field
      gpiolib: Fix references to gpiod_[gs]et_*value_cansleep() variants
      gpiolib: Clarify use of non-sleeping functions

Greg Kroah-Hartman (3):
      gpio: mockup: no need to check return value of debugfs_create functions
      gpio: No need to cast away return value of debugfs_create_file()
      gpio: tegra: No need to cast away return value of debugfs_create_file()

Jon Hunter (1):
      gpio: tegra: Clean-up debugfs initialisation

Keerthy (4):
      gpio: davinci: Fix the compiler warning with ARM64 config enabled
      gpio: Davinci: Add K3 dependencies
      gpio: davinci: Add new compatible for K3 AM654 SoCs
      dt-bindings: gpio: davinci: Add k3 am654 compatible

Kelsey Skunberg (1):
      tools: gpio: Add include/linux/gpio.h to .gitignore

Linus Walleij (21):
      gpio: Update Kconfig text for GPIO_SYSFS
      gpio: Fix minor grammar errors in documentation
      gpio: ixp4xx: Use irq_domain_translate_twocell()
      gpio: of: Handle the Freescale SPI CS
      gpio: pass lookup and descriptor flags to request_own
      fmc: Decouple from Linux GPIO subsystem
      Merge branch 'ib-snps-reset-gpio' into devel
      fmc: Delete the FMC subsystem
      gpio: ftgpio: Pass irqchip when adding gpiochip
      gpio: Add GPIOLIB_IRQCHIP cleanup to TODO
      gpio: Drop the parent_irq from gpio_irq_chip
      gpio: pl061: Pass irqchip when adding gpiochip
      gpio: siox: Do not call gpiochip_remove() on errorpath
      gpio: siox: Switch to IRQ_TYPE_NONE
      Merge tag 'gpio-v5.3-updates-for-linus' of
git://git.kernel.org/.../brgl/linux into devel
      gpio: ath79: Pass irqchip when adding gpiochip
      gpio: siox: Pass irqchip when adding gpiochip
      gpio: siox: Add struct device *dev helper variable
      gpio: siox: Use devm_ managed gpiochip
      gpio: altera: Pass irqchip when adding gpiochip
      Revert "gpio: tegra: Clean-up debugfs initialisation"

Luca Ceresoli (1):
      Documentation: gpio: remove duplicated lines

Martin Blumenstingl (5):
      gpio: of: parse stmmac PHY reset line specific active-low property
      gpio: stp-xway: simplify error handling in xway_stp_probe()
      gpio: stp-xway: improve module clock error handling
      gpio: stp-xway: get rid of the #include <lantiq_soc.h> dependency
      gpio: stp-xway: allow compile-testing

Mauro Carvalho Chehab (1):
      docs: gpio: driver.rst: fix a bad tag

Peter Robinson (1):
      gpio: pca953x: Add support for the TI TCA9539

Phil Reid (1):
      gpio: altera: Allocate irq_chip dynamically

Rob Herring (1):
      dt-bindings: gpio: Convert Arm PL061 to json-schema

Robert Hancock (1):
      gpio: xilinx: convert from OF GPIO to standard devm APIs

Russell King (20):
      gpio: omap: ensure irq is enabled before wakeup
      gpio: omap: fix lack of irqstatus_raw0 for OMAP4
      gpio: omap: remove remainder of list management
      gpio: omap: clean up edge interrupt handling
      gpio: omap: remove irq_ack method
      gpio: omap: move omap_gpio_request() and omap_gpio_free()
      gpio: omap: simplify omap_gpio_get_direction()
      gpio: omap: simplify get() method
      gpio: omap: simplify get_multiple()
      gpio: omap: simplify set_multiple()
      gpio: omap: simplify bank->level_mask
      gpio: omap: simplify read-modify-write
      gpio: omap: simplify omap_toggle_gpio_edge_triggering()
      gpio: omap: simplify omap_set_gpio_irqenable()
      gpio: omap: remove dataout variation in context handling
      gpio: omap: clean up omap_gpio_restore_context()
      gpio: omap: constify register tables
      gpio: omap: clean up wakeup handling
      gpio: omap: irq_startup() must not return error codes
      gpio: omap: clean up register access in omap2_set_gpio_debounce()

Sebastian Andrzej Siewior (1):
      gpiolib: Use spinlock_t instead of struct spinlock

Tony Lindgren (1):
      gpio: omap: Fix lost edge wake-up interrupts

 .../devicetree/bindings/gpio/gpio-davinci.txt      |  18 +
 .../devicetree/bindings/gpio/pl061-gpio.txt        |  10 -
 .../devicetree/bindings/gpio/pl061-gpio.yaml       |  69 +++
 Documentation/driver-api/gpio/consumer.rst         |   4 +-
 Documentation/driver-api/gpio/driver.rst           |  16 +-
 Documentation/fmc/API.txt                          |  47 --
 Documentation/fmc/FMC-and-SDB.txt                  |  88 ----
 Documentation/fmc/carrier.txt                      | 311 -------------
 Documentation/fmc/fmc-chardev.txt                  |  64 ---
 Documentation/fmc/fmc-fakedev.txt                  |  36 --
 Documentation/fmc/fmc-trivial.txt                  |  17 -
 Documentation/fmc/fmc-write-eeprom.txt             |  98 ----
 Documentation/fmc/identifiers.txt                  | 168 -------
 Documentation/fmc/mezzanine.txt                    | 123 -----
 Documentation/fmc/parameters.txt                   |  56 ---
 arch/arm/mach-omap1/ams-delta-fiq.c                |   4 +-
 arch/arm/mach-omap1/board-ams-delta.c              |   5 +-
 drivers/Kconfig                                    |   2 -
 drivers/Makefile                                   |   1 -
 drivers/fmc/Kconfig                                |  52 ---
 drivers/fmc/Makefile                               |  15 -
 drivers/fmc/fmc-chardev.c                          | 199 --------
 drivers/fmc/fmc-core.c                             | 388 ----------------
 drivers/fmc/fmc-debug.c                            | 172 -------
 drivers/fmc/fmc-dump.c                             |  58 ---
 drivers/fmc/fmc-fakedev.c                          | 355 --------------
 drivers/fmc/fmc-match.c                            | 113 -----
 drivers/fmc/fmc-private.h                          |   8 -
 drivers/fmc/fmc-sdb.c                              | 219 ---------
 drivers/fmc/fmc-trivial.c                          | 103 -----
 drivers/fmc/fmc-write-eeprom.c                     | 175 -------
 drivers/fmc/fru-parse.c                            |  80 ----
 drivers/gpio/Kconfig                               |  20 +-
 drivers/gpio/Makefile                              | 296 ++++++------
 drivers/gpio/TODO                                  |  40 ++
 drivers/gpio/gpio-altera.c                         |  65 ++-
 drivers/gpio/gpio-amd-fch.c                        |   4 +-
 drivers/gpio/gpio-amdpt.c                          |  10 +-
 drivers/gpio/gpio-ath79.c                          |  66 ++-
 drivers/gpio/gpio-davinci.c                        |   7 +-
 drivers/gpio/gpio-eic-sprd.c                       |   9 +-
 drivers/gpio/gpio-em.c                             |  34 +-
 drivers/gpio/gpio-ep93xx.c                         |   7 +-
 drivers/gpio/gpio-ftgpio010.c                      |  35 +-
 drivers/gpio/gpio-grgpio.c                         |   4 +-
 drivers/gpio/gpio-ixp4xx.c                         |  14 +-
 drivers/gpio/gpio-janz-ttl.c                       |   9 +-
 drivers/gpio/gpio-madera.c                         |   6 +-
 drivers/gpio/gpio-max732x.c                        |  45 +-
 drivers/gpio/gpio-mockup.c                         |  21 +-
 drivers/gpio/gpio-mvebu.c                          |  11 +-
 drivers/gpio/gpio-omap.c                           | 509 +++++++--------------
 drivers/gpio/gpio-pca953x.c                        |   1 +
 drivers/gpio/gpio-pl061.c                          |  30 +-
 drivers/gpio/gpio-rcar.c                           |   2 +-
 drivers/gpio/gpio-siox.c                           |  51 +--
 drivers/gpio/gpio-stp-xway.c                       |  33 +-
 drivers/gpio/gpio-tegra.c                          |   4 +-
 drivers/gpio/gpio-vf610.c                          |  14 +-
 drivers/gpio/gpio-vr41xx.c                         |  19 +-
 drivers/gpio/gpio-xilinx.c                         |  90 ++--
 drivers/gpio/gpiolib-acpi.c                        |   6 +-
 drivers/gpio/gpiolib-of.c                          |  43 ++
 drivers/gpio/gpiolib.c                             |  88 ++--
 drivers/gpio/gpiolib.h                             |   2 +-
 drivers/hid/hid-cp2112.c                           |   7 +-
 drivers/memory/omap-gpmc.c                         |   4 +-
 include/linux/device.h                             |   3 +-
 include/linux/fmc-sdb.h                            |  39 --
 include/linux/fmc.h                                | 269 -----------
 include/linux/gpio/driver.h                        |  29 +-
 include/linux/gpio/gpio-reg.h                      |   2 +-
 include/linux/gpio/machine.h                       |   4 +-
 include/linux/platform_data/gpio-omap.h            |   2 +-
 include/linux/siox.h                               |  10 +
 lib/devres.c                                       |   3 +-
 tools/gpio/.gitignore                              |   2 +-
 77 files changed, 835 insertions(+), 4208 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/gpio/pl061-gpio.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
 delete mode 100644 Documentation/fmc/API.txt
 delete mode 100644 Documentation/fmc/FMC-and-SDB.txt
 delete mode 100644 Documentation/fmc/carrier.txt
 delete mode 100644 Documentation/fmc/fmc-chardev.txt
 delete mode 100644 Documentation/fmc/fmc-fakedev.txt
 delete mode 100644 Documentation/fmc/fmc-trivial.txt
 delete mode 100644 Documentation/fmc/fmc-write-eeprom.txt
 delete mode 100644 Documentation/fmc/identifiers.txt
 delete mode 100644 Documentation/fmc/mezzanine.txt
 delete mode 100644 Documentation/fmc/parameters.txt
 delete mode 100644 drivers/fmc/Kconfig
 delete mode 100644 drivers/fmc/Makefile
 delete mode 100644 drivers/fmc/fmc-chardev.c
 delete mode 100644 drivers/fmc/fmc-core.c
 delete mode 100644 drivers/fmc/fmc-debug.c
 delete mode 100644 drivers/fmc/fmc-dump.c
 delete mode 100644 drivers/fmc/fmc-fakedev.c
 delete mode 100644 drivers/fmc/fmc-match.c
 delete mode 100644 drivers/fmc/fmc-private.h
 delete mode 100644 drivers/fmc/fmc-sdb.c
 delete mode 100644 drivers/fmc/fmc-trivial.c
 delete mode 100644 drivers/fmc/fmc-write-eeprom.c
 delete mode 100644 drivers/fmc/fru-parse.c
 delete mode 100644 include/linux/fmc-sdb.h
 delete mode 100644 include/linux/fmc.h

^ permalink raw reply

* Re: [PATCH] gpio: tegra: Clean-up debugfs initialisation
From: Linus Walleij @ 2019-07-08  7:04 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Bartosz Golaszewski, Thierry Reding, open list:GPIO SUBSYSTEM,
	linux-tegra, Greg KH
In-Reply-To: <20190626134258.26991-1-jonathanh@nvidia.com>

On Wed, Jun 26, 2019 at 3:43 PM Jon Hunter <jonathanh@nvidia.com> wrote:

> The function tegra_gpio_debuginit() just calls debugfs_create_file()
> and given that there is already a stub function implemented for
> debugfs_create_file() when CONFIG_DEBUG_FS is not enabled, there is
> no need for the function tegra_gpio_debuginit() and so remove it.
>
> Finally, use a space and not a tab between the #ifdef and
> CONFIG_DEBUG_FS.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Sadly I've had to revert this commit because of build errors, but
let's see if we can figure it out after the merge window.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v2] gpiolib: fix incorrect IRQ requesting of an active-low lineevent
From: Michael Wu @ 2019-07-08  5:23 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel
  Cc: morgan.chang, stable

When a pin is active-low, logical trigger edge should be inverted to match
the same interrupt opportunity.

For example, a button pushed triggers falling edge in ACTIVE_HIGH case; in
ACTIVE_LOW case, the button pushed triggers rising edge. For user space the
IRQ requesting doesn't need to do any modification except to configuring
GPIOHANDLE_REQUEST_ACTIVE_LOW.

For example, we want to catch the event when the button is pushed. The
button on the original board drives level to be low when it is pushed, and
drives level to be high when it is released.

In user space we can do:

	req.handleflags = GPIOHANDLE_REQUEST_INPUT;
	req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;

	while (1) {
		read(fd, &dat, sizeof(dat));
		if (dat.id == GPIOEVENT_EVENT_FALLING_EDGE)
			printf("button pushed\n");
	}

Run the same logic on another board which the polarity of the button is
inverted; it drives level to be high when pushed, and level to be low when
released. For this inversion we add flag GPIOHANDLE_REQUEST_ACTIVE_LOW:

	req.handleflags = GPIOHANDLE_REQUEST_INPUT |
		GPIOHANDLE_REQUEST_ACTIVE_LOW;
	req.eventflags = GPIOEVENT_REQUEST_FALLING_EDGE;

At the result, there are no any events caught when the button is pushed.
By the way, button releasing will emit a "falling" event. The timing of
"falling" catching is not expected.

Cc: stable@vger.kernel.org
Signed-off-by: Michael Wu <michael.wu@vatics.com>
---
Changes from v1:
- Correct undeclared 'IRQ_TRIGGER_RISING'
- Add an example to descibe the issue
---
 drivers/gpio/gpiolib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..9c9597f929d7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -956,9 +956,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	}
 
 	if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
-		irqflags |= IRQF_TRIGGER_RISING;
+		irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+			IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
 	if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE)
-		irqflags |= IRQF_TRIGGER_FALLING;
+		irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
+			IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
 	irqflags |= IRQF_ONESHOT;
 
 	INIT_KFIFO(le->events);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v3] gpiolib: Preserve desc->flags when setting state
From: Chris Packham @ 2019-07-07 20:35 UTC (permalink / raw)
  To: linus.walleij, bgolaszewski, ricardo.ribalda
  Cc: linux-gpio, linux-kernel, Chris Packham

desc->flags may already have values set by of_gpiochip_add() so make
sure that this isn't undone when setting the initial direction.

Fixes: 3edfb7bd76bd1cba ("gpiolib: Show correct direction from the beginning")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v2:
    - add braces to avoid ambiguious else warning
    Changes in v3:
    - clear the direction output in the negative case

 drivers/gpio/gpiolib.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..dbd1180316a0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1392,12 +1392,17 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 	for (i = 0; i < chip->ngpio; i++) {
 		struct gpio_desc *desc = &gdev->descs[i];
 
-		if (chip->get_direction && gpiochip_line_is_valid(chip, i))
-			desc->flags = !chip->get_direction(chip, i) ?
-					(1 << FLAG_IS_OUT) : 0;
-		else
-			desc->flags = !chip->direction_input ?
-					(1 << FLAG_IS_OUT) : 0;
+		if (chip->get_direction && gpiochip_line_is_valid(chip, i)) {
+			if (!chip->get_direction(chip, i))
+				set_bit(FLAG_IS_OUT, &desc->flags);
+			else
+				clear_bit(FLAG_IS_OUT, &desc->flags);
+		} else {
+			if (!chip->direction_input)
+				set_bit(FLAG_IS_OUT, &desc->flags);
+			else
+				clear_bit(FLAG_IS_OUT, &desc->flags);
+		}
 	}
 
 	acpi_gpiochip_add(chip);
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH 1/4 v1] gpio: Add support for hierarchical IRQ domains
From: Linus Walleij @ 2019-07-07  8:09 UTC (permalink / raw)
  To: Brian Masney
  Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Thomas Gleixner,
	Marc Zyngier, Lina Iyer, Jon Hunter, Sowjanya Komatineni,
	Bitan Biswas, linux-tegra, David Daney, Masahiro Yamada,
	Thierry Reding
In-Reply-To: <20190707014620.GA9690@onstation.org>

On Sun, Jul 7, 2019 at 3:46 AM Brian Masney <masneyb@onstation.org> wrote:

> I got this working with spmi-gpio with two additional changes. See below
> for details. Hopefully I'll have time tomorrow evening (GMT-4) to finish
> cleaning up what I have so I can send out my series.

Awesome! No hurry because it is v5.4 material at this point but I'm
hoping to get to something you, Lina and Thierry can all use for early
merge and smoke test.

> > +static const struct irq_domain_ops gpiochip_hierarchy_domain_ops = {
> > +     .activate = gpiochip_irq_domain_activate,
> > +     .deactivate = gpiochip_irq_domain_deactivate,
> > +     .translate = gpiochip_hierarchy_irq_domain_translate,
> > +     .alloc = gpiochip_hierarchy_irq_domain_alloc,
> > +     .free = irq_domain_free_irqs_common,
> > +};
>
> spmi and ssbi gpio both need to subtract one from the hwirq in the
> translate function.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c#L956
>
> I'm going to optionally allow overriding the translate() function
> pointer as well.

Hm was more thinking to let gpiolib call out to an optional translate
function (on top of the template) but this is maybe cleaner.

> > +             /*
> > +              * We set handle_bad_irq because the .set_type() should
> > +              * always be invoked and set the right type of handler.
> > +              */
> > +             irq_domain_set_info(d,
> > +                                 irq + i,
> > +                                 hwirq + i,
> > +                                 gc->irq.chip,
> > +                                 gc,
> > +                                 handle_bad_irq,
>                                     ^^^^^^^^^^
> In order to get this working, I had to change handle_bad_irq to
> handle_level_irq otherwise I get this attempted NULL pointer
> dereference:

Hmmmmmmmm that should not happen, we need to get to the
bottom of this.

> [    2.624430] [<c0372af4>] (irq_chip_ack_parent) from [<c0373f6c>] (__irq_do_set_handler+0x1b4/0x1bc)
> [    2.632584] [<c0373f6c>] (__irq_do_set_handler) from [<c0373fc0>] (__irq_set_handler+0x4c/0x78)
> [    2.641441] [<c0373fc0>] (__irq_set_handler) from [<c0375d44>] (irq_domain_set_info+0x38/0x4c)
> [    2.650126] [<c0375d44>] (irq_domain_set_info) from [<c06cf28c>] (gpiochip_hierarchy_irq_domain_alloc+0x16c/0x22c)
> [    2.658808] [<c06cf28c>] (gpiochip_hierarchy_irq_domain_alloc) from [<c0376bac>] (__irq_domain_alloc_irqs+0x12c/0x320)

I wonder why irq_chip_ack_parent() is called there.
Like there is some pending IRQ or something.

> The parent's irq_chip struct isn't populated yet and the error occurs
> here:
>
>     void irq_chip_ack_parent(struct irq_data *data)
>     {
>             data = data->parent_data;
>             data->chip->irq_ack(data);
>                   ^^^^
>
> We haven't called irq_domain_alloc_irqs_parent() yet, which is fine.
>
> __irq_do_set_handler() has a special check for handle_bad_irq():
>
> https://elixir.bootlin.com/linux/latest/source/kernel/irq/chip.c#L974
>
> I'm not sure what the proper fix is here and not going to dig into this
> anymore this evening.

I'm a bit puzzled too. :/

> > diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst
> > index 1ce7fcd0f989..3099c7fbefdb 100644
> > --- a/Documentation/driver-api/gpio/driver.rst
> > +++ b/Documentation/driver-api/gpio/driver.rst
>
> I'm still on linux next-20190701. Does this patch series of yours
> require any other patches? I get a merge conflict against driver.rst.
> Everything else applies cleanly. I honestly haven't looked in detail
> about the conflicts.

I have some pending documentation patch I think.
Sorry about that.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] gpio: remove less important #ifdef around declarations
From: Masahiro Yamada @ 2019-07-07  2:30 UTC (permalink / raw)
  To: linux-gpio, Linus Walleij
  Cc: Masahiro Yamada, Bartosz Golaszewski, linux-kernel

The whole struct/function declarations in this header are surrounded
by #ifdef.

As far as I understood, the motivation of doing so is probably to break
the build earlier if a driver misses to select or depend on correct
CONFIG options in Kconfig.

Since commit 94bed2a9c4ae ("Add -Werror-implicit-function-declaration")
no one cannot call functions that have not been declared.

So, I see some benefit in doing this in the cost of uglier headers.

In reality, it would not be so easy to catch missed 'select' or
'depends on' because GPIOLIB, GPIOLIB_IRQCHIP etc. are already selected
by someone else eventually. So, this kind of error, if any, will be
caught by randconfig bots.

In summary, I am not a big fan of cluttered #ifdef nesting, and this
does not matter for normal developers. The code readability wins.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

FYI,
If you want replace the commit log of the pinctrl variant,
the better log is available:
https://lkml.org/lkml/2019/6/26/1344

 include/linux/gpio/driver.h | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index a1d273c96016..845d657f2431 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -19,9 +19,6 @@ struct gpio_device;
 struct module;
 enum gpiod_flags;
 
-#ifdef CONFIG_GPIOLIB
-
-#ifdef CONFIG_GPIOLIB_IRQCHIP
 /**
  * struct gpio_irq_chip - GPIO interrupt controller
  */
@@ -167,7 +164,6 @@ struct gpio_irq_chip {
 	 */
 	void		(*irq_disable)(struct irq_data *data);
 };
-#endif
 
 /**
  * struct gpio_chip - abstract a GPIO controller
@@ -445,16 +441,12 @@ bool gpiochip_line_is_valid(const struct gpio_chip *chip, unsigned int offset);
 /* get driver data */
 void *gpiochip_get_data(struct gpio_chip *chip);
 
-struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc);
-
 struct bgpio_pdata {
 	const char *label;
 	int base;
 	int ngpio;
 };
 
-#if IS_ENABLED(CONFIG_GPIO_GENERIC)
-
 int bgpio_init(struct gpio_chip *gc, struct device *dev,
 	       unsigned long sz, void __iomem *dat, void __iomem *set,
 	       void __iomem *clr, void __iomem *dirout, void __iomem *dirin,
@@ -467,10 +459,6 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
 #define BGPIOF_READ_OUTPUT_REG_SET	BIT(4) /* reg_set stores output value */
 #define BGPIOF_NO_OUTPUT		BIT(5) /* only input */
 
-#endif
-
-#ifdef CONFIG_GPIOLIB_IRQCHIP
-
 int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
 		     irq_hw_number_t hwirq);
 void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq);
@@ -559,15 +547,11 @@ static inline int gpiochip_irqchip_add_nested(struct gpio_chip *gpiochip,
 }
 #endif /* CONFIG_LOCKDEP */
 
-#endif /* CONFIG_GPIOLIB_IRQCHIP */
-
 int gpiochip_generic_request(struct gpio_chip *chip, unsigned offset);
 void gpiochip_generic_free(struct gpio_chip *chip, unsigned offset);
 int gpiochip_generic_config(struct gpio_chip *chip, unsigned offset,
 			    unsigned long config);
 
-#ifdef CONFIG_PINCTRL
-
 /**
  * struct gpio_pin_range - pin range controlled by a gpio chip
  * @node: list for maintaining set of pin ranges, used internally
@@ -580,6 +564,8 @@ struct gpio_pin_range {
 	struct pinctrl_gpio_range range;
 };
 
+#ifdef CONFIG_PINCTRL
+
 int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
 			   unsigned int gpio_offset, unsigned int pin_offset,
 			   unsigned int npins);
@@ -620,6 +606,10 @@ void gpiochip_free_own_desc(struct gpio_desc *desc);
 void devprop_gpiochip_set_names(struct gpio_chip *chip,
 				const struct fwnode_handle *fwnode);
 
+#ifdef CONFIG_GPIOLIB
+
+struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc);
+
 #else /* CONFIG_GPIOLIB */
 
 static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
@@ -631,4 +621,4 @@ static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 
 #endif /* CONFIG_GPIOLIB */
 
-#endif
+#endif /* __LINUX_GPIO_DRIVER_H */
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 1/4 v1] gpio: Add support for hierarchical IRQ domains
From: Brian Masney @ 2019-07-07  1:46 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski, Thomas Gleixner, Marc Zyngier,
	Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
	linux-tegra, David Daney, Masahiro Yamada, Thierry Reding
In-Reply-To: <20190624132531.6184-1-linus.walleij@linaro.org>

Hi Linus,

On Mon, Jun 24, 2019 at 03:25:28PM +0200, Linus Walleij wrote:
> Hierarchical IRQ domains can be used to stack different IRQ
> controllers on top of each other.
> 
> Bring hierarchical IRQ domains into the GPIOLIB core with the
> following basic idea:

I got this working with spmi-gpio with two additional changes. See below
for details. Hopefully I'll have time tomorrow evening (GMT-4) to finish
cleaning up what I have so I can send out my series.

> +static int gpiochip_hierarchy_irq_domain_translate(struct irq_domain *d,
> +						   struct irq_fwspec *fwspec,
> +						   unsigned long *hwirq,
> +						   unsigned int *type)
> +{
> +	/* We support standard DT translation */
> +	if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2) {
> +		return irq_domain_translate_twocell(d, fwspec, hwirq, type);
> +	}
> +
> +	/* This is for board files and others not using DT */
> +	if (is_fwnode_irqchip(fwspec->fwnode)) {
> +		int ret;
> +
> +		ret = irq_domain_translate_twocell(d, fwspec, hwirq, type);
> +		if (ret)
> +			return ret;
> +		WARN_ON(*type == IRQ_TYPE_NONE);
> +		return 0;
> +	}
> +	return -EINVAL;
> +}
>
> [ snip ]
>
> +static const struct irq_domain_ops gpiochip_hierarchy_domain_ops = {
> +	.activate = gpiochip_irq_domain_activate,
> +	.deactivate = gpiochip_irq_domain_deactivate,
> +	.translate = gpiochip_hierarchy_irq_domain_translate,
> +	.alloc = gpiochip_hierarchy_irq_domain_alloc,
> +	.free = irq_domain_free_irqs_common,
> +};

spmi and ssbi gpio both need to subtract one from the hwirq in the
translate function.

https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c#L956

I'm going to optionally allow overriding the translate() function
pointer as well.

> +static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
> +					       unsigned int irq,
> +					       unsigned int nr_irqs,
> +					       void *data)
> +{
> +	struct gpio_chip *gc = d->host_data;
> +	irq_hw_number_t hwirq;
> +	unsigned int type = IRQ_TYPE_NONE;
> +	struct irq_fwspec *fwspec = data;
> +	int ret;
> +	int i;
> +
> +	chip_info(gc, "called %s\n", __func__);
> +
> +	ret = gpiochip_hierarchy_irq_domain_translate(d, fwspec, &hwirq, &type);
> +	if (ret)
> +		return ret;
> +
> +	chip_info(gc, "allocate IRQ %d..%d, hwirq %lu..%lu\n",
> +		  irq, irq + nr_irqs - 1,
> +		  hwirq, hwirq + nr_irqs - 1);
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		struct irq_fwspec parent_fwspec;
> +		unsigned int parent_hwirq;
> +		unsigned int parent_type;
> +		struct gpio_irq_chip *girq = &gc->irq;
> +
> +		ret = girq->child_to_parent_hwirq(gc, hwirq, type,
> +						  &parent_hwirq, &parent_type);
> +		if (ret) {
> +			chip_err(gc, "can't look up hwirq %lu\n", hwirq);
> +			return ret;
> +		}
> +		chip_info(gc, "found parent hwirq %u\n", parent_hwirq);
> +
> +		/*
> +		 * We set handle_bad_irq because the .set_type() should
> +		 * always be invoked and set the right type of handler.
> +		 */
> +		irq_domain_set_info(d,
> +				    irq + i,
> +				    hwirq + i,
> +				    gc->irq.chip,
> +				    gc,
> +				    handle_bad_irq,
                                    ^^^^^^^^^^
In order to get this working, I had to change handle_bad_irq to
handle_level_irq otherwise I get this attempted NULL pointer
dereference:

[    2.260256] Unable to handle kernel NULL pointer dereference at virtual address 00000018
[    2.263388] pgd = (ptrval)
[    2.271624] [00000018] *pgd=00000000
[    2.274149] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[    2.277877] Modules linked in:
[    2.283174] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc7-next-20190701-00017-g58c27736c3cb-dirty #34
[    2.286043] Hardware name: Generic DT based system
[    2.295687] PC is at irq_chip_ack_parent+0x8/0x10
[    2.300540] LR is at __irq_do_set_handler+0x1b4/0x1bc
[    2.305309] pc : [<c0372af4>]    lr : [<c0373f6c>]    psr: a0000093
[    2.310343] sp : de899be0  ip : c100f06c  fp : 00000001
[    2.316420] r10: 0000004e  r9 : 00000000  r8 : 00000000
[    2.321626] r7 : 00000000  r6 : 00000000  r5 : c036eea4  r4 : c4e7a600
[    2.326839] r3 : 00000000  r2 : 00000000  r1 : c0372aec  r0 : c4e7d5c0
[    2.333438] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    2.339947] Control: 10c5787d  Table: 0020406a  DAC: 00000051
[    2.347152] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[    2.352967] Stack: (0xde899be0 to 0xde89a000)
[    2.359063] 9be0: 00000000 00000000 c1004c48 c4e7a600 c036eea4 c0373fc0 60000013 921471da
[    2.363322] 9c00: 0000004e 00000000 c036eea4 00000000 00000000 c0375d44 c4df124c 921471da
[    2.371480] 9c20: c4df124c c4df124c 0000004e 00000000 c4de8900 c06cf28c c4df124c c036eea4
[    2.379640] 9c40: 00000000 00000000 c1004c48 c0da5e28 c036eea4 c0da5e48 de899c7c 00000001
[    2.387800] 9c60: 00000000 000000c1 00000000 00000001 c4e7a600 c036e394 c0d74960 00000000
[    2.395959] 9c80: 0000004f c0af60b0 00000000 00000000 00000002 0000004e c4df0600 00000001
[    2.404120] 9ca0: 00000000 ffffffff 00000000 c4de8900 c4e7d5c0 921471da c4e27940 c06cf120
[    2.412278] 9cc0: 00000001 0000004e 0000004e c4de8900 c0e172b4 00000001 c4e7d5c0 c0376bac
[    2.420438] 9ce0: 00000000 c0834274 00000000 c1004c48 c4de8900 de899d4c 00000000 de9d9c10
[    2.428598] 9d00: 00000001 00000000 c4e27940 c03771d4 de899d4c 00000000 00000000 c06ca5b0
[    2.436759] 9d20: de9d9db8 00000001 00000000 921471da c1004c48 c4df124c c4de8900 00000001
[    2.444919] 9d40: de9d9c10 c06c90b8 c27065bc def6e270 00000002 00000002 00000000 c10a2374
[    2.453077] 9d60: c4d6f810 de9d9c10 de899d80 6f697067 de9d0073 a0000013 c1004c48 00000000
[    2.461237] 9d80: a0000013 c098977c c4e27940 921471da c4e27940 921471da c4d6f810 c4e6ec8c
[    2.469399] 9da0: c4e2795c 00000000 c27065bc c06c8624 c4e6ec8c c092b1b8 00000001 c27065bc
[    2.477558] 9dc0: c10c59b8 c4e6ec60 00000000 c4e81000 00000000 def757d4 c4e2795c c4e6ec40
[    2.485717] 9de0: de9d9c00 c092ab44 c092aac8 c092ab60 00000000 de9d9c10 00000000 c10a2608
[    2.493876] 9e00: 00000000 c10c59b8 00000000 c10a2608 0000000e c0817c00 c110dcf4 de9d9c10
[    2.502037] 9e20: c110dcf8 c0815738 c10a2608 de9d9c10 c0f004a4 de9d9c10 c10a2608 c10a2608
[    2.510197] 9e40: c0815fa4 00000000 c0f56838 c0f56858 c0f004a4 c0815bf4 c0f56858 c0989428
[    2.518355] 9e60: c0bbf5c8 de9d9c10 00000000 c10a2608 c0815fa4 00000000 c0f56838 c0f56858
[    2.526515] 9e80: c0f004a4 c0815f9c 00000000 c10a2608 de9d9c10 c0816058 de9dab34 c1004c48
[    2.534677] 9ea0: c10a2608 c0813908 c1095780 de825858 de9dab34 921471da de82586c c10a2608
[    2.542835] 9ec0: c4e96780 c1095780 00000000 c0814aa0 c0df2cac c1004c48 ffffe000 c10a2608
[    2.550994] 9ee0: c1004c48 ffffe000 c0f39074 c0816be4 c10b97c0 c1004c48 ffffe000 c0302f7c
[    2.559153] 9f00: 00000000 c0f004a4 c10be278 00000000 00000007 c0d59294 c0dd2fe4 00000000
[    2.567314] 9f20: 00000000 c1004c48 c0d69934 c0d5934c 00002cc0 dffffbf8 dffffc0d 921471da
[    2.575475] 9f40: 00000000 c10b97c0 dffffa80 921471da c10b97c0 c0f6414c 00000008 c10d2cc0
[    2.583633] 9f60: c10d2cc0 c0f01204 00000007 00000007 00000000 c0f004a4 000000db 00000000
[    2.591793] 9f80: c0af5ba8 00000000 c0af5ba8 00000000 00000000 00000000 00000000 00000000
[    2.599951] 9fa0: 00000000 c0af5bb0 00000000 c03010e8 00000000 00000000 00000000 00000000
[    2.608111] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.616271] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[    2.624430] [<c0372af4>] (irq_chip_ack_parent) from [<c0373f6c>] (__irq_do_set_handler+0x1b4/0x1bc)
[    2.632584] [<c0373f6c>] (__irq_do_set_handler) from [<c0373fc0>] (__irq_set_handler+0x4c/0x78)
[    2.641441] [<c0373fc0>] (__irq_set_handler) from [<c0375d44>] (irq_domain_set_info+0x38/0x4c)
[    2.650126] [<c0375d44>] (irq_domain_set_info) from [<c06cf28c>] (gpiochip_hierarchy_irq_domain_alloc+0x16c/0x22c)
[    2.658808] [<c06cf28c>] (gpiochip_hierarchy_irq_domain_alloc) from [<c0376bac>] (__irq_domain_alloc_irqs+0x12c/0x320)
[    2.669134] [<c0376bac>] (__irq_domain_alloc_irqs) from [<c03771d4>] (irq_create_fwspec_mapping+0x27c/0x344)
[    2.679808] [<c03771d4>] (irq_create_fwspec_mapping) from [<c06c90b8>] (gpiochip_to_irq+0x6c/0xa0)
[    2.689793] [<c06c90b8>] (gpiochip_to_irq) from [<c06c8624>] (gpiod_to_irq+0x48/0x64)
[    2.698558] [<c06c8624>] (gpiod_to_irq) from [<c092b1b8>] (gpio_keys_probe+0x4b4/0x8e8)
[    2.706461] [<c092b1b8>] (gpio_keys_probe) from [<c0817c00>] (platform_drv_probe+0x48/0x98)
[    2.714272] [<c0817c00>] (platform_drv_probe) from [<c0815738>] (really_probe+0x108/0x40c)
[    2.722599] [<c0815738>] (really_probe) from [<c0815bf4>] (driver_probe_device+0x78/0x1c4)
[    2.730934] [<c0815bf4>] (driver_probe_device) from [<c0815f9c>] (device_driver_attach+0x58/0x60)
[    2.739182] [<c0815f9c>] (device_driver_attach) from [<c0816058>] (__driver_attach+0xb4/0x154)
[    2.748121] [<c0816058>] (__driver_attach) from [<c0813908>] (bus_for_each_dev+0x74/0xb4)
[    2.756628] [<c0813908>] (bus_for_each_dev) from [<c0814aa0>] (bus_add_driver+0x1c0/0x200)
[    2.764875] [<c0814aa0>] (bus_add_driver) from [<c0816be4>] (driver_register+0x7c/0x114)
[    2.773035] [<c0816be4>] (driver_register) from [<c0302f7c>] (do_one_initcall+0x54/0x2a0)
[    2.781284] [<c0302f7c>] (do_one_initcall) from [<c0f01204>] (kernel_init_freeable+0x2d4/0x36c)
[    2.789357] [<c0f01204>] (kernel_init_freeable) from [<c0af5bb0>] (kernel_init+0x8/0x110)
[    2.797860] [<c0af5bb0>] (kernel_init) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
[    2.806181] Exception stack(0xde899fb0 to 0xde899ff8)
[    2.813653] 9fa0:                                     00000000 00000000 00000000 00000000
[    2.818788] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.826943] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    2.835102] Code: 0592301c e12fff13 e5900018 e5903010 (e5933018) 
[    2.841513] ---[ end trace e31675e4bfb4e93f ]---

The parent's irq_chip struct isn't populated yet and the error occurs
here:

    void irq_chip_ack_parent(struct irq_data *data)
    {
            data = data->parent_data;
            data->chip->irq_ack(data);
                  ^^^^

We haven't called irq_domain_alloc_irqs_parent() yet, which is fine.

__irq_do_set_handler() has a special check for handle_bad_irq():

https://elixir.bootlin.com/linux/latest/source/kernel/irq/chip.c#L974

I'm not sure what the proper fix is here and not going to dig into this
anymore this evening.

> diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst
> index 1ce7fcd0f989..3099c7fbefdb 100644
> --- a/Documentation/driver-api/gpio/driver.rst
> +++ b/Documentation/driver-api/gpio/driver.rst

I'm still on linux next-20190701. Does this patch series of yours
require any other patches? I get a merge conflict against driver.rst.
Everything else applies cleanly. I honestly haven't looked in detail
about the conflicts.

Brian

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox