diff mbox series

[07/11] pinctrl: rp1: Implement RaspberryPi RP1 gpio support

Message ID eb39a5f3cefff2a1240a18a255dac090af16f223.1724159867.git.andrea.porta@suse.com (mailing list archive)
State Not Applicable, archived
Headers show
Series Add support for RaspberryPi RP1 PCI device using a DT overlay | expand

Commit Message

Andrea della Porta Aug. 20, 2024, 2:36 p.m. UTC
The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
Add minimum support for the gpio only portion. The driver is in
pinctrl folder since upcoming patches will add the pinmux/pinctrl
support where the gpio part can be seen as an addition.

Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
 MAINTAINERS                   |   1 +
 drivers/pinctrl/Kconfig       |  10 +
 drivers/pinctrl/Makefile      |   1 +
 drivers/pinctrl/pinctrl-rp1.c | 719 ++++++++++++++++++++++++++++++++++
 4 files changed, 731 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rp1.c

Comments

Krzysztof Kozlowski Aug. 21, 2024, 8:45 a.m. UTC | #1
On Tue, Aug 20, 2024 at 04:36:09PM +0200, Andrea della Porta wrote:
> The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
> Add minimum support for the gpio only portion. The driver is in
> pinctrl folder since upcoming patches will add the pinmux/pinctrl
> support where the gpio part can be seen as an addition.
> 
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> ---
>  MAINTAINERS                   |   1 +
>  drivers/pinctrl/Kconfig       |  10 +
>  drivers/pinctrl/Makefile      |   1 +
>  drivers/pinctrl/pinctrl-rp1.c | 719 ++++++++++++++++++++++++++++++++++
>  4 files changed, 731 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-rp1.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4ce7b049d67e..67f460c36ea1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19122,6 +19122,7 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/clock/raspberrypi,rp1-clocks.yaml
>  F:	Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
>  F:	drivers/clk/clk-rp1.c
> +F:	drivers/pinctrl/pinctrl-rp1.c
>  F:	include/dt-bindings/clock/rp1.h
>  F:	include/dt-bindings/misc/rp1.h
>  
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 7e4f93a3bc7a..18bb1a8bd102 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -565,6 +565,16 @@ config PINCTRL_MLXBF3
>  	  each pin. This driver can also be built as a module called
>  	  pinctrl-mlxbf3.
>  
> +config PINCTRL_RP1
> +	bool "Pinctrl driver for RP1"
> +	select PINMUX
> +	select PINCONF
> +	select GENERIC_PINCONF
> +	select GPIOLIB_IRQCHIP
> +	help
> +	  Enable the gpio and pinctrl/mux  driver for RaspberryPi RP1
> +	  multi function device. 
> +
>  source "drivers/pinctrl/actions/Kconfig"
>  source "drivers/pinctrl/aspeed/Kconfig"
>  source "drivers/pinctrl/bcm/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index cc809669405a..f1ca23b563f6 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
>  obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
>  obj-$(CONFIG_PINCTRL_RK805)	+= pinctrl-rk805.o
>  obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
> +obj-$(CONFIG_PINCTRL_RP1)       += pinctrl-rp1.o
>  obj-$(CONFIG_PINCTRL_SCMI)	+= pinctrl-scmi.o
>  obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
>  obj-$(CONFIG_PINCTRL_ST) 	+= pinctrl-st.o
> diff --git a/drivers/pinctrl/pinctrl-rp1.c b/drivers/pinctrl/pinctrl-rp1.c
> new file mode 100644
> index 000000000000..c035d2014505
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-rp1.c
> @@ -0,0 +1,719 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Raspberry Pi RP1 GPIO unit
> + *
> + * Copyright (C) 2023 Raspberry Pi Ltd.
> + *
> + * This driver is inspired by:
> + * pinctrl-bcm2835.c, please see original file for copyright information
> + */
> +
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/bug.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqdesc.h>
> +#include <linux/init.h>
> +#include <linux/of_address.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/seq_file.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>

Half of these headers are not used. Drop them.

