* Re: [PATCH 2/2 v3] pinctrl: rza2: Include the appropriate headers
From: Geert Uytterhoeven @ 2019-08-21 7:45 UTC (permalink / raw)
To: Linus Walleij
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Chris Brandt,
Geert Uytterhoeven
In-Reply-To: <20190820135955.14391-2-linus.walleij@linaro.org>
On Tue, Aug 20, 2019 at 4:00 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> This driver is implementing a GPIO driver so include
> <linux/gpio/driver.h> and not the legacy API <linux/gpio.h>.
> When testing it turns out it also relies on implicit
> inclusion of <linux/io.h> (readw etc) so make sure to
> include that as well.
>
> Cc: Chris Brandt <chris.brandt@renesas.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in sh-pfc-for-v5.4.
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 v2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Uwe Kleine-König @ 2019-08-21 8:08 UTC (permalink / raw)
To: Stefan Roese
Cc: linux-serial, linux-gpio, Andy Shevchenko, Geert Uytterhoeven,
Pavel Machek, Linus Walleij, Greg Kroah-Hartman, kernel
In-Reply-To: <20190815085341.28088-1-sr@denx.de>
Hello Stefan,
On Thu, Aug 15, 2019 at 10:53:41AM +0200, Stefan Roese wrote:
> This patch fixes a backward compatibility issue, when boards use the
> old style GPIO suffix "-gpio" instead of the new "-gpios". This
> potential problem has been introduced by commit d99482673f95 ("serial:
> mctrl_gpio: Check if GPIO property exisits before requesting it").
>
> This patch now fixes this issue by using gpiod_count() which iterates
> over all supported GPIO suffixes (thanks to Linus for suggesting this).
>
> With this change, the local string is not needed any more. This way
> we can remove the allocation in the loop.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> v2
> - Use gpiod_count() to check if the GPIO exists (Linus)
> - Remove the now unnecessary malloc in the loop (kasprintf)
>
> drivers/tty/serial/serial_mctrl_gpio.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index 2b400189be91..ce73b142c66b 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -117,18 +117,11 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
>
> for (i = 0; i < UART_GPIO_MAX; i++) {
> enum gpiod_flags flags;
> - char *gpio_str;
> - bool present;
> + int count;
>
> /* Check if GPIO property exists and continue if not */
> - gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
> - mctrl_gpios_desc[i].name);
> - if (!gpio_str)
> - continue;
> -
> - present = device_property_present(dev, gpio_str);
> - kfree(gpio_str);
> - if (!present)
> + count = gpiod_count(dev, mctrl_gpios_desc[i].name);
> + if (count <= 0)
> continue;
Can you explain why this check is necessary (both the new and the old
variant)? I would expect that it doesn't add any value over the
gpiod_get_index_optional that follows later.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)
From: Uwe Kleine-König @ 2019-08-21 8:26 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Stefan Roese, open list:SERIAL DRIVERS, open list:GPIO SUBSYSTEM,
Pavel Machek, Linus Walleij, Greg Kroah-Hartman, Mika Westerberg
In-Reply-To: <CAMuHMdXYLB1UShMjoZi6gzyxx=uOeLdVsZmhWZO5J+=As-5gdw@mail.gmail.com>
On Fri, Aug 16, 2019 at 09:16:39AM +0200, Geert Uytterhoeven wrote:
> CC Mika, who reported the initial issue
>
> On Thu, Aug 15, 2019 at 10:53 AM Stefan Roese <sr@denx.de> wrote:
> >
> > This patch fixes a backward compatibility issue, when boards use the
> > old style GPIO suffix "-gpio" instead of the new "-gpios". This
> > potential problem has been introduced by commit d99482673f95 ("serial:
> > mctrl_gpio: Check if GPIO property exisits before requesting it").
> >
> > This patch now fixes this issue by using gpiod_count() which iterates
> > over all supported GPIO suffixes (thanks to Linus for suggesting this).
> >
> > With this change, the local string is not needed any more. This way
> > we can remove the allocation in the loop.
> >
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> > Cc: Pavel Machek <pavel@denx.de>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > v2
> > - Use gpiod_count() to check if the GPIO exists (Linus)
> > - Remove the now unnecessary malloc in the loop (kasprintf)
> >
> > drivers/tty/serial/serial_mctrl_gpio.c | 13 +++----------
> > 1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> > index 2b400189be91..ce73b142c66b 100644
> > --- a/drivers/tty/serial/serial_mctrl_gpio.c
> > +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> > @@ -117,18 +117,11 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
> >
> > for (i = 0; i < UART_GPIO_MAX; i++) {
> > enum gpiod_flags flags;
> > - char *gpio_str;
> > - bool present;
> > + int count;
> >
> > /* Check if GPIO property exists and continue if not */
> > - gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
> > - mctrl_gpios_desc[i].name);
> > - if (!gpio_str)
> > - continue;
> > -
> > - present = device_property_present(dev, gpio_str);
> > - kfree(gpio_str);
> > - if (!present)
> > + count = gpiod_count(dev, mctrl_gpios_desc[i].name);
> > + if (count <= 0)
> > continue;
> >
> > if (mctrl_gpios_desc[i].dir_out)
>
> Seems like both device_property_present() and gpiod_count()
> eventually call into acpi_data_get_property().
>
> However, given
> commit 6fe9da42f1d98fdb4be1598e230aca97e66cf35d
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Tue May 23 20:03:20 2017 +0300
>
> gpio: acpi: Synchronize acpi_find_gpio() and acpi_gpio_count()
>
> If we pass connection ID to the both functions and at the same time
> acpi_can_fallback_to_crs() returns false we will get different results,
> i.e. the number of GPIO resources returned by acpi_gpio_count() might be
> not correct.
>
> Fix this by calling acpi_can_fallback_to_crs() in acpi_gpio_count()
> before trying to fallback.
>
> acpi_find_gpio() and acpi_gpio_count() are supposed to use the exact
> same logic, so this patch is not gonna work as intended?!?
>
> Note that I still find it strange that acpi_find_gpio() falls back to
> unnamed gpios if con_id != NULL, causing the problem in the first place.
> This is gonna bite us again later...
Ah, is this the reason we need an additional check before calling
gpiod_get_index_optional?
I would prefer to fix the acpi code then and drop the additional checks
in the mctrl-gpio code.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] RFT: microblaze: Switch to standard restart handler
From: Linus Walleij @ 2019-08-21 8:33 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Arnd Bergmann, Michal Simek
The microblaze uses the legacy APIs to dig out a GPIO pin
defined in the root of the device tree to issue a hard
reset of the platform.
Asserting a hard reset should be done using the standard
DT-enabled and fully GPIO descriptor aware driver in
drivers/power/reset/gpio-restart.c using the bindings
from Documentation/devicetree/bindings/power/reset/gpio-restart.txt
To achieve this, first make sure microblaze makes use of
the standard kernel restart path utilizing do_kernel_restart()
from <linux/reboot.h>. Put in some grace time and an
emergency print if the restart does not properly assert.
As this is basic platform functionality we patch the DTS
file and defconfig in one go for a lockstep change.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Hi Michal, would be great if you could test and fix this up
so I can get rid of one more site where legacy GPIO is used.
I am unsure of the appropriate polarity and delays, hence the
comments in the DTS file.
---
arch/microblaze/boot/dts/system.dts | 16 +++++-
arch/microblaze/configs/mmu_defconfig | 2 +
arch/microblaze/configs/nommu_defconfig | 2 +
arch/microblaze/include/asm/setup.h | 1 -
arch/microblaze/kernel/reset.c | 76 -------------------------
arch/microblaze/mm/init.c | 11 ++++
6 files changed, 30 insertions(+), 78 deletions(-)
diff --git a/arch/microblaze/boot/dts/system.dts b/arch/microblaze/boot/dts/system.dts
index 5a8a9d090c37..c4e8bed50cb0 100644
--- a/arch/microblaze/boot/dts/system.dts
+++ b/arch/microblaze/boot/dts/system.dts
@@ -18,7 +18,6 @@
#address-cells = <1>;
#size-cells = <1>;
compatible = "xlnx,microblaze";
- hard-reset-gpios = <&LEDs_8Bit 2 1>;
model = "testing";
DDR2_SDRAM: memory@90000000 {
device_type = "memory";
@@ -281,6 +280,21 @@
gpios = <&LEDs_8Bit 7 1>;
};
} ;
+
+ gpio-restart {
+ compatible = "gpio-restart";
+ /*
+ * FIXME: is this active low or active high?
+ * the current flag (1) indicates active low.
+ * delay measures are templates, should be adjusted
+ * to datasheet or trial-and-error with real hardware.
+ */
+ hard-reset-gpios = <&LEDs_8Bit 2 1>;
+ active-delay = <100>;
+ inactive-delay = <10>;
+ wait-delay = <100>;
+ };
+
RS232_Uart_1: serial@84000000 {
clock-frequency = <125000000>;
compatible = "xlnx,xps-uartlite-1.00.a";
diff --git a/arch/microblaze/configs/mmu_defconfig b/arch/microblaze/configs/mmu_defconfig
index 92fd4e95b488..ae8d7d407ff4 100644
--- a/arch/microblaze/configs/mmu_defconfig
+++ b/arch/microblaze/configs/mmu_defconfig
@@ -59,6 +59,8 @@ CONFIG_SPI_XILINX=y
CONFIG_GPIOLIB=y
CONFIG_GPIO_SYSFS=y
CONFIG_GPIO_XILINX=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_GPIO_RESTART=y
# CONFIG_HWMON is not set
CONFIG_WATCHDOG=y
CONFIG_XILINX_WATCHDOG=y
diff --git a/arch/microblaze/configs/nommu_defconfig b/arch/microblaze/configs/nommu_defconfig
index 06d69a6e192d..a2a6be511551 100644
--- a/arch/microblaze/configs/nommu_defconfig
+++ b/arch/microblaze/configs/nommu_defconfig
@@ -62,6 +62,8 @@ CONFIG_SPI_XILINX=y
CONFIG_GPIOLIB=y
CONFIG_GPIO_SYSFS=y
CONFIG_GPIO_XILINX=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_GPIO_RESTART=y
# CONFIG_HWMON is not set
CONFIG_WATCHDOG=y
CONFIG_XILINX_WATCHDOG=y
diff --git a/arch/microblaze/include/asm/setup.h b/arch/microblaze/include/asm/setup.h
index ce9b7b786156..54d634ed98e6 100644
--- a/arch/microblaze/include/asm/setup.h
+++ b/arch/microblaze/include/asm/setup.h
@@ -29,7 +29,6 @@ void machine_early_init(const char *cmdline, unsigned int ram,
unsigned int fdt, unsigned int msr, unsigned int tlb0,
unsigned int tlb1);
-void machine_restart(char *cmd);
void machine_shutdown(void);
void machine_halt(void);
void machine_power_off(void);
diff --git a/arch/microblaze/kernel/reset.c b/arch/microblaze/kernel/reset.c
index fcbe1daf6316..b56af4eb91bf 100644
--- a/arch/microblaze/kernel/reset.c
+++ b/arch/microblaze/kernel/reset.c
@@ -10,82 +10,6 @@
#include <linux/init.h>
#include <linux/of_platform.h>
-/* Trigger specific functions */
-#ifdef CONFIG_GPIOLIB
-
-#include <linux/of_gpio.h>
-
-static int handle; /* reset pin handle */
-static unsigned int reset_val;
-
-static int of_platform_reset_gpio_probe(void)
-{
- int ret;
- handle = of_get_named_gpio(of_find_node_by_path("/"),
- "hard-reset-gpios", 0);
-
- if (!gpio_is_valid(handle)) {
- pr_info("Skipping unavailable RESET gpio %d (%s)\n",
- handle, "reset");
- return -ENODEV;
- }
-
- ret = gpio_request(handle, "reset");
- if (ret < 0) {
- pr_info("GPIO pin is already allocated\n");
- return ret;
- }
-
- /* get current setup value */
- reset_val = gpio_get_value(handle);
- /* FIXME maybe worth to perform any action */
- pr_debug("Reset: Gpio output state: 0x%x\n", reset_val);
-
- /* Setup GPIO as output */
- ret = gpio_direction_output(handle, 0);
- if (ret < 0)
- goto err;
-
- /* Setup output direction */
- gpio_set_value(handle, 0);
-
- pr_info("RESET: Registered gpio device: %d, current val: %d\n",
- handle, reset_val);
- return 0;
-err:
- gpio_free(handle);
- return ret;
-}
-device_initcall(of_platform_reset_gpio_probe);
-
-
-static void gpio_system_reset(void)
-{
- if (gpio_is_valid(handle))
- gpio_set_value(handle, 1 - reset_val);
- else
- pr_notice("Reset GPIO unavailable - halting!\n");
-}
-#else
-static void gpio_system_reset(void)
-{
- pr_notice("No reset GPIO present - halting!\n");
-}
-
-void of_platform_reset_gpio_probe(void)
-{
- return;
-}
-#endif
-
-void machine_restart(char *cmd)
-{
- pr_notice("Machine restart...\n");
- gpio_system_reset();
- while (1)
- ;
-}
-
void machine_shutdown(void)
{
pr_notice("Machine shutdown...\n");
diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c
index a015a951c8b7..4a45e037107f 100644
--- a/arch/microblaze/mm/init.c
+++ b/arch/microblaze/mm/init.c
@@ -17,6 +17,8 @@
#include <linux/slab.h>
#include <linux/swap.h>
#include <linux/export.h>
+#include <linux/delay.h>
+#include <linux/reboot.h>
#include <asm/page.h>
#include <asm/mmu_context.h>
@@ -265,6 +267,15 @@ static void __init mmu_init_hw(void)
: : : "r11");
}
+void machine_restart(char *cmd)
+{
+ do_kernel_restart(cmd);
+ /* Give the restart hook 1 s to take us down */
+ mdelay(1000);
+ pr_emerg("Reboot failed -- System halted\n");
+ while (1);
+}
+
/*
* MMU_init sets up the basic memory mappings for the kernel,
* including both RAM and possibly some I/O regions,
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] pinctrl: st: Add of_node_put() before return
From: Linus Walleij @ 2019-08-21 10:04 UTC (permalink / raw)
To: Nishka Dasgupta; +Cc: Patrice CHOTARD, Linux ARM, open list:GPIO SUBSYSTEM
In-Reply-To: <20190815060609.3056-1-nishkadg.linux@gmail.com>
On Thu, Aug 15, 2019 at 8:06 AM Nishka Dasgupta
<nishkadg.linux@gmail.com> wrote:
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak. Hence add an of_node_put before the return in
> three places.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [pinctrl:for-next 54/63] drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c:2325:9: error: initialization from incompatible pointer type
From: Linus Walleij @ 2019-08-21 11:36 UTC (permalink / raw)
To: Andrew Jeffery
Cc: kbuild test robot, Nathan Chancellor, kbuild-all,
open list:GPIO SUBSYSTEM
In-Reply-To: <15f11bd1-cde0-4345-8daf-234d61ebc9c0@www.fastmail.com>
On Wed, Aug 21, 2019 at 3:09 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> This is resolved by back-merging the pinctrl-v5.3-2 into pinctrl/devel
> or pinctrl/for-next as mentioned in the thread on Nathan's patch.
OK I merged v5.3-rc5 into my devel branch, that should cut it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] m68k: coldfire: Include the GPIO driver header
From: Greg Ungerer @ 2019-08-21 12:22 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-m68k
In-Reply-To: <CAMuHMdWF1GXYtFbjrALRMObqzezd-cBwDPAqhC-9d=RbrLxNyQ@mail.gmail.com>
On 21/8/19 5:19 pm, Geert Uytterhoeven wrote:
> CC Greg (coldfire)
Thanks Geert.
I am happy to take it via the m68knommu tree if you prefer?
Regards
Greg
> On Wed, Aug 21, 2019 at 9:09 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>> The Coldfire GPIO driver needs to explicitly incldue the
>> GPIO driver header since it is providing a driver.
>>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> Geert can you pick this up for m68k?
>> ---
>> arch/m68k/coldfire/gpio.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/m68k/coldfire/gpio.c b/arch/m68k/coldfire/gpio.c
>> index a83898426127..ca26de257871 100644
>> --- a/arch/m68k/coldfire/gpio.c
>> +++ b/arch/m68k/coldfire/gpio.c
>> @@ -9,6 +9,7 @@
>> #include <linux/module.h>
>> #include <linux/init.h>
>> #include <linux/device.h>
>> +#include <linux/gpio/driver.h>
>>
>> #include <linux/io.h>
>> #include <asm/coldfire.h>
>> --
>> 2.21.0
>
^ permalink raw reply
* Re: [PATCH] m68k: coldfire: Include the GPIO driver header
From: Geert Uytterhoeven @ 2019-08-21 12:50 UTC (permalink / raw)
To: Greg Ungerer
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
linux-m68k
In-Reply-To: <fb01f312-5cc5-03a1-a1a5-a12819e2ff7b@linux-m68k.org>
Hi Greg,
On Wed, Aug 21, 2019 at 2:22 PM Greg Ungerer <gerg@linux-m68k.org> wrote:
> On 21/8/19 5:19 pm, Geert Uytterhoeven wrote:
> > CC Greg (coldfire)
>
> Thanks Geert.
> I am happy to take it via the m68knommu tree if you prefer?
Sounds most logical to me.
Thanks!
> > On Wed, Aug 21, 2019 at 9:09 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >> The Coldfire GPIO driver needs to explicitly incldue the
> >> GPIO driver header since it is providing a driver.
> >>
> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> ---
> >> Geert can you pick this up for m68k?
> >> ---
> >> arch/m68k/coldfire/gpio.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/m68k/coldfire/gpio.c b/arch/m68k/coldfire/gpio.c
> >> index a83898426127..ca26de257871 100644
> >> --- a/arch/m68k/coldfire/gpio.c
> >> +++ b/arch/m68k/coldfire/gpio.c
> >> @@ -9,6 +9,7 @@
> >> #include <linux/module.h>
> >> #include <linux/init.h>
> >> #include <linux/device.h>
> >> +#include <linux/gpio/driver.h>
> >>
> >> #include <linux/io.h>
> >> #include <asm/coldfire.h>
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] RFT: microblaze: Switch to standard restart handler
From: Michal Simek @ 2019-08-21 14:03 UTC (permalink / raw)
To: Linus Walleij, linux-gpio
Cc: Bartosz Golaszewski, Arnd Bergmann, Michal Simek
In-Reply-To: <20190821083320.4788-1-linus.walleij@linaro.org>
[-- Attachment #1.1: Type: text/plain, Size: 8306 bytes --]
Hi Linus,
On 21. 08. 19 10:33, Linus Walleij wrote:
> The microblaze uses the legacy APIs to dig out a GPIO pin
> defined in the root of the device tree to issue a hard
> reset of the platform.
>
> Asserting a hard reset should be done using the standard
> DT-enabled and fully GPIO descriptor aware driver in
> drivers/power/reset/gpio-restart.c using the bindings
> from Documentation/devicetree/bindings/power/reset/gpio-restart.txt
>
> To achieve this, first make sure microblaze makes use of
> the standard kernel restart path utilizing do_kernel_restart()
> from <linux/reboot.h>. Put in some grace time and an
> emergency print if the restart does not properly assert.
>
> As this is basic platform functionality we patch the DTS
> file and defconfig in one go for a lockstep change.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Michal Simek <monstr@monstr.eu>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Hi Michal, would be great if you could test and fix this up
> so I can get rid of one more site where legacy GPIO is used.
> I am unsure of the appropriate polarity and delays, hence the
> comments in the DTS file.
> ---
> arch/microblaze/boot/dts/system.dts | 16 +++++-
> arch/microblaze/configs/mmu_defconfig | 2 +
> arch/microblaze/configs/nommu_defconfig | 2 +
> arch/microblaze/include/asm/setup.h | 1 -
> arch/microblaze/kernel/reset.c | 76 -------------------------
> arch/microblaze/mm/init.c | 11 ++++
> 6 files changed, 30 insertions(+), 78 deletions(-)
>
> diff --git a/arch/microblaze/boot/dts/system.dts b/arch/microblaze/boot/dts/system.dts
> index 5a8a9d090c37..c4e8bed50cb0 100644
> --- a/arch/microblaze/boot/dts/system.dts
> +++ b/arch/microblaze/boot/dts/system.dts
> @@ -18,7 +18,6 @@
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "xlnx,microblaze";
> - hard-reset-gpios = <&LEDs_8Bit 2 1>;
> model = "testing";
> DDR2_SDRAM: memory@90000000 {
> device_type = "memory";
> @@ -281,6 +280,21 @@
> gpios = <&LEDs_8Bit 7 1>;
> };
> } ;
> +
> + gpio-restart {
> + compatible = "gpio-restart";
> + /*
> + * FIXME: is this active low or active high?
> + * the current flag (1) indicates active low.
> + * delay measures are templates, should be adjusted
> + * to datasheet or trial-and-error with real hardware.
> + */
> + hard-reset-gpios = <&LEDs_8Bit 2 1>;
> + active-delay = <100>;
> + inactive-delay = <10>;
> + wait-delay = <100>;
> + };
> +
> RS232_Uart_1: serial@84000000 {
> clock-frequency = <125000000>;
> compatible = "xlnx,xps-uartlite-1.00.a";
> diff --git a/arch/microblaze/configs/mmu_defconfig b/arch/microblaze/configs/mmu_defconfig
> index 92fd4e95b488..ae8d7d407ff4 100644
> --- a/arch/microblaze/configs/mmu_defconfig
> +++ b/arch/microblaze/configs/mmu_defconfig
> @@ -59,6 +59,8 @@ CONFIG_SPI_XILINX=y
> CONFIG_GPIOLIB=y
> CONFIG_GPIO_SYSFS=y
> CONFIG_GPIO_XILINX=y
> +CONFIG_POWER_RESET=y
> +CONFIG_POWER_RESET_GPIO_RESTART=y
> # CONFIG_HWMON is not set
> CONFIG_WATCHDOG=y
> CONFIG_XILINX_WATCHDOG=y
> diff --git a/arch/microblaze/configs/nommu_defconfig b/arch/microblaze/configs/nommu_defconfig
> index 06d69a6e192d..a2a6be511551 100644
> --- a/arch/microblaze/configs/nommu_defconfig
> +++ b/arch/microblaze/configs/nommu_defconfig
> @@ -62,6 +62,8 @@ CONFIG_SPI_XILINX=y
> CONFIG_GPIOLIB=y
> CONFIG_GPIO_SYSFS=y
> CONFIG_GPIO_XILINX=y
> +CONFIG_POWER_RESET=y
> +CONFIG_POWER_RESET_GPIO_RESTART=y
> # CONFIG_HWMON is not set
> CONFIG_WATCHDOG=y
> CONFIG_XILINX_WATCHDOG=y
> diff --git a/arch/microblaze/include/asm/setup.h b/arch/microblaze/include/asm/setup.h
> index ce9b7b786156..54d634ed98e6 100644
> --- a/arch/microblaze/include/asm/setup.h
> +++ b/arch/microblaze/include/asm/setup.h
> @@ -29,7 +29,6 @@ void machine_early_init(const char *cmdline, unsigned int ram,
> unsigned int fdt, unsigned int msr, unsigned int tlb0,
> unsigned int tlb1);
>
> -void machine_restart(char *cmd);
> void machine_shutdown(void);
> void machine_halt(void);
> void machine_power_off(void);
> diff --git a/arch/microblaze/kernel/reset.c b/arch/microblaze/kernel/reset.c
> index fcbe1daf6316..b56af4eb91bf 100644
> --- a/arch/microblaze/kernel/reset.c
> +++ b/arch/microblaze/kernel/reset.c
> @@ -10,82 +10,6 @@
> #include <linux/init.h>
> #include <linux/of_platform.h>
>
> -/* Trigger specific functions */
> -#ifdef CONFIG_GPIOLIB
> -
> -#include <linux/of_gpio.h>
> -
> -static int handle; /* reset pin handle */
> -static unsigned int reset_val;
> -
> -static int of_platform_reset_gpio_probe(void)
> -{
> - int ret;
> - handle = of_get_named_gpio(of_find_node_by_path("/"),
> - "hard-reset-gpios", 0);
> -
> - if (!gpio_is_valid(handle)) {
> - pr_info("Skipping unavailable RESET gpio %d (%s)\n",
> - handle, "reset");
> - return -ENODEV;
> - }
> -
> - ret = gpio_request(handle, "reset");
> - if (ret < 0) {
> - pr_info("GPIO pin is already allocated\n");
> - return ret;
> - }
> -
> - /* get current setup value */
> - reset_val = gpio_get_value(handle);
> - /* FIXME maybe worth to perform any action */
> - pr_debug("Reset: Gpio output state: 0x%x\n", reset_val);
> -
> - /* Setup GPIO as output */
> - ret = gpio_direction_output(handle, 0);
> - if (ret < 0)
> - goto err;
> -
> - /* Setup output direction */
> - gpio_set_value(handle, 0);
> -
> - pr_info("RESET: Registered gpio device: %d, current val: %d\n",
> - handle, reset_val);
> - return 0;
> -err:
> - gpio_free(handle);
> - return ret;
> -}
> -device_initcall(of_platform_reset_gpio_probe);
> -
> -
> -static void gpio_system_reset(void)
> -{
> - if (gpio_is_valid(handle))
> - gpio_set_value(handle, 1 - reset_val);
> - else
> - pr_notice("Reset GPIO unavailable - halting!\n");
> -}
> -#else
> -static void gpio_system_reset(void)
> -{
> - pr_notice("No reset GPIO present - halting!\n");
> -}
> -
> -void of_platform_reset_gpio_probe(void)
> -{
> - return;
> -}
> -#endif
> -
> -void machine_restart(char *cmd)
> -{
> - pr_notice("Machine restart...\n");
> - gpio_system_reset();
> - while (1)
> - ;
> -}
> -
> void machine_shutdown(void)
> {
> pr_notice("Machine shutdown...\n");
> diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c
> index a015a951c8b7..4a45e037107f 100644
> --- a/arch/microblaze/mm/init.c
> +++ b/arch/microblaze/mm/init.c
> @@ -17,6 +17,8 @@
> #include <linux/slab.h>
> #include <linux/swap.h>
> #include <linux/export.h>
> +#include <linux/delay.h>
> +#include <linux/reboot.h>
>
> #include <asm/page.h>
> #include <asm/mmu_context.h>
> @@ -265,6 +267,15 @@ static void __init mmu_init_hw(void)
> : : : "r11");
> }
>
> +void machine_restart(char *cmd)
> +{
> + do_kernel_restart(cmd);
> + /* Give the restart hook 1 s to take us down */
> + mdelay(1000);
> + pr_emerg("Reboot failed -- System halted\n");
> + while (1);
> +}
> +
> /*
> * MMU_init sets up the basic memory mappings for the kernel,
> * including both RAM and possibly some I/O regions,
>
This should got to kernel/reset.c.
Anyway I have tested it and I have issue to use restart-gpio driver and
I am using gpio-xilinx driver which drives that pin.
[ 6.475449] restart-gpio amba_pl:gpio-restart: GPIO lookup for
consumer (null)
[ 6.482887] restart-gpio amba_pl:gpio-restart: using device tree for
GPIO lookup
[ 6.495623] restart-gpio amba_pl:gpio-restart: using lookup tables
for GPIO lookup
[ 6.505016] restart-gpio amba_pl:gpio-restart: No GPIO consumer
(null) found
[ 6.512275] restart-gpio amba_pl:gpio-restart: Could net get reset GPIO
[ 6.519076] restart-gpio: probe of amba_pl:gpio-restart failed with
error -2
Is there an issue with gpio-xilinx driver that of_find_gpio() is failing?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH] pinctrl: meson: meson: Add of_node_put() before return
From: Neil Armstrong @ 2019-08-21 14:24 UTC (permalink / raw)
To: Nishka Dasgupta, khilman, linus.walleij, linux-arm-kernel,
linux-gpio, linux-amlogic
In-Reply-To: <20190815060718.3286-1-nishkadg.linux@gmail.com>
On 15/08/2019 08:07, Nishka Dasgupta wrote:
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak. Hence add an of_node_put before the return.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> drivers/pinctrl/meson/pinctrl-meson.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 596786926209..8bba9d053d9f 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -651,6 +651,7 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
> continue;
> if (gpio_np) {
> dev_err(pc->dev, "multiple gpio nodes\n");
> + of_node_put(np);
> return -EINVAL;
> }
> gpio_np = np;
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [v7 2/2] gpio: aspeed: Add SGPIO driver
From: Hongwei Zhang @ 2019-08-21 14:25 UTC (permalink / raw)
To: Andrew Jeffery, Linus Walleij, linux-gpio
Cc: Hongwei Zhang, Joel Stanley, linux-aspeed, Bartosz Golaszewski,
linux-kernel, linux-arm-kernel
In-Reply-To: <1564603297-1391-3-git-send-email-hongweiz@ami.com>
Hello Linus,
Thanks for your review! I just submitted v8 to the list, please help to review it again.
Since you have already merged the dt-binding document [v7 1/2], and I don't have your
update to this file, so to avoid confusion, I only include the driver code in v8.
Regards,
--Hongwei
P.S. sorry previous response used wrong In-Reply-To ID, so resent it again.
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: Wednesday, August 14, 2019 4:09 AM
> To: Hongwei Zhang
> Cc: Andrew Jeffery; open list:GPIO SUBSYSTEM; Joel Stanley; linux-aspeed; Bartosz Golaszewski;
> linux-kernel@vger.kernel.org; Linux ARM
> Subject: Re: [v7 2/2] gpio: aspeed: Add SGPIO driver
>
> Hi Hongwei,
>
> thanks for your patch!
>
> I have now merged the bindings so you only need to respin this patch.
>
> On Wed, Jul 31, 2019 at 10:02 PM Hongwei Zhang <hongweiz@ami.com> wrote:
>
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
> > Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>
> I guess I need to go with this, there are some minor things I still want to be fixed:
>
> > +static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> > +offset, int val)
>
> I don't like __underscore_functions because their semantic is ambiguous.
>
done, please see v8.
> Rename this something like aspeed_sgpio_commit() or whatever best fits the actual use.
>
> > +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
> > + struct platform_device *pdev) {
> (...)
> > + rc = gpiochip_irqchip_add(&gpio->chip, &aspeed_sgpio_irqchip,
> > + 0, handle_bad_irq, IRQ_TYPE_NONE);
> (...)
> > + gpiochip_set_chained_irqchip(&gpio->chip, &aspeed_sgpio_irqchip,
> > + gpio->irq,
> > + aspeed_sgpio_irq_handler);
>
> We do not set up chained irqchips like this anymore, sorry.
>
> I am currently rewriting all existing chained drivers to pass an initialized irqchip when registering the
> whole gpio chip.
> See drivers/gpio/TODO.
>
> Here are examples:
> https://lore.kernel.org/linux-gpio/20190811080539.15647-1-linus.walleij@linaro.org/
> https://lore.kernel.org/linux-gpio/20190812132554.18313-1-linus.walleij@linaro.org/
>
done, please see v8.
> > + /* set all SGPIO pins as input (1). */
> > + memset(gpio->dir_in, 0xff, sizeof(gpio->dir_in));
>
> Do the irqchip set-up here, before adding the gpio_chip.
>
> > + rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return aspeed_sgpio_setup_irqs(gpio, pdev);
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: cp110: document the new CP115 pinctrl compatible
From: Rob Herring @ 2019-08-21 18:53 UTC (permalink / raw)
To: Miquel Raynal
Cc: Mark Rutland, devicetree, Thomas Petazzoni, Gregory Clement,
Antoine Tenart, Maxime Chevallier, Nadav Haklai, Linus Walleij,
linux-gpio, linux-arm-kernel, Grzegorz Jaszczyk, Marcin Wojtas,
Stefan Chulski, Yan Markman, Miquel Raynal
In-Reply-To: <20190805101607.29811-3-miquel.raynal@bootlin.com>
On Mon, 5 Aug 2019 12:16:06 +0200, Miquel Raynal wrote:
> From: Grzegorz Jaszczyk <jaz@semihalf.com>
>
> A new compatible is going to be used for Armada CP115 pinctrl block,
> document it.
>
> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> [<miquel.raynal@bootlin.com>: split the documentation out of the
> driver commit]
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../bindings/arm/marvell/cp110-system-controller.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 1/3] gpio: mpc8xxx: add ls1088a platform gpio node DT binding description
From: Rob Herring @ 2019-08-21 20:53 UTC (permalink / raw)
To: Hui Song
Cc: Shawn Guo, Li Yang, Mark Rutland, Linus Walleij,
Bartosz Golaszewski, linux-arm-kernel, devicetree, linux-kernel,
linux-gpio, Song Hui
In-Reply-To: <20190808101628.36782-1-hui.song_1@nxp.com>
On Thu, 8 Aug 2019 18:16:26 +0800, Hui Song wrote:
> From: Song Hui <hui.song_1@nxp.com>
>
> ls1088a and ls1028a platform share common gpio node.
>
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
> ---
> Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 5/6] dt-bindings: pinctrl: k3: Introduce pinmux definitions for J721E
From: Rob Herring @ 2019-08-21 21:02 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Tero Kristo, Nishanth Menon, linus.walleij, Keerthy, linux-gpio,
Device Tree Mailing List, Linux ARM Mailing List
In-Reply-To: <20190809082947.30590-6-lokeshvutla@ti.com>
On Fri, Aug 09, 2019 at 01:59:46PM +0530, Lokesh Vutla wrote:
> Add pinctrl macros for J721E SoC. These macro definitions are
> similar to that of AM6, but adding new definitions to avoid
> any naming confusions in the soc dts files.
>
> Acked-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> include/dt-bindings/pinctrl/k3.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/dt-bindings/pinctrl/k3.h b/include/dt-bindings/pinctrl/k3.h
> index 45e11b6170ca..499de6216581 100644
> --- a/include/dt-bindings/pinctrl/k3.h
> +++ b/include/dt-bindings/pinctrl/k3.h
> @@ -32,4 +32,7 @@
> #define AM65X_IOPAD(pa, val, muxmode) (((pa) & 0x1fff)) ((val) | (muxmode))
> #define AM65X_WKUP_IOPAD(pa, val, muxmode) (((pa) & 0x1fff)) ((val) | (muxmode))
>
> +#define J721E_IOPAD(pa, val, muxmode) (((pa) & 0x1fff)) ((val) | (muxmode))
> +#define J721E_WKUP_IOPAD(pa, val, muxmode) (((pa) & 0x1fff)) ((val) | (muxmode))
checkpatch reports a parentheses error: (((pa) & 0x1fff) ((val) | (muxmode)))
> +
> #endif
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH] m68k: coldfire: Include the GPIO driver header
From: Greg Ungerer @ 2019-08-22 0:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
linux-m68k
In-Reply-To: <CAMuHMdVsqKqppkvXYm=NiGeikhC_i99hH+Y2ecjQfr3S2-BTZA@mail.gmail.com>
Hi Geert, Linus,
On 21/8/19 10:50 pm, Geert Uytterhoeven wrote:
> On Wed, Aug 21, 2019 at 2:22 PM Greg Ungerer <gerg@linux-m68k.org> wrote:
>> On 21/8/19 5:19 pm, Geert Uytterhoeven wrote:
>>> CC Greg (coldfire)
>>
>> Thanks Geert.
>> I am happy to take it via the m68knommu tree if you prefer?
>
> Sounds most logical to me.
> Thanks!
Pushed to the for-next branch of the m68knommu git tree.
Regards
Greg
>>> On Wed, Aug 21, 2019 at 9:09 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>>>> The Coldfire GPIO driver needs to explicitly incldue the
>>>> GPIO driver header since it is providing a driver.
>>>>
>>>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>>> ---
>>>> Geert can you pick this up for m68k?
>>>> ---
>>>> arch/m68k/coldfire/gpio.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/m68k/coldfire/gpio.c b/arch/m68k/coldfire/gpio.c
>>>> index a83898426127..ca26de257871 100644
>>>> --- a/arch/m68k/coldfire/gpio.c
>>>> +++ b/arch/m68k/coldfire/gpio.c
>>>> @@ -9,6 +9,7 @@
>>>> #include <linux/module.h>
>>>> #include <linux/init.h>
>>>> #include <linux/device.h>
>>>> +#include <linux/gpio/driver.h>
>>>>
>>>> #include <linux/io.h>
>>>> #include <asm/coldfire.h>
>
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply
* Re: [pinctrl:for-next 54/63] drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c:2325:9: error: initialization from incompatible pointer type
From: Andrew Jeffery @ 2019-08-22 1:12 UTC (permalink / raw)
To: Linus Walleij
Cc: kbuild test robot, Nathan Chancellor, kbuild-all,
open list:GPIO SUBSYSTEM
In-Reply-To: <CACRpkdakewyaF4Xp6=2c_h1_T1kY-1MQ3bbZzNv2uHSCndHQgg@mail.gmail.com>
On Wed, 21 Aug 2019, at 21:06, Linus Walleij wrote:
> On Wed, Aug 21, 2019 at 3:09 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> > This is resolved by back-merging the pinctrl-v5.3-2 into pinctrl/devel
> > or pinctrl/for-next as mentioned in the thread on Nathan's patch.
>
> OK I merged v5.3-rc5 into my devel branch, that should cut it.
Yep, I just did a successful build of pinctrl/devel with multi_v5_defconfig and
CONFIG_COMPILE_TEST=y (the AST2600 arch support only just hit the lists),
looks good.
Thanks,
Andrew
^ permalink raw reply
* [PATCH] gpio: Move gpiochip_lock/unlock_as_irq to gpio/driver.h
From: YueHaibing @ 2019-08-22 3:18 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, acourbot
Cc: linux-kernel, linux-gpio, YueHaibing
If CONFIG_GPIOLIB is not, gpiochip_lock/unlock_as_irq will
conflict as this:
In file included from sound/soc/codecs/wm5100.c:18:0:
./include/linux/gpio.h:224:19: error: static declaration of gpiochip_lock_as_irq follows non-static declaration
static inline int gpiochip_lock_as_irq(struct gpio_chip *chip,
^~~~~~~~~~~~~~~~~~~~
In file included from sound/soc/codecs/wm5100.c:17:0:
./include/linux/gpio/driver.h:494:5: note: previous declaration of gpiochip_lock_as_irq was here
int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset);
^~~~~~~~~~~~~~~~~~~~
In file included from sound/soc/codecs/wm5100.c:18:0:
./include/linux/gpio.h:231:20: error: static declaration of gpiochip_unlock_as_irq follows non-static declaration
static inline void gpiochip_unlock_as_irq(struct gpio_chip *chip,
^~~~~~~~~~~~~~~~~~~~~~
In file included from sound/soc/codecs/wm5100.c:17:0:
./include/linux/gpio/driver.h:495:6: note: previous declaration of gpiochip_unlock_as_irq was here
void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset);
^~~~~~~~~~~~~~~~~~~~~~
Move them to gpio/driver.h and use CONFIG_GPIOLIB guard this.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: d74be6dfea1b ("gpio: remove gpiod_lock/unlock_as_irq()")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
include/linux/gpio.h | 13 -------------
include/linux/gpio/driver.h | 19 ++++++++++++++++---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index f757a58..2157717 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -221,19 +221,6 @@ static inline int gpio_to_irq(unsigned gpio)
return -EINVAL;
}
-static inline int gpiochip_lock_as_irq(struct gpio_chip *chip,
- unsigned int offset)
-{
- WARN_ON(1);
- return -EINVAL;
-}
-
-static inline void gpiochip_unlock_as_irq(struct gpio_chip *chip,
- unsigned int offset)
-{
- WARN_ON(1);
-}
-
static inline int irq_to_gpio(unsigned irq)
{
/* irq can never have been returned from gpio_to_irq() */
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 4d2d7b7..1778106 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -490,9 +490,6 @@ extern int devm_gpiochip_add_data(struct device *dev, struct gpio_chip *chip,
extern struct gpio_chip *gpiochip_find(void *data,
int (*match)(struct gpio_chip *chip, void *data));
-/* lock/unlock as IRQ */
-int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset);
-void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset);
bool gpiochip_line_is_irq(struct gpio_chip *chip, unsigned int offset);
int gpiochip_reqres_irq(struct gpio_chip *chip, unsigned int offset);
void gpiochip_relres_irq(struct gpio_chip *chip, unsigned int offset);
@@ -710,6 +707,10 @@ void devprop_gpiochip_set_names(struct gpio_chip *chip,
struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc);
+/* lock/unlock as IRQ */
+int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset);
+void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset);
+
#else /* CONFIG_GPIOLIB */
static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
@@ -719,6 +720,18 @@ static inline struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
return ERR_PTR(-ENODEV);
}
+static inline int gpiochip_lock_as_irq(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ WARN_ON(1);
+ return -EINVAL;
+}
+
+static inline void gpiochip_unlock_as_irq(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ WARN_ON(1);
+}
#endif /* CONFIG_GPIOLIB */
#endif /* __LINUX_GPIO_DRIVER_H */
--
2.7.4
^ permalink raw reply related
* [PATCH v1] MAINTAINERS: Remove staled record for gpio-intel-mid.c
From: Andy Shevchenko @ 2019-08-22 8:42 UTC (permalink / raw)
To: Dave Hansen, linux-gpio, Linus Walleij, Bartosz Golaszewski
Cc: Andy Shevchenko
David Cohen seems left Intel few years back. Remove the staled record in
MAINTAINERS data base. The file is anyway listed under my maintainership.
Reported-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
MAINTAINERS | 6 ------
1 file changed, 6 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 33e6b9e2da9e..3c8cc45c3cc7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8446,12 +8446,6 @@ F: Documentation/x86/intel_txt.rst
F: include/linux/tboot.h
F: arch/x86/kernel/tboot.c
-INTEL-MID GPIO DRIVER
-M: David Cohen <david.a.cohen@linux.intel.com>
-L: linux-gpio@vger.kernel.org
-S: Maintained
-F: drivers/gpio/gpio-intel-mid.c
-
INTERCONNECT API
M: Georgi Djakov <georgi.djakov@linaro.org>
L: linux-pm@vger.kernel.org
--
2.23.0.rc1
^ permalink raw reply related
* Re: [PATCH v1] MAINTAINERS: Remove staled record for gpio-intel-mid.c
From: Andy Shevchenko @ 2019-08-22 8:51 UTC (permalink / raw)
To: Dave Hansen, linux-gpio, Linus Walleij, Bartosz Golaszewski
In-Reply-To: <20190822084252.61856-1-andriy.shevchenko@linux.intel.com>
On Thu, Aug 22, 2019 at 11:42:52AM +0300, Andy Shevchenko wrote:
> David Cohen seems left Intel few years back. Remove the staled record in
> MAINTAINERS data base. The file is anyway listed under my maintainership.
This is supposed to go via gpio-intel tree.
>
> Reported-by: Dave Hansen <dave.hansen@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> MAINTAINERS | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 33e6b9e2da9e..3c8cc45c3cc7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8446,12 +8446,6 @@ F: Documentation/x86/intel_txt.rst
> F: include/linux/tboot.h
> F: arch/x86/kernel/tboot.c
>
> -INTEL-MID GPIO DRIVER
> -M: David Cohen <david.a.cohen@linux.intel.com>
> -L: linux-gpio@vger.kernel.org
> -S: Maintained
> -F: drivers/gpio/gpio-intel-mid.c
> -
> INTERCONNECT API
> M: Georgi Djakov <georgi.djakov@linaro.org>
> L: linux-pm@vger.kernel.org
> --
> 2.23.0.rc1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] Skip deferred request irqs for devices known to fail
From: Hans de Goede @ 2019-08-22 16:27 UTC (permalink / raw)
To: Ian W MORRISON, benjamin.tissoires, mika.westerberg,
andriy.shevchenko, linus.walleij, bgolaszewski
Cc: linux-gpio, linux-acpi, linux-kernel, stable
In-Reply-To: <1bd012ca-9b2f-78c3-abb1-6b5680add404@redhat.com>
Hi All,
On 19-08-19 13:31, Hans de Goede wrote:
> Also I might be able to get my hands on a Minix Neo Z83-4 myself
> in a couple of days and then I can try to reproduce this, so lets
> wait a bit for that and see how that goes.
So I've access to a Minix Neo z83-4 myself now. The problem is
the DSDT contains an _E03 handler on the second (INT33FF UID 2)
GPIO controller which is clearly copy pasted from some DSDT
from a tablet as it deals with the ID pin of the micro-usb
connector, which the Minix Neo z83-4 mini-PC does not have.
This _E03 method switches the XHCI role switch between
host and device roles (those data lines are nor used, so don't
care) *and* for some reason it sets GN66 to 0 or 1, with GN66
being defined as:
Connection (
GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
"\\_SB.GPO1", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0042
}
),
This leads to the following difference in a pinctrl debug dump
between a good (running of ACPI edge GPIO handlers at boot disabled)
and bad run:
@@ -51,7 +51,7 @@
pin 63 (PANEL1_BKLTCTL) GPIO 0x00008102 0x04c00000
pin 64 (HV_DDI1_HPD) mode 1 0x03010000 0x04c00020
pin 65 (PANEL0_BKLTCTL) GPIO 0x30008202 0x04c00003
-pin 66 (HV_DDI0_DDC_SDA) GPIO 0x00018000 0x04c00000
+pin 66 (HV_DDI0_DDC_SDA) mode 1 0x00010001 0x04c00000
pin 67 (HV_DDI2_DDC_SCL) mode 3 0x00930301 0x04c00000
pin 68 (HV_DDI2_HPD) mode 1 0x03010001 0x04c00020
pin 69 (PANEL1_VDDEN) GPIO 0x00008102 0x04c00000
With a bad run ssh still works, basically everything still works except
for DDC on the HDMI conector which is causing the blackscreen.
Through ssh I could get the above pinctrl difference and
also see this new errors in the logs:
kernel: i915 0000:00:02.0: HDMI-A-1: EDID is invalid:
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [00] ZERO 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
kernel: [drm] Cannot find any crtc or sizes
kernel: [drm] Cannot find any crtc or sizes
Which matches with the DDC data pin being changes from connected
to the DDC i2c-controller into a generic (G)PIO
So this is really a case of a broken DSDT I am afraid and as such
the DMI blacklist seems the best (least bad) we can do.
But I do not believe that the current patch is a good fix, this problem
first surfaced when we started running edge ACPI GPIO event handlers at
boot to ensure that any state which is set by the handler matches the
current value of the pin. So that e.g. USB host/device role switches are
set the right value.
Where as the fix proposed by Ian, disabled us from registering a
handler all together, not only for the troublesome _E03 (which will
never trigger normally since there is no id-pin), but also for the
e.g. the INT0002 vgpio device.
And not registering a handler for the INT0002 vgpio device causes
an interrupt storm on irq 9, although for some reason that storm
stops after a 100000 interrupts or so on the Minix Neo Z83-4.
which is different from other devices where it never stops and we
get millions of interrupts.
So I believe a better fix would be to:
1) Add a kernel parameter to disable the run of edge ACPI
GPIO events at startup:
gpiolib_acpi_run_edge_events_on_startup
2) Make this default to auto which uses a DMI blacklist
This will allow us to easily test for similar problems on other
hardware and it fixes the issue at hand without disabling all
ACPI GPIO event handlers.
I will prep a patch implementing this approach sometime this
weekend.
Regards,
Hans
>> ---
>> drivers/gpio/gpiolib-acpi.c | 33 +++++++++++++++++++++++++++------
>> 1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
>> index fdee8afa5339..f6c3dcdc91c9 100644
>> --- a/drivers/gpio/gpiolib-acpi.c
>> +++ b/drivers/gpio/gpiolib-acpi.c
>> @@ -13,6 +13,7 @@
>> #include <linux/gpio/machine.h>
>> #include <linux/export.h>
>> #include <linux/acpi.h>
>> +#include <linux/dmi.h>
>> #include <linux/interrupt.h>
>> #include <linux/mutex.h>
>> #include <linux/pinctrl/pinctrl.h>
>> @@ -20,6 +21,17 @@
>> #include "gpiolib.h"
>> #include "gpiolib-acpi.h"
>> +static const struct dmi_system_id skip_deferred_request_irqs_table[] = {
>> + {
>> + .ident = "MINIX Z83-4",
>> + .matches = {
>> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "MINIX"),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
>> + },
>> + },
>> + {}
>> +};
>> +
>> /**
>> * struct acpi_gpio_event - ACPI GPIO event handler data
>> *
>> @@ -1273,19 +1285,28 @@ bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
>> return con_id == NULL;
>> }
>> -/* Run deferred acpi_gpiochip_request_irqs() */
>> +/*
>> + * Run deferred acpi_gpiochip_request_irqs()
>> + * but exclude devices known to fail
>> +*/
>> static int acpi_gpio_handle_deferred_request_irqs(void)
>> {
>> struct acpi_gpio_chip *acpi_gpio, *tmp;
>> + const struct dmi_system_id *dmi_id;
>> - mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
>> - list_for_each_entry_safe(acpi_gpio, tmp,
>> + dmi_id = dmi_first_match(skip_deferred_request_irqs_table);
>> + if (dmi_id)
>> + return 0;
>> + else {
>> + mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
>> + list_for_each_entry_safe(acpi_gpio, tmp,
>> &acpi_gpio_deferred_req_irqs_list,
>> deferred_req_irqs_list_entry)
>> - acpi_gpiochip_request_irqs(acpi_gpio);
>> + acpi_gpiochip_request_irqs(acpi_gpio);
>> - acpi_gpio_deferred_req_irqs_done = true;
>> - mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
>> + acpi_gpio_deferred_req_irqs_done = true;
>> + mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
>> + }
>> return 0;
>> }
>>
^ permalink raw reply
* [PATCH] gpio: ftgpio: Fix an error handling path in 'ftgpio_gpio_probe()'
From: Christophe JAILLET @ 2019-08-22 20:45 UTC (permalink / raw)
To: linus.walleij, bgolaszewski
Cc: linux-gpio, linux-kernel, kernel-janitors, Christophe JAILLET
If 'devm_kcalloc()' fails, we should go through the error handling path,
should some clean-up be needed.
Fixes: 42d9fc7176eb ("gpio: ftgpio: Pass irqchip when adding gpiochip")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/gpio/gpio-ftgpio010.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-ftgpio010.c b/drivers/gpio/gpio-ftgpio010.c
index 250e71f3e688..1478259102be 100644
--- a/drivers/gpio/gpio-ftgpio010.c
+++ b/drivers/gpio/gpio-ftgpio010.c
@@ -290,8 +290,10 @@ static int ftgpio_gpio_probe(struct platform_device *pdev)
girq->num_parents = 1;
girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
GFP_KERNEL);
- if (!girq->parents)
- return -ENOMEM;
+ if (!girq->parents) {
+ ret = -ENOMEM;
+ goto dis_clk;
+ }
girq->default_type = IRQ_TYPE_NONE;
girq->handler = handle_bad_irq;
girq->parents[0] = irq;
--
2.20.1
^ permalink raw reply related
* gpiolib-acpi problematic trigger of edge events during boot
From: Daniel Drake @ 2019-08-23 2:59 UTC (permalink / raw)
To: Hans de Goede, Benjamin Tissoires
Cc: Endless Linux Upstreaming Team, open list:PIN CONTROL SUBSYSTEM,
ACPI Devel Maling List
Hi,
acpi_gpiochip_request_irq() has this code:
/* Make sure we trigger the initial state of edge-triggered IRQs */
value = gpiod_get_raw_value_cansleep(event->desc);
if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
((event->irqflags & IRQF_TRIGGER_FALLING) && value == 0))
event->handler(event->irq, event);
Originally introduced in:
commit ca876c7483b697b498868b1f575997191b077885
Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date: Thu Jul 12 17:25:06 2018 +0200
gpiolib-acpi: make sure we trigger edge events at least once on boot
This code is causing a problem on a new consumer laptop, which is
based on the new ACPI reduced hardware standard. Under this design,
the power button is now just an ordinary GPIO that should be handled
by software: ACPI's _AEI method lists this GPIO as one that the OS
should monitor for changes, and the OS is expected to call the
corresponding _EVT method when that happens, which will in turn raise
a Notify event on the power button device.
Here, the GpioInt defined in _AEI is Edge-triggered, ActiveHigh. The
GPIO level is ordinarily high, but goes low when the button is
pressed. We checked this definition and behaviour with the vendor,
even suggesting that it should maybe be ActiveLow instead, but they
responded that this is correct and by-design.
These conditions set the IRQF_TRIGGER_RISING flag and cause the _EVT
event handler to be called by the code above as soon as the pinctrl
module is loaded. In other words, loading the pinctrl driver causes
the system to incorrectly believe the power button has been pressed so
it will immediately go into suspend or shutdown.
Fortunately this is perhaps not a serious issue, as at least Ubuntu
and Endless build the corresponding pinctrl drivers directly into the
kernel image. They are then loaded in early boot, and despite a power
button event being reported, it's so early that userspace doesn't
notice and no action is taken.
But I raise this anyway as a potential problem should that ever
change, it may also become a more widespead issue as the ACPI reduced
hardware standard becomes more and more common in consumer devices.
Any ideas for how we can better deal with this issue?
I can see the rationale for handling the specific cases mentioned in
the original commit message, but at the same time this code seems to
be assuming that an edge transition has happened, which is not true in
this case.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH] gpio: Fix irqchip initialization order
From: Linus Walleij @ 2019-08-23 6:48 UTC (permalink / raw)
To: open list:GPIO SUBSYSTEM, Wei Xu
Cc: Bartosz Golaszewski, Thierry Reding, Grygorii Strashko,
Andy Shevchenko
In-Reply-To: <20190820080527.11796-1-linus.walleij@linaro.org>
On Tue, Aug 20, 2019 at 10:05 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> Wei: it would be great if you could test this and
> confirm if it solves your problem, so I can apply this
> for fixes.
Wei, could you test this patch? I have a strong urge
to push it to Torvalds as a fix for this problem but it'd
be super if you could test it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] gpio: mt7621: Pass irqchip when adding gpiochip
From: René van Dorst @ 2019-08-23 6:56 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Bartosz Golaszewski, Greg Ungerer, Nicholas Mc Guire,
Sergio Paracuellos, Thierry Reding
In-Reply-To: <20190809141116.16403-1-linus.walleij@linaro.org>
Hi Linus,
Sorry for the late reply.
Quoting Linus Walleij <linus.walleij@linaro.org>:
> We need to convert all old gpio irqchips to pass the irqchip
> setup along when adding the gpio_chip. For more info see
> drivers/gpio/TODO.
>
> For chained irqchips this is a pretty straight-forward
> conversion.
>
> This driver requests the IRQ directly in the driver so it
> differs a bit from the others.
>
> Cc: René van Dorst <opensource@vdorst.com>
> Cc: Greg Ungerer <gerg@kernel.org>
> Cc: Nicholas Mc Guire <hofrat@osadl.org>
> Cc: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpio-mt7621.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
> index 79654fb2e50f..d1d785f983a7 100644
> --- a/drivers/gpio/gpio-mt7621.c
> +++ b/drivers/gpio/gpio-mt7621.c
> @@ -241,13 +241,6 @@ mediatek_gpio_bank_probe(struct device *dev,
> if (!rg->chip.label)
> return -ENOMEM;
>
> - ret = devm_gpiochip_add_data(dev, &rg->chip, mtk);
> - if (ret < 0) {
> - dev_err(dev, "Could not register gpio %d, ret=%d\n",
> - rg->chip.ngpio, ret);
> - return ret;
> - }
> -
> rg->irq_chip.name = dev_name(dev);
> rg->irq_chip.parent_device = dev;
> rg->irq_chip.irq_unmask = mediatek_gpio_irq_unmask;
> @@ -256,8 +249,10 @@ mediatek_gpio_bank_probe(struct device *dev,
> rg->irq_chip.irq_set_type = mediatek_gpio_irq_type;
>
> if (mtk->gpio_irq) {
> + struct gpio_irq_chip *girq;
> +
> /*
> - * Manually request the irq here instead of passing
> + * Directly request the irq here instead of passing
> * a flow-handler to gpiochip_set_chained_irqchip,
> * because the irq is shared.
> */
> @@ -271,15 +266,21 @@ mediatek_gpio_bank_probe(struct device *dev,
> return ret;
> }
>
> - ret = gpiochip_irqchip_add(&rg->chip, &rg->irq_chip,
> - 0, handle_simple_irq, IRQ_TYPE_NONE);
> - if (ret) {
> - dev_err(dev, "failed to add gpiochip_irqchip\n");
> - return ret;
> - }
> + girq = &rg->chip.irq;
> + girq->chip = &rg->irq_chip;
> + /* This will let us handle the parent IRQ in the driver */
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->handler = handle_simple_irq;
> + }
>
> - gpiochip_set_chained_irqchip(&rg->chip, &rg->irq_chip,
> - mtk->gpio_irq, NULL);
> + ret = devm_gpiochip_add_data(dev, &rg->chip, mtk);
> + if (ret < 0) {
> + dev_err(dev, "Could not register gpio %d, ret=%d\n",
> + rg->chip.ngpio, ret);
> + return ret;
> }
>
> /* set polarity to low for all gpios */
> --
> 2.21.0
I have this patch applied for a while.
I didn't encounter an issues.
Tested-by: René van Dorst <opensource@vdorst.com>
Greats,
René
^ permalink raw reply
* Re: [PATCH v2 3/3] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Geert Uytterhoeven @ 2019-08-23 7:10 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, open list:GPIO SUBSYSTEM,
Linux-Renesas
In-Reply-To: <CAMuHMdW32TRw3Awf-5C2eJiZ1iys-vK7YihFwqPxOP66Eh9+Lg@mail.gmail.com>
On Thu, Aug 8, 2019 at 10:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Aug 8, 2019 at 8:20 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > R-Car PWM controller requires the gpio to output zero duty,
> > this patch allows to roll it back from gpio to mux when the gpio
> > is freed.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
FTR: as the PWM work is put on hold, I have changed the commit
description to:
pinctrl: sh-pfc: Rollback to mux if required when the gpio is freed
Some drivers require switching between function and gpio at run-time.
Allow to roll back from gpio to mux when the gpio is freed.
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox