* Re: [PATCH 10/14] ARM: lpc32xx: clean up header files
From: Sylvain Lemieux @ 2019-08-06 20:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-11-arnd@arndb.de>
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Wed, Jul 31, 2019 at 4:03 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> All device drivers have stopped relying on mach/*.h headers,
> so move the remaining headers into arch/arm/mach-lpc32xx/lpc32xx.h
> to prepare for multiplatform builds.
>
> The mach/entry-macro.S file has been unused for a long time now
> and can simply get removed.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-lpc32xx/common.c | 3 +-
> .../mach-lpc32xx/include/mach/entry-macro.S | 28 -------------------
> arch/arm/mach-lpc32xx/include/mach/hardware.h | 25 -----------------
> .../mach-lpc32xx/include/mach/uncompress.h | 4 +--
> .../{include/mach/platform.h => lpc32xx.h} | 18 ++++++++++--
> arch/arm/mach-lpc32xx/pm.c | 3 +-
> arch/arm/mach-lpc32xx/serial.c | 3 +-
> arch/arm/mach-lpc32xx/suspend.S | 3 +-
> 8 files changed, 21 insertions(+), 66 deletions(-)
> delete mode 100644 arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> delete mode 100644 arch/arm/mach-lpc32xx/include/mach/hardware.h
> rename arch/arm/mach-lpc32xx/{include/mach/platform.h => lpc32xx.h} (98%)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index a475339333c1..304ea61a0716 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -13,8 +13,7 @@
> #include <asm/mach/map.h>
> #include <asm/system_info.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> +#include "lpc32xx.h"
> #include "common.h"
>
> /*
> diff --git a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S b/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> deleted file mode 100644
> index eec0f5f7e722..000000000000
> --- a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> - *
> - * Author: Kevin Wells <kevin.wells@nxp.com>
> - *
> - * Copyright (C) 2010 NXP Semiconductors
> - */
> -
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> -#define LPC32XX_INTC_MASKED_STATUS_OFS 0x8
> -
> - .macro get_irqnr_preamble, base, tmp
> - ldr \base, =IO_ADDRESS(LPC32XX_MIC_BASE)
> - .endm
> -
> -/*
> - * Return IRQ number in irqnr. Also return processor Z flag status in CPSR
> - * as set if an interrupt is pending.
> - */
> - .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
> - ldr \irqstat, [\base, #LPC32XX_INTC_MASKED_STATUS_OFS]
> - clz \irqnr, \irqstat
> - rsb \irqnr, \irqnr, #31
> - teq \irqstat, #0
> - .endm
> diff --git a/arch/arm/mach-lpc32xx/include/mach/hardware.h b/arch/arm/mach-lpc32xx/include/mach/hardware.h
> deleted file mode 100644
> index 4866f096ffce..000000000000
> --- a/arch/arm/mach-lpc32xx/include/mach/hardware.h
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * arch/arm/mach-lpc32xx/include/mach/hardware.h
> - *
> - * Copyright (c) 2005 MontaVista Software, Inc. <source@mvista.com>
> - */
> -
> -#ifndef __ASM_ARCH_HARDWARE_H
> -#define __ASM_ARCH_HARDWARE_H
> -
> -/*
> - * Start of virtual addresses for IO devices
> - */
> -#define IO_BASE 0xF0000000
> -
> -/*
> - * This macro relies on fact that for all HW i/o addresses bits 20-23 are 0
> - */
> -#define IO_ADDRESS(x) IOMEM(((((x) & 0xff000000) >> 4) | ((x) & 0xfffff)) |\
> - IO_BASE)
> -
> -#define io_p2v(x) ((void __iomem *) (unsigned long) IO_ADDRESS(x))
> -#define io_v2p(x) ((((x) & 0x0ff00000) << 4) | ((x) & 0x000fffff))
> -
> -#endif
> diff --git a/arch/arm/mach-lpc32xx/include/mach/uncompress.h b/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> index a568812a0b91..74b7aa0da0e4 100644
> --- a/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> +++ b/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> @@ -12,15 +12,13 @@
>
> #include <linux/io.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> /*
> * Uncompress output is hardcoded to standard UART 5
> */
>
> #define UART_FIFO_CTL_TX_RESET (1 << 2)
> #define UART_STATUS_TX_MT (1 << 6)
> +#define LPC32XX_UART5_BASE 0x40090000
>
> #define _UARTREG(x) (void __iomem *)(LPC32XX_UART5_BASE + (x))
>
> diff --git a/arch/arm/mach-lpc32xx/include/mach/platform.h b/arch/arm/mach-lpc32xx/lpc32xx.h
> similarity index 98%
> rename from arch/arm/mach-lpc32xx/include/mach/platform.h
> rename to arch/arm/mach-lpc32xx/lpc32xx.h
> index 1c53790444fc..5eeb884a1993 100644
> --- a/arch/arm/mach-lpc32xx/include/mach/platform.h
> +++ b/arch/arm/mach-lpc32xx/lpc32xx.h
> @@ -7,8 +7,8 @@
> * Copyright (C) 2010 NXP Semiconductors
> */
>
> -#ifndef __ASM_ARCH_PLATFORM_H
> -#define __ASM_ARCH_PLATFORM_H
> +#ifndef __ARM_LPC32XX_H
> +#define __ARM_LPC32XX_H
>
> #define _SBF(f, v) ((v) << (f))
> #define _BIT(n) _SBF(n, 1)
> @@ -700,4 +700,18 @@
> #define LPC32XX_USB_OTG_DEV_CLOCK_ON _BIT(1)
> #define LPC32XX_USB_OTG_HOST_CLOCK_ON _BIT(0)
>
> +/*
> + * Start of virtual addresses for IO devices
> + */
> +#define IO_BASE 0xF0000000
> +
> +/*
> + * This macro relies on fact that for all HW i/o addresses bits 20-23 are 0
> + */
> +#define IO_ADDRESS(x) IOMEM(((((x) & 0xff000000) >> 4) | ((x) & 0xfffff)) |\
> + IO_BASE)
> +
> +#define io_p2v(x) ((void __iomem *) (unsigned long) IO_ADDRESS(x))
> +#define io_v2p(x) ((((x) & 0x0ff00000) << 4) | ((x) & 0x000fffff))
> +
> #endif
> diff --git a/arch/arm/mach-lpc32xx/pm.c b/arch/arm/mach-lpc32xx/pm.c
> index 32bca351a73b..b27fa1b9f56c 100644
> --- a/arch/arm/mach-lpc32xx/pm.c
> +++ b/arch/arm/mach-lpc32xx/pm.c
> @@ -70,8 +70,7 @@
>
> #include <asm/cacheflush.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> +#include "lpc32xx.h"
> #include "common.h"
>
> #define TEMP_IRAM_AREA IO_ADDRESS(LPC32XX_IRAM_BASE)
> diff --git a/arch/arm/mach-lpc32xx/serial.c b/arch/arm/mach-lpc32xx/serial.c
> index cfb35e5691cd..3e765c4bf986 100644
> --- a/arch/arm/mach-lpc32xx/serial.c
> +++ b/arch/arm/mach-lpc32xx/serial.c
> @@ -16,8 +16,7 @@
> #include <linux/clk.h>
> #include <linux/io.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> +#include "lpc32xx.h"
> #include "common.h"
>
> #define LPC32XX_SUART_FIFO_SIZE 64
> diff --git a/arch/arm/mach-lpc32xx/suspend.S b/arch/arm/mach-lpc32xx/suspend.S
> index 374f9f07fe48..3f0a8282ef6f 100644
> --- a/arch/arm/mach-lpc32xx/suspend.S
> +++ b/arch/arm/mach-lpc32xx/suspend.S
> @@ -11,8 +11,7 @@
> */
> #include <linux/linkage.h>
> #include <asm/assembler.h>
> -#include <mach/platform.h>
> -#include <mach/hardware.h>
> +#include "lpc32xx.h"
>
> /* Using named register defines makes the code easier to follow */
> #define WORK1_REG r0
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 08/14] net: lpc-enet: allow compile testing
From: Sylvain Lemieux @ 2019-08-06 20:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-9-arnd@arndb.de>
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The lpc-enet driver can now be built on all platforms, so
> allow compile testing as well.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/nxp/Kconfig | 2 +-
> drivers/net/ethernet/nxp/lpc_eth.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/nxp/Kconfig b/drivers/net/ethernet/nxp/Kconfig
> index 261f107e2be0..418afb84c84b 100644
> --- a/drivers/net/ethernet/nxp/Kconfig
> +++ b/drivers/net/ethernet/nxp/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config LPC_ENET
> tristate "NXP ethernet MAC on LPC devices"
> - depends on ARCH_LPC32XX
> + depends on ARCH_LPC32XX || COMPILE_TEST
> select PHYLIB
> help
> Say Y or M here if you want to use the NXP ethernet MAC included on
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
> index 0893b77c385d..34fdf2100772 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -14,6 +14,7 @@
> #include <linux/crc32.h>
> #include <linux/etherdevice.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/of_net.h>
> #include <linux/phy.h>
> #include <linux/platform_device.h>
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 07/14] net: lpc-enet: move phy setup into platform code
From: Sylvain Lemieux @ 2019-08-06 20:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-8-arnd@arndb.de>
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Setting the phy mode requires touching a platform specific
> register, which prevents us from building the driver without
> its header files.
>
> Move it into a separate function in arch/arm/mach/lpc32xx
> to hide the core registers from the network driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-lpc32xx/common.c | 12 ++++++++++++
> drivers/net/ethernet/nxp/lpc_eth.c | 12 +-----------
> include/linux/soc/nxp/lpc32xx-misc.h | 5 +++++
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index f648324d5fb4..a475339333c1 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -63,6 +63,18 @@ u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> }
> EXPORT_SYMBOL_GPL(lpc32xx_return_iram);
>
> +void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> + u32 tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> + tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> + if (mode == PHY_INTERFACE_MODE_MII)
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> + else
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> + __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(lpc32xx_set_phy_interface_mode);
> +
> static struct map_desc lpc32xx_io_desc[] __initdata = {
> {
> .virtual = (unsigned long)IO_ADDRESS(LPC32XX_AHB0_START),
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
> index bcdd0adcfb0c..0893b77c385d 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -20,9 +20,6 @@
> #include <linux/spinlock.h>
> #include <linux/soc/nxp/lpc32xx-misc.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> #define MODNAME "lpc-eth"
> #define DRV_VERSION "1.00"
>
> @@ -1237,16 +1234,9 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
> dma_addr_t dma_handle;
> struct resource *res;
> int irq, ret;
> - u32 tmp;
>
> /* Setup network interface for RMII or MII mode */
> - tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> - tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> - if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> - else
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> - __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> + lpc32xx_set_phy_interface_mode(lpc_phy_interface_mode(dev));
>
> /* Get platform resources */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/include/linux/soc/nxp/lpc32xx-misc.h b/include/linux/soc/nxp/lpc32xx-misc.h
> index f232e1a1bcdc..af4f82f6cf3b 100644
> --- a/include/linux/soc/nxp/lpc32xx-misc.h
> +++ b/include/linux/soc/nxp/lpc32xx-misc.h
> @@ -9,9 +9,11 @@
> #define __SOC_LPC32XX_MISC_H
>
> #include <linux/types.h>
> +#include <linux/phy.h>
>
> #ifdef CONFIG_ARCH_LPC32XX
> extern u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr);
> +extern void lpc32xx_set_phy_interface_mode(phy_interface_t mode);
> #else
> static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> {
> @@ -19,6 +21,9 @@ static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaadd
> *dmaaddr = 0;
> return 0;
> }
> +static inline void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> +}
> #endif
>
> #endif /* __SOC_LPC32XX_MISC_H */
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 07/14] net: lpc-enet: move phy setup into platform code
From: Sylvain Lemieux @ 2019-08-06 20:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-8-arnd@arndb.de>
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Setting the phy mode requires touching a platform specific
> register, which prevents us from building the driver without
> its header files.
>
> Move it into a separate function in arch/arm/mach/lpc32xx
> to hide the core registers from the network driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-lpc32xx/common.c | 12 ++++++++++++
> drivers/net/ethernet/nxp/lpc_eth.c | 12 +-----------
> include/linux/soc/nxp/lpc32xx-misc.h | 5 +++++
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index f648324d5fb4..a475339333c1 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -63,6 +63,18 @@ u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> }
> EXPORT_SYMBOL_GPL(lpc32xx_return_iram);
>
> +void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> + u32 tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> + tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> + if (mode == PHY_INTERFACE_MODE_MII)
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> + else
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> + __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(lpc32xx_set_phy_interface_mode);
> +
> static struct map_desc lpc32xx_io_desc[] __initdata = {
> {
> .virtual = (unsigned long)IO_ADDRESS(LPC32XX_AHB0_START),
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
> index bcdd0adcfb0c..0893b77c385d 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -20,9 +20,6 @@
> #include <linux/spinlock.h>
> #include <linux/soc/nxp/lpc32xx-misc.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> #define MODNAME "lpc-eth"
> #define DRV_VERSION "1.00"
>
> @@ -1237,16 +1234,9 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
> dma_addr_t dma_handle;
> struct resource *res;
> int irq, ret;
> - u32 tmp;
>
> /* Setup network interface for RMII or MII mode */
> - tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> - tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> - if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> - else
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> - __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> + lpc32xx_set_phy_interface_mode(lpc_phy_interface_mode(dev));
>
> /* Get platform resources */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/include/linux/soc/nxp/lpc32xx-misc.h b/include/linux/soc/nxp/lpc32xx-misc.h
> index f232e1a1bcdc..af4f82f6cf3b 100644
> --- a/include/linux/soc/nxp/lpc32xx-misc.h
> +++ b/include/linux/soc/nxp/lpc32xx-misc.h
> @@ -9,9 +9,11 @@
> #define __SOC_LPC32XX_MISC_H
>
> #include <linux/types.h>
> +#include <linux/phy.h>
>
> #ifdef CONFIG_ARCH_LPC32XX
> extern u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr);
> +extern void lpc32xx_set_phy_interface_mode(phy_interface_t mode);
> #else
> static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> {
> @@ -19,6 +21,9 @@ static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaadd
> *dmaaddr = 0;
> return 0;
> }
> +static inline void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> +}
> #endif
>
> #endif /* __SOC_LPC32XX_MISC_H */
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 05/14] gpio: lpc32xx: allow building on non-lpc32xx targets
From: Sylvain Lemieux @ 2019-08-06 20:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Bartosz Golaszewski, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, David S. Miller,
Greg Kroah-Hartman, Alan Stern, Guenter Roeck,
open list:GPIO SUBSYSTEM, Networking, linux-serial, USB list,
LINUXWATCHDOG, Lee Jones, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-6-arnd@arndb.de>
Hi Arnd,
On Wed, Jul 31, 2019 at 4:00 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The driver uses hardwire MMIO addresses instead of the data
> that is passed in device tree. Change it over to only
> hardcode the register offset values and allow compile-testing.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/gpio/Kconfig | 8 +++++
> drivers/gpio/Makefile | 2 +-
> drivers/gpio/gpio-lpc32xx.c | 63 ++++++++++++++++++++++++-------------
> 3 files changed, 50 insertions(+), 23 deletions(-)
>
[...]
> diff --git a/drivers/gpio/gpio-lpc32xx.c b/drivers/gpio/gpio-lpc32xx.c
> index 24885b3db3d5..548f7cb69386 100644
> --- a/drivers/gpio/gpio-lpc32xx.c
> +++ b/drivers/gpio/gpio-lpc32xx.c
[...]
> @@ -498,6 +509,10 @@ static int lpc32xx_gpio_probe(struct platform_device *pdev)
> {
> int i;
>
> + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> + if (gpio_reg_base)
> + return -ENXIO;
The probe function will always return an error.
Please replace the previous 2 lines with:
if (IS_ERR(gpio_reg_base))
return PTR_ERR(gpio_reg_base);
You can add my acked-by and tested-by in the v2 patch.
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
Tested-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
> +
> for (i = 0; i < ARRAY_SIZE(lpc32xx_gpiochip); i++) {
> if (pdev->dev.of_node) {
> lpc32xx_gpiochip[i].chip.of_xlate = lpc32xx_of_xlate;
> @@ -527,3 +542,7 @@ static struct platform_driver lpc32xx_gpio_driver = {
> };
>
> module_platform_driver(lpc32xx_gpio_driver);
> +
> +MODULE_AUTHOR("Kevin Wells <kevin.wells@nxp.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("GPIO driver for LPC32xx SoC");
> --
> 2.20.0
>
Sylvain
^ permalink raw reply
* Re: [PATCH] gpiolib: Take MUX usage into account
From: Ramon Fried @ 2019-08-06 19:54 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
linux-kernel@vger.kernel.org, Stefan Wahren
In-Reply-To: <CACRpkdaTmxQn2Z=vD6nyqk-iXCFrnCG1xpkXwO3-+sazOhGBvw@mail.gmail.com>
On August 6, 2019 4:11:27 PM GMT+03:00, Linus Walleij <linus.walleij@linaro.org> wrote:
>On Tue, Aug 6, 2019 at 3:04 PM Linus Walleij <linus.walleij@linaro.org>
>wrote:
>> On Sat, Aug 3, 2019 at 3:34 PM Ramon Fried <rfried.dev@gmail.com>
>wrote:
>>
>> > From: Stefan Wahren <stefan.wahren@i2se.com>
>> >
>> > The user space like gpioinfo only see the GPIO usage but not the
>> > MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to
>know which
>> > pin is free/safe to use. So take the MUX usage of strict pinmux
>controllers
>> > into account to get a more realistic view for ioctl
>GPIO_GET_LINEINFO_IOCTL.
>> >
>> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> > Tested-By: Ramon Fried <rfried.dev@gmail.com>
>> > Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
>> > ---
>> > Sending Stefan's RFC as patch, as I tested it and it seems to work,
>> > additionally, an accompanying fix was made by me to gpiolibd to fix
>a
>> > display error of the actual result:
>> > https://patchwork.ozlabs.org/patch/1139923/
>>
>> This is mostly fine, some style nits so I fixed it up when
>> applying.
>
>Ooops no. It needs a deeper rework in accordance to my comments
>last time it was posted. Please read this reply to Stefan's patch
>and address the comments:
>
NP, I'll try to address these in a new patch.
Thanks.
Ramon
>https://lore.kernel.org/linux-gpio/CACRpkdb5DjAMRYkT+b0U6HVk7E6ccLT79-LB=QGQWWtE17aPUg@mail.gmail.com/
>
>Yours,
>Linus Walleij
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v7 01/20] pinctrl: tegra: Add suspend and resume support
From: Dmitry Osipenko @ 2019-08-06 17:59 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree, rjw, viresh.kumar, linux-pm
In-Reply-To: <b45ca99a-188a-c695-3f3d-48d273808f9c@nvidia.com>
05.08.2019 21:06, Sowjanya Komatineni пишет:
>
> On 8/5/19 3:50 AM, Dmitry Osipenko wrote:
>> 01.08.2019 0:10, Sowjanya Komatineni пишет:
>>> This patch adds support for Tegra pinctrl driver suspend and resume.
>>>
>>> During suspend, context of all pinctrl registers are stored and
>>> on resume they are all restored to have all the pinmux and pad
>>> configuration for normal operation.
>>>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 59
>>> +++++++++++++++++++++++++++++++++++
>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
>>> 2 files changed, 62 insertions(+)
>>>
>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> index 186ef98e7b2b..e3a237534281 100644
>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> @@ -631,6 +631,58 @@ static void
>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>> }
>>> }
>>> +static size_t tegra_pinctrl_get_bank_size(struct device *dev,
>>> + unsigned int bank_id)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct resource *res;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, bank_id);
>>> +
>>> + return resource_size(res) / 4;
>>> +}
>>> +
>>> +static int tegra_pinctrl_suspend(struct device *dev)
>>> +{
>>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>>> + u32 *backup_regs = pmx->backup_regs;
>>> + u32 *regs;
>>> + size_t bank_size;
>>> + unsigned int i, k;
>>> +
>>> + for (i = 0; i < pmx->nbanks; i++) {
>>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>>> + regs = pmx->regs[i];
>>> + for (k = 0; k < bank_size; k++)
>>> + *backup_regs++ = readl_relaxed(regs++);
>>> + }
>>> +
>>> + return pinctrl_force_sleep(pmx->pctl);
>>> +}
>>> +
>>> +static int tegra_pinctrl_resume(struct device *dev)
>>> +{
>>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>>> + u32 *backup_regs = pmx->backup_regs;
>>> + u32 *regs;
>>> + size_t bank_size;
>>> + unsigned int i, k;
>>> +
>>> + for (i = 0; i < pmx->nbanks; i++) {
>>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>>> + regs = pmx->regs[i];
>>> + for (k = 0; k < bank_size; k++)
>>> + writel_relaxed(*backup_regs++, regs++);
>>> + }
>> I'm now curious whether any kind of barrier is needed after the
>> writings. The pmx_writel() doesn't insert a barrier after the write and
>> seems it just misuses writel, which actually should be writel_relaxed()
>> + barrier, IIUC.
>
> pmx_writel uses writel and it has wmb before raw_write which complete
> all writes initiated prior to this.
>
> By misusing writel, you mean to have barrier after register write?
Yes, at least to me it doesn't make much sense for this driver to stall
before the write. It's the pinctrl user which should be taking care
about everything to be ready before making a change to the pinctrl's
configuration.
>> It's also not obvious whether PINCTRL HW has any kind of write-FIFO and
>> thus maybe read-back + rmb() is needed in order ensure that writes are
>> actually completed.
> I believe adding write barrier wmb after writel_relaxed should be good
> rather than doing readback + rmb
>>
>> The last thing which is not obvious is when the new configuration
>> actually takes into effect, does it happen immediately or maybe some
>> delay is needed?
>>
>> [snip]
>
> Based on internal design there is no internal delay and it all depends
> on APB rate that it takes to write to register.
>
> Pinmux value change to reflect internally might take couple of clock
> cycles which is much faster than SW can read.
Still not quite obvious if it's possible to have a case where some
hardware is touched before necessary pinctrl change is fully completed
and then to get into trouble because of it.
^ permalink raw reply
* Re: [PATCH RFC 5/7] pwm: rcar: remove a redundant condition in rcar_pwm_apply()
From: Uwe Kleine-König @ 2019-08-06 16:00 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Yoshihiro Shimoda, Linus Walleij, Geert Uytterhoeven,
Thierry Reding, Rob Herring, Mark Rutland,
open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <CAMuHMdWw1Gh_CxgiO5gd+MY0vUvWX_ACDj+L3_Wcomkaf5Oo4Q@mail.gmail.com>
Hello,
On Tue, Aug 06, 2019 at 11:05:30AM +0200, Geert Uytterhoeven wrote:
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Since the rcar_pwm_apply() has already check whehter state->enabled
> > is not set or not, this patch removes a redundant condition.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> This is completely independent from the rest of the series, and can be applied
> immediately, right?
The original patch didn't make it into my mailbox. I only see a few
replies. Is it only me?
https://patchwork.ozlabs.org/project/linux-pwm/list/ doesn't seem to
have it either.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v1] pinctrl: intel: Allow to request locked pins
From: Linus Walleij @ 2019-08-06 14:27 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Mika Westerberg, open list:GPIO SUBSYSTEM
In-Reply-To: <20190726200830.52728-1-andriy.shevchenko@linux.intel.com>
Hi Andy,
On Fri, Jul 26, 2019 at 10:08 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Some firmwares would like to protect pins from being modified by OS
> and at the same time provide them to OS as a resource. So, the driver
> in such circumstances may request pin and may not change its state.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Interesting patch!
> +enum {
> + PAD_UNLOCKED = 0,
> + PAD_LOCKED = 1,
> + PAD_LOCKED_TX = 2,
> + PAD_LOCKED_FULL = PAD_LOCKED | PAD_LOCKED_TX,
> +};
Please add some kerneldoc explaining what these different lock
states are. My head is spinning. Locked? Locked TX? Locked full?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/4] pinctrl: samsung: Fix device node refcount leaks in Exynos wakeup controller init
From: Krzysztof Kozlowski @ 2019-08-06 14:25 UTC (permalink / raw)
To: Linus Walleij
Cc: Tomasz Figa, Sylwester Nawrocki, Kukjin Kim, Linux ARM,
linux-samsung-soc, open list:GPIO SUBSYSTEM,
linux-kernel@vger.kernel.org, Chanwoo Choi, Marek Szyprowski,
notify
In-Reply-To: <CACRpkda8P522pkxctZbf2Ut13V6Rzx=mSYsRuHv0BvPyF6q1gA@mail.gmail.com>
On Tue, 6 Aug 2019 at 16:15, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Aug 5, 2019 at 6:27 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > In exynos_eint_wkup_init() the for_each_child_of_node() loop is used
> > with a break to find a matching child node. Although each iteration of
> > for_each_child_of_node puts the previous node, but early exit from loop
> > misses it. This leads to leak of device node.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> I assume you're collecting these for a pull request to me
> at some later point, all look good to me.
Yes, I'll take these and one more patch from lists and send them to you.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] gpiolib: never report open-drain/source lines as 'input' to user-space
From: Linus Walleij @ 2019-08-06 14:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org,
Bartosz Golaszewski, stable
In-Reply-To: <20190806114151.17652-1-brgl@bgdev.pl>
On Tue, Aug 6, 2019 at 1:41 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> If the driver doesn't support open-drain/source config options, we
> emulate this behavior when setting the direction by calling
> gpiod_direction_input() if the default value is 0 (open-source) or
> 1 (open-drain), thus not actively driving the line in those cases.
>
> This however clears the FLAG_IS_OUT bit for the GPIO line descriptor
> and makes the LINEINFO ioctl() incorrectly report this line's mode as
> 'input' to user-space.
>
> This commit modifies the ioctl() to always set the GPIOLINE_FLAG_IS_OUT
> bit in the lineinfo structure's flags field. Since it's impossible to
> use the input mode and open-drain/source options at the same time, we
> can be sure the reported information will be correct.
>
> Fixes: 521a2ad6f862 ("gpio: add userspace ABI for GPIO line information")
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Patch applied for fixes!
Good catch.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] pinctrl: spear: spear: Add of_node_put() before return
From: Linus Walleij @ 2019-08-06 14:19 UTC (permalink / raw)
To: Nishka Dasgupta; +Cc: Viresh Kumar, Linux ARM, open list:GPIO SUBSYSTEM
In-Reply-To: <20190804154948.4584-1-nishkadg.linux@gmail.com>
On Sun, Aug 4, 2019 at 5:50 PM 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
> two places.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Patch applied with Viresh's ACK.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/4] pinctrl: samsung: Fix device node refcount leaks in Exynos wakeup controller init
From: Linus Walleij @ 2019-08-06 14:15 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Tomasz Figa, Sylwester Nawrocki, Kukjin Kim, Linux ARM,
linux-samsung-soc, open list:GPIO SUBSYSTEM,
linux-kernel@vger.kernel.org, Chanwoo Choi, Marek Szyprowski,
notify
In-Reply-To: <20190805162710.7789-1-krzk@kernel.org>
On Mon, Aug 5, 2019 at 6:27 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> In exynos_eint_wkup_init() the for_each_child_of_node() loop is used
> with a break to find a matching child node. Although each iteration of
> for_each_child_of_node puts the previous node, but early exit from loop
> misses it. This leads to leak of device node.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
I assume you're collecting these for a pull request to me
at some later point, all look good to me.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] gpiolib: Take MUX usage into account
From: Linus Walleij @ 2019-08-06 13:11 UTC (permalink / raw)
To: Ramon Fried
Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
linux-kernel@vger.kernel.org, Stefan Wahren
In-Reply-To: <CACRpkdYEdQdk62bWJ2=i2Mbvpz3kwL=9bnMoxksFsTgAHRh68w@mail.gmail.com>
On Tue, Aug 6, 2019 at 3:04 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Aug 3, 2019 at 3:34 PM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> > From: Stefan Wahren <stefan.wahren@i2se.com>
> >
> > The user space like gpioinfo only see the GPIO usage but not the
> > MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
> > pin is free/safe to use. So take the MUX usage of strict pinmux controllers
> > into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > Tested-By: Ramon Fried <rfried.dev@gmail.com>
> > Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> > ---
> > Sending Stefan's RFC as patch, as I tested it and it seems to work,
> > additionally, an accompanying fix was made by me to gpiolibd to fix a
> > display error of the actual result:
> > https://patchwork.ozlabs.org/patch/1139923/
>
> This is mostly fine, some style nits so I fixed it up when
> applying.
Ooops no. It needs a deeper rework in accordance to my comments
last time it was posted. Please read this reply to Stefan's patch
and address the comments:
https://lore.kernel.org/linux-gpio/CACRpkdb5DjAMRYkT+b0U6HVk7E6ccLT79-LB=QGQWWtE17aPUg@mail.gmail.com/
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] gpiolib: Take MUX usage into account
From: Linus Walleij @ 2019-08-06 13:04 UTC (permalink / raw)
To: Ramon Fried
Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
linux-kernel@vger.kernel.org, Stefan Wahren
In-Reply-To: <20190803133436.15016-1-rfried.dev@gmail.com>
On Sat, Aug 3, 2019 at 3:34 PM Ramon Fried <rfried.dev@gmail.com> wrote:
> From: Stefan Wahren <stefan.wahren@i2se.com>
>
> The user space like gpioinfo only see the GPIO usage but not the
> MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
> pin is free/safe to use. So take the MUX usage of strict pinmux controllers
> into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Tested-By: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> ---
> Sending Stefan's RFC as patch, as I tested it and it seems to work,
> additionally, an accompanying fix was made by me to gpiolibd to fix a
> display error of the actual result:
> https://patchwork.ozlabs.org/patch/1139923/
This is mostly fine, some style nits so I fixed it up when
applying.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] pinctrl: freescale: mxs: Add of_node_put() before return
From: Linus Walleij @ 2019-08-06 12:52 UTC (permalink / raw)
To: Nishka Dasgupta
Cc: Linux ARM, open list:GPIO SUBSYSTEM, NXP Linux Team, Sascha Hauer,
Sascha Hauer, Stefan Agner, Shawn Guo, Fabio Estevam,
Dong Aisheng
In-Reply-To: <20190804160420.5309-1-nishkadg.linux@gmail.com>
On Sun, Aug 4, 2019 at 6:04 PM 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: [PATCH] pinctrl: nomadik: nomadik: Add of_node_put() before return
From: Linus Walleij @ 2019-08-06 12:51 UTC (permalink / raw)
To: Nishka Dasgupta; +Cc: Linux ARM, open list:GPIO SUBSYSTEM
In-Reply-To: <20190804155117.4753-1-nishkadg.linux@gmail.com>
On Sun, Aug 4, 2019 at 5:51 PM 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.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] pinctrl: falcon: Add of_node_put() before return
From: Linus Walleij @ 2019-08-06 12:50 UTC (permalink / raw)
To: Nishka Dasgupta; +Cc: open list:GPIO SUBSYSTEM
In-Reply-To: <20190804152745.2231-1-nishkadg.linux@gmail.com>
On Sun, Aug 4, 2019 at 5:28 PM Nishka Dasgupta <nishkadg.linux@gmail.com> wrote:
> Each iteration of for_each_compatible_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 two
> places.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Patch applied!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2] gpio: mpc8xxx: Add new platforms GPIO DT node description
From: Linus Walleij @ 2019-08-06 12:48 UTC (permalink / raw)
To: Hui Song
Cc: Shawn Guo, Li Yang, Rob Herring, Mark Rutland,
Bartosz Golaszewski, Linux ARM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <20190806024923.34355-1-hui.song_1@nxp.com>
Hi Hui,
On Tue, Aug 6, 2019 at 4:59 AM Hui Song <hui.song_1@nxp.com> wrote:
> From: Song Hui <hui.song_1@nxp.com>
>
> Update the NXP GPIO node dt-binding file for QorIQ and
> Layerscape platforms, and add one more example with
> ls1028a GPIO node.
>
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
(...)
> +Example of gpio-controller node for a ls1028a SoC:
> +
> +gpio1: gpio@2300000 {
> + compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
What you need to do is to add "fsl,ls1028a-gpio" to the list
of compatible values at the top of the file "Required properties".
Please send a v2 with this fixed.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] gpio: Fix build error of function redefinition
From: Linus Walleij @ 2019-08-06 12:45 UTC (permalink / raw)
To: YueHaibing
Cc: Bartosz Golaszewski, Masahiro Yamada,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <20190731123814.46624-1-yuehaibing@huawei.com>
On Wed, Jul 31, 2019 at 2:39 PM YueHaibing <yuehaibing@huawei.com> wrote:
> when do randbuilding, I got this error:
>
> In file included from drivers/hwmon/pmbus/ucd9000.c:19:0:
> ./include/linux/gpio/driver.h:576:1: error: redefinition of gpiochip_add_pin_range
> gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
> ^~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/hwmon/pmbus/ucd9000.c:18:0:
> ./include/linux/gpio.h:245:1: note: previous definition of gpiochip_add_pin_range was here
> gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
> ^~~~~~~~~~~~~~~~~~~~~~
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 964cb341882f ("gpio: move pincontrol calls to <linux/gpio/driver.h>")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Yeah those get covered twice these days I suppose.
Patch applied, good catch.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFC 3/7] pinctrl: sh-pfc: Rollback to mux if requires when the gpio is freed
From: Geert Uytterhoeven @ 2019-08-06 12:01 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <OSBPR01MB4536870EEEE634B06199722ED8D50@OSBPR01MB4536.jpnprd01.prod.outlook.com>
Hi Shimoda-san,
On Tue, Aug 6, 2019 at 1:38 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:03 PM
> > On Mon, Jul 8, 2019 at 11:08 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>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> > > +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> > > @@ -26,6 +26,7 @@
> > > #include "../pinconf.h"
> > >
> > > struct sh_pfc_pin_config {
> > > + unsigned int mux_mark;
> >
> > Due to padding, adding this field will increase memory consumption by
> > 6 bytes per pin.
>
> I see.
>
> > Probably sh_pfc_pin_group.{pins,mux} should be changed from unsigned int
> > to u16, but that's out of scope for this patch.
>
> I got it.
For now, please don't worry about it. I can make that change later, as it will
affect all drivers.
> > > @@ -353,6 +354,15 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> > > spin_lock_irqsave(&pfc->lock, flags);
> > >
> > > for (i = 0; i < grp->nr_pins; ++i) {
> > > + int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]);
> > > + struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
> > > +
> > > + /*
> > > + * This doesn't assume the order which gpios are enabled
> > > + * and then mux is set.
> >
> > I'm sorry, I don't understand what you mean?
> > Can you please reword or elaborate?
>
> I was also difficult to remember what I meant...
> Anyway, this meant,
> 1) if a device has the default pinctrl-0 property, the set_mux() ops is called
> before the device driver's probe() function is called by pinctrl_bind_pins() first,
> 2) so that any device drivers cannot call gpiod_get() before the 1).
>
> However, this comments don't cover an imbalance pinctrl/gpio handling.
> For example (as pseudo):
> - SCIF driver uses SCIF2 pinctrl,
> - but, IOMMU driver gets the SCIF2 pins before SCIF driver is probed.
>
> So, I'd like to revise the comments as following. What do you think?
>
> --
> This driver cannot manage both gpio and mux when the gpio pin
> is already enabled. So, this function failed.
> --
>
> > > + */
> > > + WARN_ON(cfg->gpio_enabled);
> >
> > Can this actually happen?
>
> This cannot happen actually.
>
> > Should this cause a failure instead?
>
> I think so.
OK.
> > > + if (cfg->mux_set)
> > > + sh_pfc_config_mux(pfc, cfg->mux_mark, PINMUX_TYPE_FUNCTION);
> >
> > Have you considered the case where more than one pin of a pinmux group
> > was used as a GPIO? In that case sh_pfc_gpio_disable_free() will be called
> > multiple times, possibly with the same mux_mark.
>
> I haven't considered the case. But, about the mux_mark, I checked the values and then
> they are not the same.
IC. At first I thought they were the internal enum for the whole pin group, but
I was wrong.
They are the mux *_MARK enu, which is unique for each pin/function combo.
> For example (debug printk patch):
> diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
> index bc29066..fdac71b 100644
> --- a/drivers/pinctrl/sh-pfc/pinctrl.c
> +++ b/drivers/pinctrl/sh-pfc/pinctrl.c
> @@ -349,7 +349,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
> unsigned int i;
> int ret = 0;
>
> - dev_dbg(pctldev->dev, "Configuring pin group %s\n", grp->name);
> + dev_info(pctldev->dev, "Configuring pin group %s\n", grp->name);
>
> spin_lock_irqsave(&pfc->lock, flags);
>
> @@ -375,6 +375,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
>
> cfg->mux_set = true;
> cfg->mux_mark = grp->mux[i];
> + dev_info(pctldev->dev, "%d: %x\n", i, cfg->mux_mark);
> }
>
> done:
> --
> 2.7.4
>
> For example (log):
> [ 0.497647] sh-pfc e6060000.pin-controller: Configuring pin group scif2_data_a
> [ 0.497711] sh-pfc e6060000.pin-controller: 0: 77b
> [ 0.497715] sh-pfc e6060000.pin-controller: 1: 760
Thanks for checking!
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 RFC 2/7] pinctrl: sh-pfc: remove incomplete flag "cfg->type"
From: Yoshihiro Shimoda @ 2019-08-06 11:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Geert Uytterhoeven, Thierry Reding, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, Linux PWM List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <CAMuHMdVcAw_ApKMmrV7DaoJBGUZ1GzW3kmxnsTn72FtCGWhXPA@mail.gmail.com>
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:24 PM
>
> Hi Shimoda-san,
>
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> > support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> > sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> > Now if we fix the cfg->type condition, it gets worse because:
> > - Some drivers might be deferred so that .set_mux() will be called
> > multiple times.
> > - In such the case, the sh-pfc driver returns -EBUSY even if
> > the group is the same, and then that driver fails to probe.
> >
> > Since the pinctrl subsystem already has such conditions according
> > to @set_mux and @gpio_request_enable, this patch just remove
> > the incomplete flag from sh-pfc/pinctrl.c.
>
> Do we need to set sh_pfc_pinmux_ops.strict = true?
If the .strict = true, the final pwm patch on this series failed with the following error:
[ 11.453716] sh-pfc e6060000.pin-controller: pin GP_2_7 already requested by e6e31000.pwm; cannot claim for e6052000.gpio:459
Best regards,
Yoshihiro Shimoda
> 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 v1] pinctrl: intel: Allow to request locked pins
From: Mika Westerberg @ 2019-08-06 11:47 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij
In-Reply-To: <20190806114517.GT23480@smile.fi.intel.com>
On Tue, Aug 06, 2019 at 02:45:17PM +0300, Andy Shevchenko wrote:
> On Tue, Aug 06, 2019 at 02:01:26PM +0300, Mika Westerberg wrote:
> > On Fri, Jul 26, 2019 at 11:08:30PM +0300, Andy Shevchenko wrote:
> > > Some firmwares would like to protect pins from being modified by OS
> > > and at the same time provide them to OS as a resource. So, the driver
> > > in such circumstances may request pin and may not change its state.
> >
> > This is definitely good idea.
>
> Thanks for review, my answers below.
>
> > > * the pad is considered unlocked. Any other case means that it is
> > > * either fully or partially locked and we don't touch it.
> >
> > I think you should update the above comment as well.
>
> Will do for v2.
>
> > > raw_spin_lock_irqsave(&pctrl->lock, flags);
> > >
> > > - if (!intel_pad_usable(pctrl, pin)) {
> > > + if (!intel_pad_owned_by_host(pctrl, pin)) {
> > > raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > > return -EBUSY;
> > > }
> > >
> > > + if (!intel_pad_is_unlocked(pctrl, pin)) {
> > > + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > > + return 0;
> >
> > Hmm, if I'm reading this right it still does not allow requesting locked
> > pins. What I'm missing here?
>
> We do not return an error code here, so pin is left requested but pad
> configuration register untouched.
Indeed, now it is clear. Thanks for explaining :)
^ permalink raw reply
* Re: [PATCH v1] pinctrl: intel: Allow to request locked pins
From: Andy Shevchenko @ 2019-08-06 11:45 UTC (permalink / raw)
To: Mika Westerberg; +Cc: linux-gpio, Linus Walleij
In-Reply-To: <20190806110126.GR2548@lahna.fi.intel.com>
On Tue, Aug 06, 2019 at 02:01:26PM +0300, Mika Westerberg wrote:
> On Fri, Jul 26, 2019 at 11:08:30PM +0300, Andy Shevchenko wrote:
> > Some firmwares would like to protect pins from being modified by OS
> > and at the same time provide them to OS as a resource. So, the driver
> > in such circumstances may request pin and may not change its state.
>
> This is definitely good idea.
Thanks for review, my answers below.
> > * the pad is considered unlocked. Any other case means that it is
> > * either fully or partially locked and we don't touch it.
>
> I think you should update the above comment as well.
Will do for v2.
> > raw_spin_lock_irqsave(&pctrl->lock, flags);
> >
> > - if (!intel_pad_usable(pctrl, pin)) {
> > + if (!intel_pad_owned_by_host(pctrl, pin)) {
> > raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > return -EBUSY;
> > }
> >
> > + if (!intel_pad_is_unlocked(pctrl, pin)) {
> > + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > + return 0;
>
> Hmm, if I'm reading this right it still does not allow requesting locked
> pins. What I'm missing here?
We do not return an error code here, so pin is left requested but pad
configuration register untouched.
> > + }
> > +
> > padcfg0 = intel_get_padcfg(pctrl, pin, PADCFG0);
> > intel_gpio_set_gpio_mode(padcfg0);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] gpiolib: never report open-drain/source lines as 'input' to user-space
From: Bartosz Golaszewski @ 2019-08-06 11:41 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, stable
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
If the driver doesn't support open-drain/source config options, we
emulate this behavior when setting the direction by calling
gpiod_direction_input() if the default value is 0 (open-source) or
1 (open-drain), thus not actively driving the line in those cases.
This however clears the FLAG_IS_OUT bit for the GPIO line descriptor
and makes the LINEINFO ioctl() incorrectly report this line's mode as
'input' to user-space.
This commit modifies the ioctl() to always set the GPIOLINE_FLAG_IS_OUT
bit in the lineinfo structure's flags field. Since it's impossible to
use the input mode and open-drain/source options at the same time, we
can be sure the reported information will be correct.
Fixes: 521a2ad6f862 ("gpio: add userspace ABI for GPIO line information")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
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 f497003f119c..80a2a2cb673b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1091,9 +1091,11 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
lineinfo.flags |= GPIOLINE_FLAG_ACTIVE_LOW;
if (test_bit(FLAG_OPEN_DRAIN, &desc->flags))
- lineinfo.flags |= GPIOLINE_FLAG_OPEN_DRAIN;
+ lineinfo.flags |= (GPIOLINE_FLAG_OPEN_DRAIN |
+ GPIOLINE_FLAG_IS_OUT);
if (test_bit(FLAG_OPEN_SOURCE, &desc->flags))
- lineinfo.flags |= GPIOLINE_FLAG_OPEN_SOURCE;
+ lineinfo.flags |= (GPIOLINE_FLAG_OPEN_SOURCE |
+ GPIOLINE_FLAG_IS_OUT);
if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
return -EFAULT;
--
2.21.0
^ permalink raw reply related
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