Best regards,
Krzysztof
kernel test robot Aug. 21, 2024, 9:22 a.m. UTC | #2
Hi Andrea,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.11-rc4 next-20240821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrea-della-Porta/dt-bindings-clock-Add-RaspberryPi-RP1-clock-bindings/20240821-023901
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/eb39a5f3cefff2a1240a18a255dac090af16f223.1724159867.git.andrea.porta%40suse.com
patch subject: [PATCH 07/11] pinctrl: rp1: Implement RaspberryPi RP1 gpio support
config: nios2-kismet-CONFIG_GPIOLIB_IRQCHIP-CONFIG_PINCTRL_RP1-0-0 (https://download.01.org/0day-ci/archive/20240821/202408211702.1WVqlgTb-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20240821/202408211702.1WVqlgTb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408211702.1WVqlgTb-lkp@intel.com/

kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP when selected by PINCTRL_RP1
   WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP
     Depends on [n]: GPIOLIB [=n]
     Selected by [y]:
     - PINCTRL_RP1 [=y] && PINCTRL [=y]
kernel test robot Aug. 21, 2024, 12:06 p.m. UTC | #3
Hi Andrea,

kernel test robot noticed the following build errors:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on robh/for-next char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.11-rc4 next-20240821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrea-della-Porta/dt-bindings-clock-Add-RaspberryPi-RP1-clock-bindings/20240821-023901
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/eb39a5f3cefff2a1240a18a255dac090af16f223.1724159867.git.andrea.porta%40suse.com
patch subject: [PATCH 07/11] pinctrl: rp1: Implement RaspberryPi RP1 gpio support
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240821/202408211907.cUrf3RpN-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240821/202408211907.cUrf3RpN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408211907.cUrf3RpN-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pinctrl/pinctrl-rp1.c: In function 'rp1_get_fsel':
>> drivers/pinctrl/pinctrl-rp1.c:237:22: error: implicit declaration of function 'FIELD_GET'; did you mean 'FIELD_SET'? [-Werror=implicit-function-declaration]
     237 |         u32 oeover = FIELD_GET(RP1_GPIO_CTRL_OEOVER_MASK, ctrl);
         |                      ^~~~~~~~~
         |                      FIELD_SET
   drivers/pinctrl/pinctrl-rp1.c: In function 'rp1_set_fsel':
>> drivers/pinctrl/pinctrl-rp1.c:146:25: error: implicit declaration of function 'FIELD_PREP'; did you mean 'FIELD_SET'? [-Werror=implicit-function-declaration]
     146 |                 _reg |= FIELD_PREP((_mask), (_val));    \
         |                         ^~~~~~~~~~
   drivers/pinctrl/pinctrl-rp1.c:257:17: note: in expansion of macro 'FIELD_SET'
     257 |                 FIELD_SET(ctrl, RP1_GPIO_CTRL_OEOVER_MASK, RP1_OEOVER_DISABLE);
         |                 ^~~~~~~~~
   cc1: some warnings being treated as errors


vim +237 drivers/pinctrl/pinctrl-rp1.c

   136	
   137	#define RP1_PAD_DRIVE_2MA		0x00000000
   138	#define RP1_PAD_DRIVE_4MA		BIT(4)
   139	#define RP1_PAD_DRIVE_8MA		BIT(5)
   140	#define RP1_PAD_DRIVE_12MA		(RP1_PAD_DRIVE_4MA | \
   141						RP1_PAD_DRIVE_8MA)
   142	
   143	#define FIELD_SET(_reg, _mask, _val)			\
   144		({						\
   145			_reg &= ~(_mask);				\
 > 146			_reg |= FIELD_PREP((_mask), (_val));	\
   147		})
   148	
   149	#define FUNC(f) \
   150		[func_##f] = #f
   151	
   152	struct rp1_iobank_desc {
   153		int min_gpio;
   154		int num_gpios;
   155		int gpio_offset;
   156		int inte_offset;
   157		int ints_offset;
   158		int rio_offset;
   159		int pads_offset;
   160	};
   161	
   162	struct rp1_pin_info {
   163		u8 num;
   164		u8 bank;
   165		u8 offset;
   166		u8 fsel;
   167		u8 irq_type;
   168	
   169		void __iomem *gpio;
   170		void __iomem *rio;
   171		void __iomem *inte;
   172		void __iomem *ints;
   173		void __iomem *pad;
   174	};
   175	
   176	struct rp1_pinctrl {
   177		struct device *dev;
   178		void __iomem *gpio_base;
   179		void __iomem *rio_base;
   180		void __iomem *pads_base;
   181		int irq[RP1_NUM_BANKS];
   182		struct rp1_pin_info pins[RP1_NUM_GPIOS];
   183	
   184		struct pinctrl_dev *pctl_dev;
   185		struct gpio_chip gpio_chip;
   186		struct pinctrl_gpio_range gpio_range;
   187	
   188		raw_spinlock_t irq_lock[RP1_NUM_BANKS];
   189	};
   190	
   191	const struct rp1_iobank_desc rp1_iobanks[RP1_NUM_BANKS] = {
   192		/*         gpio   inte    ints     rio    pads */
   193		{  0, 28, 0x0000, 0x011c, 0x0124, 0x0000, 0x0004 },
   194		{ 28,  6, 0x4000, 0x411c, 0x4124, 0x4000, 0x4004 },
   195		{ 34, 20, 0x8000, 0x811c, 0x8124, 0x8000, 0x8004 },
   196	};
   197	
   198	static int rp1_pinconf_set(struct rp1_pin_info *pin,
   199				   unsigned int offset, unsigned long *configs,
   200				   unsigned int num_configs);
   201	
   202	static struct rp1_pin_info *rp1_get_pin(struct gpio_chip *chip,
   203						unsigned int offset)
   204	{
   205		struct rp1_pinctrl *pc = gpiochip_get_data(chip);
   206	
   207		if (pc && offset < RP1_NUM_GPIOS)
   208			return &pc->pins[offset];
   209		return NULL;
   210	}
   211	
   212	static void rp1_pad_update(struct rp1_pin_info *pin, u32 clr, u32 set)
   213	{
   214		u32 padctrl = readl(pin->pad);
   215	
   216		padctrl &= ~clr;
   217		padctrl |= set;
   218	
   219		writel(padctrl, pin->pad);
   220	}
   221	
   222	static void rp1_input_enable(struct rp1_pin_info *pin, int value)
   223	{
   224		rp1_pad_update(pin, RP1_PAD_IN_ENABLE_MASK,
   225			       value ? RP1_PAD_IN_ENABLE_MASK : 0);
   226	}
   227	
   228	static void rp1_output_enable(struct rp1_pin_info *pin, int value)
   229	{
   230		rp1_pad_update(pin, RP1_PAD_OUT_DISABLE_MASK,
   231			       value ? 0 : RP1_PAD_OUT_DISABLE_MASK);
   232	}
   233	
   234	static u32 rp1_get_fsel(struct rp1_pin_info *pin)
   235	{
   236		u32 ctrl = readl(pin->gpio + RP1_GPIO_CTRL);
 > 237		u32 oeover = FIELD_GET(RP1_GPIO_CTRL_OEOVER_MASK, ctrl);
   238		u32 fsel = FIELD_GET(RP1_GPIO_CTRL_FUNCSEL_MASK, ctrl);
   239	
   240		if (oeover != RP1_OEOVER_PERI || fsel >= RP1_FSEL_COUNT)
   241			fsel = RP1_FSEL_NONE;
   242	
   243		return fsel;
   244	}
   245
Simon Horman Aug. 21, 2024, 1:27 p.m. UTC | #4
On Tue, Aug 20, 2024 at 04:36:09PM +0200, Andrea della Porta wrote:
> The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
> Add minimum support for the gpio only portion. The driver is in
> pinctrl folder since upcoming patches will add the pinmux/pinctrl
> support where the gpio part can be seen as an addition.
> 
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>

...

> diff --git a/drivers/pinctrl/pinctrl-rp1.c b/drivers/pinctrl/pinctrl-rp1.c

...

> +const struct rp1_iobank_desc rp1_iobanks[RP1_NUM_BANKS] = {
> +	/*         gpio   inte    ints     rio    pads */
> +	{  0, 28, 0x0000, 0x011c, 0x0124, 0x0000, 0x0004 },
> +	{ 28,  6, 0x4000, 0x411c, 0x4124, 0x4000, 0x4004 },
> +	{ 34, 20, 0x8000, 0x811c, 0x8124, 0x8000, 0x8004 },
> +};

rp1_iobanks seems to only be used in this file.
If so, it should be static.

Flagged by Sparse.

...
kernel test robot Aug. 21, 2024, 8:51 p.m. UTC | #5
Hi Andrea,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.11-rc4 next-20240821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrea-della-Porta/dt-bindings-clock-Add-RaspberryPi-RP1-clock-bindings/20240821-023901
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/eb39a5f3cefff2a1240a18a255dac090af16f223.1724159867.git.andrea.porta%40suse.com
patch subject: [PATCH 07/11] pinctrl: rp1: Implement RaspberryPi RP1 gpio support
config: powerpc64-randconfig-r123-20240821 (https://download.01.org/0day-ci/archive/20240822/202408220406.czlwSpkP-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce: (https://download.01.org/0day-ci/archive/20240822/202408220406.czlwSpkP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408220406.czlwSpkP-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/pinctrl/pinctrl-rp1.c:191:30: sparse: sparse: symbol 'rp1_iobanks' was not declared. Should it be static?

vim +/rp1_iobanks +191 drivers/pinctrl/pinctrl-rp1.c

   190	
 > 191	const struct rp1_iobank_desc rp1_iobanks[RP1_NUM_BANKS] = {
   192		/*         gpio   inte    ints     rio    pads */
   193		{  0, 28, 0x0000, 0x011c, 0x0124, 0x0000, 0x0004 },
   194		{ 28,  6, 0x4000, 0x411c, 0x4124, 0x4000, 0x4004 },
   195		{ 34, 20, 0x8000, 0x811c, 0x8124, 0x8000, 0x8004 },
   196	};
   197
Andrea della Porta Aug. 23, 2024, 5:16 p.m. UTC | #6
On 14:27 Wed 21 Aug     , Simon Horman wrote:
> On Tue, Aug 20, 2024 at 04:36:09PM +0200, Andrea della Porta wrote:
> > The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
> > Add minimum support for the gpio only portion. The driver is in
> > pinctrl folder since upcoming patches will add the pinmux/pinctrl
> > support where the gpio part can be seen as an addition.
> > 
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> 
> ...
> 
> > diff --git a/drivers/pinctrl/pinctrl-rp1.c b/drivers/pinctrl/pinctrl-rp1.c
> 
> ...
> 
> > +const struct rp1_iobank_desc rp1_iobanks[RP1_NUM_BANKS] = {
> > +	/*         gpio   inte    ints     rio    pads */
> > +	{  0, 28, 0x0000, 0x011c, 0x0124, 0x0000, 0x0004 },
> > +	{ 28,  6, 0x4000, 0x411c, 0x4124, 0x4000, 0x4004 },
> > +	{ 34, 20, 0x8000, 0x811c, 0x8124, 0x8000, 0x8004 },
> > +};
> 
> rp1_iobanks seems to only be used in this file.
> If so, it should be static.

Fixed, thanks.

> 
> Flagged by Sparse.
> 
> ...
Linus Walleij Aug. 26, 2024, 8:59 a.m. UTC | #7
Hi Andrea,

thanks for your patch!

On Tue, Aug 20, 2024 at 4:36 PM Andrea della Porta
<andrea.porta@suse.com> wrote:

> The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
> Add minimum support for the gpio only portion. The driver is in
> pinctrl folder since upcoming patches will add the pinmux/pinctrl
> support where the gpio part can be seen as an addition.
>
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
(...)

> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
(...)

> +static void rp1_pad_update(struct rp1_pin_info *pin, u32 clr, u32 set)
> +{
> +       u32 padctrl = readl(pin->pad);
> +
> +       padctrl &= ~clr;
> +       padctrl |= set;
> +
> +       writel(padctrl, pin->pad);
> +}

Looks a bit like a reimplementation of regmap-mmio? If you want to do
this why not use regmap-mmio?

> +static void rp1_set_dir(struct rp1_pin_info *pin, bool is_input)
> +{
> +       int offset = is_input ? RP1_CLR_OFFSET : RP1_SET_OFFSET;
> +
> +       writel(1 << pin->offset, pin->rio + RP1_RIO_OE + offset);

If you include bitops.h what about:

writel(BIT(pin->offset), pin->rio + RP1_RIO_OE + offset);

> +static int rp1_get_value(struct rp1_pin_info *pin)
> +{
> +       return !!(readl(pin->rio + RP1_RIO_IN) & (1 << pin->offset));
> +}

Also here

> +
> +static void rp1_set_value(struct rp1_pin_info *pin, int value)
> +{
> +       /* Assume the pin is already an output */
> +       writel(1 << pin->offset,
> +              pin->rio + RP1_RIO_OUT + (value ? RP1_SET_OFFSET : RP1_CLR_OFFSET));
> +}

And here

> +static int rp1_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> +                              unsigned long config)
> +{
> +       struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
> +       unsigned long configs[] = { config };
> +
> +       return rp1_pinconf_set(pin, offset, configs,
> +                              ARRAY_SIZE(configs));
> +}

Nice that you implement this!

> +static void rp1_gpio_irq_config(struct rp1_pin_info *pin, bool enable)
> +{
> +       writel(1 << pin->offset,
> +              pin->inte + (enable ? RP1_SET_OFFSET : RP1_CLR_OFFSET));

BIT()

Yours,
Linus Walleij
Andrea della Porta Aug. 28, 2024, 3:24 p.m. UTC | #8
Hi Linus,

On 10:59 Mon 26 Aug     , Linus Walleij wrote:
> Hi Andrea,
> 
> thanks for your patch!

Thanks for your review!

> 
> On Tue, Aug 20, 2024 at 4:36 PM Andrea della Porta
> <andrea.porta@suse.com> wrote:
> 
> > The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
> > Add minimum support for the gpio only portion. The driver is in
> > pinctrl folder since upcoming patches will add the pinmux/pinctrl
> > support where the gpio part can be seen as an addition.
> >
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> (...)
> 
> > +#include <linux/bitmap.h>
> > +#include <linux/bitops.h>
> (...)
> 
> > +static void rp1_pad_update(struct rp1_pin_info *pin, u32 clr, u32 set)
> > +{
> > +       u32 padctrl = readl(pin->pad);
> > +
> > +       padctrl &= ~clr;
> > +       padctrl |= set;
> > +
> > +       writel(padctrl, pin->pad);
> > +}
> 
> Looks a bit like a reimplementation of regmap-mmio? If you want to do
> this why not use regmap-mmio?

Agreed. I can leverage regmail_field to get rid of the reimplemented code
for the pin->pad register region. Do you think it could be worth using
regmap-mmio also on pin->gpio, pin->inte, pin->ints and pin->rio even
though they are not doing any special field manipulation as the pin->pad
case? 

> 
> > +static void rp1_set_dir(struct rp1_pin_info *pin, bool is_input)
> > +{
> > +       int offset = is_input ? RP1_CLR_OFFSET : RP1_SET_OFFSET;
> > +
> > +       writel(1 << pin->offset, pin->rio + RP1_RIO_OE + offset);
> 
> If you include bitops.h what about:
> 
> writel(BIT(pin->offset), pin->rio + RP1_RIO_OE + offset);

Ack.

> 
> > +static int rp1_get_value(struct rp1_pin_info *pin)
> > +{
> > +       return !!(readl(pin->rio + RP1_RIO_IN) & (1 << pin->offset));
> > +}
> 
> Also here

Ack.

> 
> > +
> > +static void rp1_set_value(struct rp1_pin_info *pin, int value)
> > +{
> > +       /* Assume the pin is already an output */
> > +       writel(1 << pin->offset,
> > +              pin->rio + RP1_RIO_OUT + (value ? RP1_SET_OFFSET : RP1_CLR_OFFSET));
> > +}
> 
> And here

Ack.

> 
> > +static int rp1_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> > +                              unsigned long config)
> > +{
> > +       struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
> > +       unsigned long configs[] = { config };
> > +
> > +       return rp1_pinconf_set(pin, offset, configs,
> > +                              ARRAY_SIZE(configs));
> > +}
> 
> Nice that you implement this!

Thanks :)

> 
> > +static void rp1_gpio_irq_config(struct rp1_pin_info *pin, bool enable)
> > +{
> > +       writel(1 << pin->offset,
> > +              pin->inte + (enable ? RP1_SET_OFFSET : RP1_CLR_OFFSET));
> 
> BIT()

Ack.

Many thanks,
Andrea

> 
> Yours,
> Linus Walleij
Andrea della Porta Aug. 30, 2024, 10:39 a.m. UTC | #9
Hi Krzysztof,

On 10:45 Wed 21 Aug     , Krzysztof Kozlowski wrote:
> On Tue, Aug 20, 2024 at 04:36:09PM +0200, Andrea della Porta wrote:
> > The RP1 is an MFD supporting a gpio controller and /pinmux/pinctrl.
> > Add minimum support for the gpio only portion. The driver is in
> > pinctrl folder since upcoming patches will add the pinmux/pinctrl
> > support where the gpio part can be seen as an addition.
> > 
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > ---
> >  MAINTAINERS                   |   1 +
> >  drivers/pinctrl/Kconfig       |  10 +
> >  drivers/pinctrl/Makefile      |   1 +
> >  drivers/pinctrl/pinctrl-rp1.c | 719 ++++++++++++++++++++++++++++++++++
> >  4 files changed, 731 insertions(+)
> >  create mode 100644 drivers/pinctrl/pinctrl-rp1.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 4ce7b049d67e..67f460c36ea1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -19122,6 +19122,7 @@ S:	Maintained
> >  F:	Documentation/devicetree/bindings/clock/raspberrypi,rp1-clocks.yaml
> >  F:	Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
> >  F:	drivers/clk/clk-rp1.c
> > +F:	drivers/pinctrl/pinctrl-rp1.c
> >  F:	include/dt-bindings/clock/rp1.h
> >  F:	include/dt-bindings/misc/rp1.h
> >  
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index 7e4f93a3bc7a..18bb1a8bd102 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -565,6 +565,16 @@ config PINCTRL_MLXBF3
> >  	  each pin. This driver can also be built as a module called
> >  	  pinctrl-mlxbf3.
> >  
> > +config PINCTRL_RP1
> > +	bool "Pinctrl driver for RP1"
> > +	select PINMUX
> > +	select PINCONF
> > +	select GENERIC_PINCONF
> > +	select GPIOLIB_IRQCHIP
> > +	help
> > +	  Enable the gpio and pinctrl/mux  driver for RaspberryPi RP1
> > +	  multi function device. 
> > +
> >  source "drivers/pinctrl/actions/Kconfig"
> >  source "drivers/pinctrl/aspeed/Kconfig"
> >  source "drivers/pinctrl/bcm/Kconfig"
> > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> > index cc809669405a..f1ca23b563f6 100644
> > --- a/drivers/pinctrl/Makefile
> > +++ b/drivers/pinctrl/Makefile
> > @@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
> >  obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
> >  obj-$(CONFIG_PINCTRL_RK805)	+= pinctrl-rk805.o
> >  obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
> > +obj-$(CONFIG_PINCTRL_RP1)       += pinctrl-rp1.o
> >  obj-$(CONFIG_PINCTRL_SCMI)	+= pinctrl-scmi.o
> >  obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
> >  obj-$(CONFIG_PINCTRL_ST) 	+= pinctrl-st.o
> > diff --git a/drivers/pinctrl/pinctrl-rp1.c b/drivers/pinctrl/pinctrl-rp1.c
> > new file mode 100644
> > index 000000000000..c035d2014505
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinctrl-rp1.c
> > @@ -0,0 +1,719 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Raspberry Pi RP1 GPIO unit
> > + *
> > + * Copyright (C) 2023 Raspberry Pi Ltd.
> > + *
> > + * This driver is inspired by:
> > + * pinctrl-bcm2835.c, please see original file for copyright information
> > + */
> > +
> > +#include <linux/bitmap.h>
> > +#include <linux/bitops.h>
> > +#include <linux/bug.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/gpio/driver.h>
> > +#include <linux/io.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqdesc.h>
> > +#include <linux/init.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> 
> Half of these headers are not used. Drop them.

Ack.

Many thanks,
Andrea

> 
> Best regards,
> Krzysztof
>
Linus Walleij Sept. 2, 2024, 8:31 a.m. UTC | #10
On Wed, Aug 28, 2024 at 5:24 PM Andrea della Porta
<andrea.porta@suse.com> wrote:

> > Looks a bit like a reimplementation of regmap-mmio? If you want to do
> > this why not use regmap-mmio?
>
> Agreed. I can leverage regmail_field to get rid of the reimplemented code
> for the pin->pad register region. Do you think it could be worth using
> regmap-mmio also on pin->gpio, pin->inte, pin->ints and pin->rio even
> though they are not doing any special field manipulation as the pin->pad
> case?

Don't know without looking at the result, I bet you will see what
looks best when you edit the patch, let's see what you come
up with, I trust you on this.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ce7b049d67e..67f460c36ea1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19122,6 +19122,7 @@  S:	Maintained
 F:	Documentation/devicetree/bindings/clock/raspberrypi,rp1-clocks.yaml
 F:	Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
 F:	drivers/clk/clk-rp1.c
+F:	drivers/pinctrl/pinctrl-rp1.c
 F:	include/dt-bindings/clock/rp1.h
 F:	include/dt-bindings/misc/rp1.h
 
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 7e4f93a3bc7a..18bb1a8bd102 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -565,6 +565,16 @@  config PINCTRL_MLXBF3
 	  each pin. This driver can also be built as a module called
 	  pinctrl-mlxbf3.
 
+config PINCTRL_RP1
+	bool "Pinctrl driver for RP1"
+	select PINMUX
+	select PINCONF
+	select GENERIC_PINCONF
+	select GPIOLIB_IRQCHIP
+	help
+	  Enable the gpio and pinctrl/mux  driver for RaspberryPi RP1
+	  multi function device. 
+
 source "drivers/pinctrl/actions/Kconfig"
 source "drivers/pinctrl/aspeed/Kconfig"
 source "drivers/pinctrl/bcm/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index cc809669405a..f1ca23b563f6 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@  obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
 obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
 obj-$(CONFIG_PINCTRL_RK805)	+= pinctrl-rk805.o
 obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
+obj-$(CONFIG_PINCTRL_RP1)       += pinctrl-rp1.o
 obj-$(CONFIG_PINCTRL_SCMI)	+= pinctrl-scmi.o
 obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
 obj-$(CONFIG_PINCTRL_ST) 	+= pinctrl-st.o
diff --git a/drivers/pinctrl/pinctrl-rp1.c b/drivers/pinctrl/pinctrl-rp1.c
new file mode 100644
index 000000000000..c035d2014505
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rp1.c
@@ -0,0 +1,719 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Raspberry Pi RP1 GPIO unit
+ *
+ * Copyright (C) 2023 Raspberry Pi Ltd.
+ *
+ * This driver is inspired by:
+ * pinctrl-bcm2835.c, please see original file for copyright information
+ */
+
+#include <linux/bitmap.h>
+#include <linux/bitops.h>
+#include <linux/bug.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/init.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define MODULE_NAME "pinctrl-rp1"
+#define RP1_NUM_GPIOS	54
+#define RP1_NUM_BANKS	3
+
+#define RP1_RW_OFFSET			0x0000
+#define RP1_XOR_OFFSET			(RP1_RW_OFFSET + 0x1000)
+#define RP1_SET_OFFSET			(RP1_RW_OFFSET + 0x2000)
+#define RP1_CLR_OFFSET			(RP1_RW_OFFSET + 0x3000)
+
+#define RP1_GPIO_REG_OFFSET		0x0000
+#define RP1_GPIO_STATUS			(RP1_GPIO_REG_OFFSET + 0x0000)
+#define RP1_GPIO_CTRL			(RP1_GPIO_REG_OFFSET + 0x0004)
+
+#define RP1_GPIO_EVENTS_SHIFT_RAW	20
+
+#define RP1_GPIO_CTRL_FUNCSEL_LSB	0
+#define RP1_GPIO_CTRL_FUNCSEL_MASK	GENMASK(RP1_GPIO_CTRL_FUNCSEL_LSB + 4, \
+						RP1_GPIO_CTRL_FUNCSEL_LSB)
+#define RP1_GPIO_CTRL_OUTOVER_LSB	12
+#define RP1_GPIO_CTRL_OUTOVER_MASK	GENMASK(RP1_GPIO_CTRL_OUTOVER_LSB + 1, \
+						RP1_GPIO_CTRL_OUTOVER_LSB)
+#define RP1_GPIO_CTRL_OEOVER_LSB	14
+#define RP1_GPIO_CTRL_OEOVER_MASK	GENMASK(RP1_GPIO_CTRL_OEOVER_LSB + 1, \
+						RP1_GPIO_CTRL_OEOVER_LSB)
+#define RP1_GPIO_CTRL_INOVER_LSB	16
+#define RP1_GPIO_CTRL_INOVER_MASK	GENMASK(RP1_GPIO_CTRL_INOVER_LSB + 1, \
+						RP1_GPIO_CTRL_INOVER_LSB)
+#define RP1_GPIO_CTRL_IRQEN_FALLING	BIT(20)
+#define RP1_GPIO_CTRL_IRQEN_RISING	BIT(21)
+#define RP1_GPIO_CTRL_IRQEN_LOW		BIT(22)
+#define RP1_GPIO_CTRL_IRQEN_HIGH	BIT(23)
+#define RP1_GPIO_CTRL_IRQEN_F_FALLING	BIT(24)
+#define RP1_GPIO_CTRL_IRQEN_F_RISING	BIT(25)
+#define RP1_GPIO_CTRL_IRQEN_F_LOW	BIT(26)
+#define RP1_GPIO_CTRL_IRQEN_F_HIGH	BIT(27)
+#define RP1_GPIO_CTRL_IRQRESET		BIT(28)
+#define RP1_GPIO_CTRL_IRQOVER_LSB	30
+#define RP1_GPIO_CTRL_IRQOVER_MASK	GENMASK(RP1_GPIO_CTRL_IRQOVER_LSB + 1, \
+						RP1_GPIO_CTRL_IRQOVER_LSB)
+
+#define RP1_INT_EDGE_FALLING		BIT(0)
+#define RP1_INT_EDGE_RISING		BIT(1)
+#define RP1_INT_LEVEL_LOW		BIT(2)
+#define RP1_INT_LEVEL_HIGH		BIT(3)
+#define RP1_INT_MASK			GENMASK(3, 0)
+
+#define RP1_INT_EDGE_BOTH		(RP1_INT_EDGE_FALLING |	\
+					 RP1_INT_EDGE_RISING)
+
+enum {
+	RP1_PUD_OFF			= 0,
+	RP1_PUD_DOWN			= 1,
+	RP1_PUD_UP			= 2,
+};
+
+#define RP1_FSEL_COUNT			9
+
+#define RP1_FSEL_ALT0			0x00
+#define RP1_FSEL_GPIO			0x05
+#define RP1_FSEL_NONE			0x09
+#define RP1_FSEL_NONE_HW		0x1f
+
+enum {
+	RP1_DIR_OUTPUT			= 0,
+	RP1_DIR_INPUT			= 1,
+};
+
+enum {
+	RP1_OUTOVER_PERI		= 0,
+	RP1_OUTOVER_INVPERI		= 1,
+	RP1_OUTOVER_LOW			= 2,
+	RP1_OUTOVER_HIGH		= 3,
+};
+
+enum {
+	RP1_OEOVER_PERI			= 0,
+	RP1_OEOVER_INVPERI		= 1,
+	RP1_OEOVER_DISABLE		= 2,
+	RP1_OEOVER_ENABLE		= 3,
+};
+
+enum {
+	RP1_INOVER_PERI			= 0,
+	RP1_INOVER_INVPERI		= 1,
+	RP1_INOVER_LOW			= 2,
+	RP1_INOVER_HIGH			= 3,
+};
+
+#define RP1_RIO_OUT			0x00
+#define RP1_RIO_OE			(RP1_RIO_OUT + 0x04)
+#define RP1_RIO_IN			(RP1_RIO_OUT + 0x08)
+
+#define RP1_PAD_SLEWFAST_LSB		0
+#define RP1_PAD_SLEWFAST_MASK		BIT(RP1_PAD_SLEWFAST_LSB)
+#define RP1_PAD_SCHMITT_LSB		1
+#define RP1_PAD_SCHMITT_MASK		BIT(RP1_PAD_SCHMITT_LSB)
+#define RP1_PAD_PULL_LSB		2
+#define RP1_PAD_PULL_MASK		GENMASK(RP1_PAD_PULL_LSB + 1, \
+						RP1_PAD_PULL_LSB)
+#define RP1_PAD_DRIVE_LSB		4
+#define RP1_PAD_DRIVE_MASK		GENMASK(RP1_PAD_DRIVE_LSB + 1, \
+						RP1_PAD_DRIVE_LSB)
+#define RP1_PAD_IN_ENABLE_LSB		6
+#define RP1_PAD_IN_ENABLE_MASK		BIT(RP1_PAD_IN_ENABLE_LSB)
+#define RP1_PAD_OUT_DISABLE_LSB		7
+#define RP1_PAD_OUT_DISABLE_MASK	BIT(RP1_PAD_OUT_DISABLE_LSB)
+
+#define RP1_PAD_DRIVE_2MA		0x00000000
+#define RP1_PAD_DRIVE_4MA		BIT(4)
+#define RP1_PAD_DRIVE_8MA		BIT(5)
+#define RP1_PAD_DRIVE_12MA		(RP1_PAD_DRIVE_4MA | \
+					RP1_PAD_DRIVE_8MA)
+
+#define FIELD_SET(_reg, _mask, _val)			\
+	({						\
+		_reg &= ~(_mask);				\
+		_reg |= FIELD_PREP((_mask), (_val));	\
+	})
+
+#define FUNC(f) \
+	[func_##f] = #f
+
+struct rp1_iobank_desc {
+	int min_gpio;
+	int num_gpios;
+	int gpio_offset;
+	int inte_offset;
+	int ints_offset;
+	int rio_offset;
+	int pads_offset;
+};
+
+struct rp1_pin_info {
+	u8 num;
+	u8 bank;
+	u8 offset;
+	u8 fsel;
+	u8 irq_type;
+
+	void __iomem *gpio;
+	void __iomem *rio;
+	void __iomem *inte;
+	void __iomem *ints;
+	void __iomem *pad;
+};
+
+struct rp1_pinctrl {
+	struct device *dev;
+	void __iomem *gpio_base;
+	void __iomem *rio_base;
+	void __iomem *pads_base;
+	int irq[RP1_NUM_BANKS];
+	struct rp1_pin_info pins[RP1_NUM_GPIOS];
+
+	struct pinctrl_dev *pctl_dev;
+	struct gpio_chip gpio_chip;
+	struct pinctrl_gpio_range gpio_range;
+
+	raw_spinlock_t irq_lock[RP1_NUM_BANKS];
+};
+
+const struct rp1_iobank_desc rp1_iobanks[RP1_NUM_BANKS] = {
+	/*         gpio   inte    ints     rio    pads */
+	{  0, 28, 0x0000, 0x011c, 0x0124, 0x0000, 0x0004 },
+	{ 28,  6, 0x4000, 0x411c, 0x4124, 0x4000, 0x4004 },
+	{ 34, 20, 0x8000, 0x811c, 0x8124, 0x8000, 0x8004 },
+};
+
+static int rp1_pinconf_set(struct rp1_pin_info *pin,
+			   unsigned int offset, unsigned long *configs,
+			   unsigned int num_configs);
+
+static struct rp1_pin_info *rp1_get_pin(struct gpio_chip *chip,
+					unsigned int offset)
+{
+	struct rp1_pinctrl *pc = gpiochip_get_data(chip);
+
+	if (pc && offset < RP1_NUM_GPIOS)
+		return &pc->pins[offset];
+	return NULL;
+}
+
+static void rp1_pad_update(struct rp1_pin_info *pin, u32 clr, u32 set)
+{
+	u32 padctrl = readl(pin->pad);
+
+	padctrl &= ~clr;
+	padctrl |= set;
+
+	writel(padctrl, pin->pad);
+}
+
+static void rp1_input_enable(struct rp1_pin_info *pin, int value)
+{
+	rp1_pad_update(pin, RP1_PAD_IN_ENABLE_MASK,
+		       value ? RP1_PAD_IN_ENABLE_MASK : 0);
+}
+
+static void rp1_output_enable(struct rp1_pin_info *pin, int value)
+{
+	rp1_pad_update(pin, RP1_PAD_OUT_DISABLE_MASK,
+		       value ? 0 : RP1_PAD_OUT_DISABLE_MASK);
+}
+
+static u32 rp1_get_fsel(struct rp1_pin_info *pin)
+{
+	u32 ctrl = readl(pin->gpio + RP1_GPIO_CTRL);
+	u32 oeover = FIELD_GET(RP1_GPIO_CTRL_OEOVER_MASK, ctrl);
+	u32 fsel = FIELD_GET(RP1_GPIO_CTRL_FUNCSEL_MASK, ctrl);
+
+	if (oeover != RP1_OEOVER_PERI || fsel >= RP1_FSEL_COUNT)
+		fsel = RP1_FSEL_NONE;
+
+	return fsel;
+}
+
+static void rp1_set_fsel(struct rp1_pin_info *pin, u32 fsel)
+{
+	u32 ctrl = readl(pin->gpio + RP1_GPIO_CTRL);
+
+	if (fsel >= RP1_FSEL_COUNT)
+		fsel = RP1_FSEL_NONE_HW;
+
+	rp1_input_enable(pin, 1);
+	rp1_output_enable(pin, 1);
+
+	if (fsel == RP1_FSEL_NONE) {
+		FIELD_SET(ctrl, RP1_GPIO_CTRL_OEOVER_MASK, RP1_OEOVER_DISABLE);
+	} else {
+		FIELD_SET(ctrl, RP1_GPIO_CTRL_OUTOVER_MASK, RP1_OUTOVER_PERI);
+		FIELD_SET(ctrl, RP1_GPIO_CTRL_OEOVER_MASK, RP1_OEOVER_PERI);
+	}
+
+	FIELD_SET(ctrl, RP1_GPIO_CTRL_FUNCSEL_MASK, fsel);
+	writel(ctrl, pin->gpio + RP1_GPIO_CTRL);
+}
+
+static int rp1_get_dir(struct rp1_pin_info *pin)
+{
+	return !(readl(pin->rio + RP1_RIO_OE) & (1 << pin->offset)) ?
+		RP1_DIR_INPUT : RP1_DIR_OUTPUT;
+}
+
+static void rp1_set_dir(struct rp1_pin_info *pin, bool is_input)
+{
+	int offset = is_input ? RP1_CLR_OFFSET : RP1_SET_OFFSET;
+
+	writel(1 << pin->offset, pin->rio + RP1_RIO_OE + offset);
+}
+
+static int rp1_get_value(struct rp1_pin_info *pin)
+{
+	return !!(readl(pin->rio + RP1_RIO_IN) & (1 << pin->offset));
+}
+
+static void rp1_set_value(struct rp1_pin_info *pin, int value)
+{
+	/* Assume the pin is already an output */
+	writel(1 << pin->offset,
+	       pin->rio + RP1_RIO_OUT + (value ? RP1_SET_OFFSET : RP1_CLR_OFFSET));
+}
+
+static int rp1_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
+	int ret;
+
+	if (!pin)
+		return -EINVAL;
+
+	ret = rp1_get_value(pin);
+
+	return ret;
+}
+
+static void rp1_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
+
+	if (pin)
+		rp1_set_value(pin, value);
+}
+
+static int rp1_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
+	u32 fsel;
+
+	if (!pin)
+		return -EINVAL;
+
+	fsel = rp1_get_fsel(pin);
+	if (fsel != RP1_FSEL_GPIO)
+		return -EINVAL;
+
+	return (rp1_get_dir(pin) == RP1_DIR_OUTPUT) ?
+		GPIO_LINE_DIRECTION_OUT :
+		GPIO_LINE_DIRECTION_IN;
+}
+
+static int rp1_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
+
+	if (!pin)
+		return -EINVAL;
+	rp1_set_dir(pin, RP1_DIR_INPUT);
+	rp1_set_fsel(pin, RP1_FSEL_GPIO);
+
+	return 0;
+}
+
+static int rp1_gpio_direction_output(struct gpio_chip *chip, unsigned int offset,
+				     int value)
+{
+	struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
+
+	if (!pin)
+		return -EINVAL;
+	rp1_set_value(pin, value);
+	rp1_set_dir(pin, RP1_DIR_OUTPUT);
+	rp1_set_fsel(pin, RP1_FSEL_GPIO);
+
+	return 0;
+}
+
+static int rp1_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
+			       unsigned long config)
+{
+	struct rp1_pin_info *pin = rp1_get_pin(chip, offset);
+	unsigned long configs[] = { config };
+
+	return rp1_pinconf_set(pin, offset, configs,
+			       ARRAY_SIZE(configs));
+}
+
+static const struct gpio_chip rp1_gpio_chip = {
+	.label = MODULE_NAME,
+	.owner = THIS_MODULE,
+	.request = gpiochip_generic_request,
+	.free = gpiochip_generic_free,
+	.direction_input = rp1_gpio_direction_input,
+	.direction_output = rp1_gpio_direction_output,
+	.get_direction = rp1_gpio_get_direction,
+	.get = rp1_gpio_get,
+	.set = rp1_gpio_set,
+	.base = -1,
+	.set_config = rp1_gpio_set_config,
+	.ngpio = RP1_NUM_GPIOS,
+	.can_sleep = false,
+};
+
+static void rp1_gpio_irq_handler(struct irq_desc *desc)
+{
+	struct gpio_chip *chip = irq_desc_get_handler_data(desc);
+	struct irq_chip *host_chip = irq_desc_get_chip(desc);
+	struct rp1_pinctrl *pc = gpiochip_get_data(chip);
+	const struct rp1_iobank_desc *bank;
+	int irq = irq_desc_get_irq(desc);
+	unsigned long ints;
+	int bit_pos;
+
+	if (pc->irq[0] == irq)
+		bank = &rp1_iobanks[0];
+	else if (pc->irq[1] == irq)
+		bank = &rp1_iobanks[1];
+	else
+		bank = &rp1_iobanks[2];
+
+	chained_irq_enter(host_chip, desc);
+
+	ints = readl(pc->gpio_base + bank->ints_offset);
+	for_each_set_bit(bit_pos, &ints, 32) {
+		struct rp1_pin_info *pin = rp1_get_pin(chip, bit_pos);
+
+		writel(RP1_GPIO_CTRL_IRQRESET,
+		       pin->gpio + RP1_SET_OFFSET + RP1_GPIO_CTRL);
+		generic_handle_irq(irq_linear_revmap(pc->gpio_chip.irq.domain,
+						     bank->gpio_offset + bit_pos));
+	}
+
+	chained_irq_exit(host_chip, desc);
+}
+
+static void rp1_gpio_irq_config(struct rp1_pin_info *pin, bool enable)
+{
+	writel(1 << pin->offset,
+	       pin->inte + (enable ? RP1_SET_OFFSET : RP1_CLR_OFFSET));
+	if (!enable)
+		/* Clear any latched events */
+		writel(RP1_GPIO_CTRL_IRQRESET,
+		       pin->gpio + RP1_SET_OFFSET + RP1_GPIO_CTRL);
+}
+
+static void rp1_gpio_irq_enable(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	unsigned int gpio = irqd_to_hwirq(data);
+	struct rp1_pin_info *pin = rp1_get_pin(chip, gpio);
+
+	rp1_gpio_irq_config(pin, true);
+}
+
+static void rp1_gpio_irq_disable(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	unsigned int gpio = irqd_to_hwirq(data);
+	struct rp1_pin_info *pin = rp1_get_pin(chip, gpio);
+
+	rp1_gpio_irq_config(pin, false);
+}
+
+static int rp1_irq_set_type(struct rp1_pin_info *pin, unsigned int type)
+{
+	u32 irq_flags;
+
+	switch (type) {
+	case IRQ_TYPE_NONE:
+		irq_flags = 0;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		irq_flags = RP1_INT_EDGE_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		irq_flags = RP1_INT_EDGE_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		irq_flags = RP1_INT_EDGE_RISING | RP1_INT_EDGE_FALLING;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		irq_flags = RP1_INT_LEVEL_HIGH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		irq_flags = RP1_INT_LEVEL_LOW;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	/* Clear them all */
+	writel(RP1_INT_MASK << RP1_GPIO_EVENTS_SHIFT_RAW,
+	       pin->gpio + RP1_CLR_OFFSET + RP1_GPIO_CTRL);
+	/* Set those that are needed */
+	writel(irq_flags << RP1_GPIO_EVENTS_SHIFT_RAW,
+	       pin->gpio + RP1_SET_OFFSET + RP1_GPIO_CTRL);
+	pin->irq_type = type;
+
+	return 0;
+}
+
+static int rp1_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	unsigned int gpio = irqd_to_hwirq(data);
+	struct rp1_pin_info *pin = rp1_get_pin(chip, gpio);
+	struct rp1_pinctrl *pc = gpiochip_get_data(chip);
+	int bank = pin->bank;
+	unsigned long flags;
+	int ret;
+
+	raw_spin_lock_irqsave(&pc->irq_lock[bank], flags);
+
+	ret = rp1_irq_set_type(pin, type);
+	if (!ret) {
+		if (type & IRQ_TYPE_EDGE_BOTH)
+			irq_set_handler_locked(data, handle_edge_irq);
+		else
+			irq_set_handler_locked(data, handle_level_irq);
+	}
+
+	raw_spin_unlock_irqrestore(&pc->irq_lock[bank], flags);
+
+	return ret;
+}
+
+static void rp1_gpio_irq_ack(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	unsigned int gpio = irqd_to_hwirq(data);
+	struct rp1_pin_info *pin = rp1_get_pin(chip, gpio);
+
+	/* Clear any latched events */
+	writel(RP1_GPIO_CTRL_IRQRESET, pin->gpio + RP1_SET_OFFSET + RP1_GPIO_CTRL);
+}
+
+static struct irq_chip rp1_gpio_irq_chip = {
+	.name = MODULE_NAME,
+	.irq_enable = rp1_gpio_irq_enable,
+	.irq_disable = rp1_gpio_irq_disable,
+	.irq_set_type = rp1_gpio_irq_set_type,
+	.irq_ack = rp1_gpio_irq_ack,
+	.irq_mask = rp1_gpio_irq_disable,
+	.irq_unmask = rp1_gpio_irq_enable,
+	.flags = IRQCHIP_IMMUTABLE,
+};
+
+static void rp1_pull_config_set(struct rp1_pin_info *pin, unsigned int arg)
+{
+	u32 padctrl = readl(pin->pad);
+
+	FIELD_SET(padctrl, RP1_PAD_PULL_MASK, arg & 0x3);
+	writel(padctrl, pin->pad);
+}
+
+static int rp1_pinconf_set(struct rp1_pin_info *pin, unsigned int offset,
+			   unsigned long *configs, unsigned int num_configs)
+{
+	u32 param, arg;
+	int i;
+
+	if (!pin)
+		return -EINVAL;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			rp1_pull_config_set(pin, RP1_PUD_OFF);
+			break;
+
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			rp1_pull_config_set(pin, RP1_PUD_DOWN);
+			break;
+
+		case PIN_CONFIG_BIAS_PULL_UP:
+			rp1_pull_config_set(pin, RP1_PUD_UP);
+			break;
+
+		case PIN_CONFIG_INPUT_ENABLE:
+			rp1_input_enable(pin, arg);
+			break;
+
+		case PIN_CONFIG_OUTPUT_ENABLE:
+			rp1_output_enable(pin, arg);
+			break;
+
+		case PIN_CONFIG_OUTPUT:
+			rp1_set_value(pin, arg);
+			rp1_set_dir(pin, RP1_DIR_OUTPUT);
+			rp1_set_fsel(pin, RP1_FSEL_GPIO);
+			break;
+
+		case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+			rp1_pad_update(pin, RP1_PAD_SCHMITT_MASK,
+				       arg ? RP1_PAD_SCHMITT_MASK : 0);
+			break;
+
+		case PIN_CONFIG_SLEW_RATE:
+			rp1_pad_update(pin, RP1_PAD_SLEWFAST_MASK,
+				       arg ? RP1_PAD_SLEWFAST_MASK : 0);
+			break;
+
+		case PIN_CONFIG_DRIVE_STRENGTH:
+			switch (arg) {
+			case 2:
+				arg = RP1_PAD_DRIVE_2MA;
+				break;
+			case 4:
+				arg = RP1_PAD_DRIVE_4MA;
+				break;
+			case 8:
+				arg = RP1_PAD_DRIVE_8MA;
+				break;
+			case 12:
+				arg = RP1_PAD_DRIVE_12MA;
+				break;
+			default:
+				return -ENOTSUPP;
+			}
+			rp1_pad_update(pin, RP1_PAD_DRIVE_MASK, arg);
+			break;
+
+		default:
+			return -ENOTSUPP;
+
+		} /* switch param type */
+	} /* for each config */
+
+	return 0;
+}
+
+static const struct of_device_id rp1_pinctrl_match[] = {
+	{ .compatible = "raspberrypi,rp1-gpio" },
+	{},
+};
+
+static int rp1_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct gpio_irq_chip *girq;
+	struct rp1_pinctrl *pc;
+	int err, i;
+
+	pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+	if (!pc)
+		return -ENOMEM;
+
+	pc->dev = dev;
+	pc->gpio_chip = rp1_gpio_chip;
+	pc->gpio_chip.parent = dev;
+
+	pc->gpio_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pc->gpio_base)) {
+		dev_err(dev, "could not get GPIO IO memory\n");
+		return PTR_ERR(pc->gpio_base);
+	}
+
+	pc->rio_base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(pc->rio_base)) {
+		dev_err(dev, "could not get RIO IO memory\n");
+		return PTR_ERR(pc->rio_base);
+	}
+
+	pc->pads_base = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(pc->pads_base)) {
+		dev_err(dev, "could not get PADS IO memory\n");
+		return PTR_ERR(pc->pads_base);
+	}
+
+	for (i = 0; i < RP1_NUM_BANKS; i++) {
+		const struct rp1_iobank_desc *bank = &rp1_iobanks[i];
+		int j;
+
+		for (j = 0; j < bank->num_gpios; j++) {
+			struct rp1_pin_info *pin =
+				&pc->pins[bank->min_gpio + j];
+
+			pin->num = bank->min_gpio + j;
+			pin->bank = i;
+			pin->offset = j;
+
+			pin->gpio = pc->gpio_base + bank->gpio_offset +
+				    j * sizeof(u32) * 2;
+			pin->inte = pc->gpio_base + bank->inte_offset;
+			pin->ints = pc->gpio_base + bank->ints_offset;
+			pin->rio  = pc->rio_base + bank->rio_offset;
+			pin->pad  = pc->pads_base + bank->pads_offset +
+				    j * sizeof(u32);
+		}
+
+		raw_spin_lock_init(&pc->irq_lock[i]);
+	}
+
+	girq = &pc->gpio_chip.irq;
+	girq->chip = &rp1_gpio_irq_chip;
+	girq->parent_handler = rp1_gpio_irq_handler;
+	girq->num_parents = RP1_NUM_BANKS;
+	girq->parents = pc->irq;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_level_irq;
+
+	/*
+	 * Use the same handler for all groups: this is necessary
+	 * since we use one gpiochip to cover all lines - the
+	 * irq handler then needs to figure out which group and
+	 * bank that was firing the IRQ and look up the per-group
+	 * and bank data.
+	 */
+	for (i = 0; i < RP1_NUM_BANKS; i++) {
+		pc->irq[i] = irq_of_parse_and_map(np, i);
+		if (!pc->irq[i]) {
+			girq->num_parents = i;
+			break;
+		}
+	}
+
+	platform_set_drvdata(pdev, pc);
+
+	err = devm_gpiochip_add_data(dev, &pc->gpio_chip, pc);
+	if (err) {
+		dev_err(dev, "could not add GPIO chip\n");
+		return err;
+	}
+
+	return 0;
+}
+
+static struct platform_driver rp1_pinctrl_driver = {
+	.probe = rp1_pinctrl_probe,
+	.driver = {
+		.name = MODULE_NAME,
+		.of_match_table = rp1_pinctrl_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(rp1_pinctrl_driver);