[PATCH v3 0/4] Nintendo Wii GPIO driver

[PATCH v3 0/4] Nintendo Wii GPIO driver

[Jonathan Neuschäfer]

v2: https://www.spinics.net/lists/devicetree/msg211283.html

This series adds a driver for the GPIO controller used in the Nintendo
Wii game console.

Previous versions of this series included a patch to kernel/resource.c
("resource: Extend the PPC32 reserved memory hack") to work around a
resource allocation problem on PPC32. In this version, I dropped this
patch, because the problem will be solved differently and in a separate
patchset.

I also dropped the dt-bindings patch, because Linus Walleij has already
applied it.

Jonathan Neuschäfer (4):
  powerpc: wii: Explicitly configure GPIO owner for poweroff pin
  gpio: Add GPIO driver for Nintendo Wii
  powerpc: wii.dts: Add ngpios property
  powerpc: wii.dts: Add GPIO line names

 arch/powerpc/boot/dts/wii.dts            |   9 +++
 arch/powerpc/platforms/embedded6xx/wii.c |   7 ++
 drivers/gpio/Kconfig                     |   9 +++
 drivers/gpio/Makefile                    |   1 +
 drivers/gpio/gpio-hlwd.c                 | 115 +++++++++++++++++++++++++++++++
 5 files changed, 141 insertions(+)
 create mode 100644 drivers/gpio/gpio-hlwd.c

-- 
2.15.1

[PATCH v3 1/4] powerpc: wii: Explicitly configure GPIO owner for poweroff pin

[Jonathan Neuschäfer]

The Hollywood chipset's GPIO controller has two sets of registers: One
for access by the PowerPC CPU, and one for access by the ARM coprocessor
(but both are accessible from the PPC because the memory firewall
(AHBPROT) is usually disabled when booting Linux, today).

The wii_power_off function currently assumes that the poweroff GPIO pin
is configured for use via the ARM side, but the upcoming GPIO driver
configures all pins for use via the PPC side, breaking poweroff.

Configure the owner register explicitly in wii_power_off to make
wii_power_off work with and without the new GPIO driver.

I think the Wii can be switched to the generic gpio-poweroff driver,
after the GPIO driver is merged.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>
---

v2, v3:
- no change
---
 arch/powerpc/platforms/embedded6xx/wii.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
index 160fa09533ba..4682327f76a9 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -45,6 +45,7 @@
 #define HW_GPIO_BASE(idx)	(idx * 0x20)
 #define HW_GPIO_OUT(idx)	(HW_GPIO_BASE(idx) + 0)
 #define HW_GPIO_DIR(idx)	(HW_GPIO_BASE(idx) + 4)
+#define HW_GPIO_OWNER		(HW_GPIO_BASE(1) + 0x1c)
 
 #define HW_GPIO_SHUTDOWN	(1<<1)
 #define HW_GPIO_SLOT_LED	(1<<5)
@@ -177,6 +178,12 @@ static void wii_power_off(void)
 	local_irq_disable();
 
 	if (hw_gpio) {
+		/*
+		 * set the owner of the shutdown pin to ARM, because it is
+		 * accessed through the registers for the ARM, below
+		 */
+		clrbits32(hw_gpio + HW_GPIO_OWNER, HW_GPIO_SHUTDOWN);
+
 		/* make sure that the poweroff GPIO is configured as output */
 		setbits32(hw_gpio + HW_GPIO_DIR(1), HW_GPIO_SHUTDOWN);
 
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v3, 1/4] powerpc: wii: Explicitly configure GPIO owner for poweroff pin

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

On Fri, 2018-02-09 at 12:07:28 UTC, =?utf-8?q?Jonathan_Neusch=C3=A4fer?= wrote:
> The Hollywood chipset's GPIO controller has two sets of registers: One
> for access by the PowerPC CPU, and one for access by the ARM coprocessor
> (but both are accessible from the PPC because the memory firewall
> (AHBPROT) is usually disabled when booting Linux, today).
> 
> The wii_power_off function currently assumes that the poweroff GPIO pin
> is configured for use via the ARM side, but the upcoming GPIO driver
> configures all pins for use via the PPC side, breaking poweroff.
> 
> Configure the owner register explicitly in wii_power_off to make
> wii_power_off work with and without the new GPIO driver.
> 
> I think the Wii can be switched to the generic gpio-poweroff driver,
> after the GPIO driver is merged.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>

Patches 1, 3 and 4 applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/9cbaaec1cf0c9f4861c4c1dd65f3ed

cheers

[PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
that supports a configurable number of pins (up to 32), interrupts, and
some special mechanisms to share the controller between the system's
security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
not supported.

This patch adds a basic driver for this GPIO controller. Interrupt
support will come in a later patch.

This patch is based on code developed by Albert Herranz and the GameCube
Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
has grown quite dissimilar.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>
Cc: Albert Herranz <albert_herranz-mRCrAkd8dF0@public.gmane.org>
Cc: Segher Boessenkool <segher-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
<---

v3:
- Do some style cleanups, as suggest by Andy Shevchenko

v2:
- Change hlwd_gpio_driver.driver.name to "gpio-hlwd" to match the
  filename (was "hlwd_gpio")
- Remove unnecessary include of linux/of_gpio.h, as suggested by Linus
  Walleij.
- Add struct device pointer to context struct to make it possible to use
  dev_info(hlwd->dev, "..."), as suggested by Linus Walleij
- Use the GPIO_GENERIC library to reduce code size, as suggested by
  Linus Walleij
- Use iowrite32be instead of __raw_writel for big-endian MMIO access, as
  suggested by Linus Walleij
- Remove commit message paragraph suggesting to diff against the
  original driver, because it's so different now
---
 drivers/gpio/Kconfig     |   9 ++++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-hlwd.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/gpio/gpio-hlwd.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d6a8e851ad13..47606dfe06cc 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -229,6 +229,15 @@ config GPIO_GRGPIO
 	  Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
 	  VHDL IP core library.
 
+config GPIO_HLWD
+	tristate "Nintendo Wii (Hollywood) GPIO"
+	depends on OF_GPIO
+	select GPIO_GENERIC
+	help
+	  Select this to support the GPIO controller of the Nintendo Wii.
+
+	  If unsure, say N.
+
 config GPIO_ICH
 	tristate "Intel ICH GPIO"
 	depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4bc24febb889..492f62d0eb59 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_FTGPIO010)	+= gpio-ftgpio010.o
 obj-$(CONFIG_GPIO_GE_FPGA)	+= gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM)	+= gpio-gpio-mm.o
 obj-$(CONFIG_GPIO_GRGPIO)	+= gpio-grgpio.o
+obj-$(CONFIG_GPIO_HLWD)		+= gpio-hlwd.o
 obj-$(CONFIG_HTC_EGPIO)		+= gpio-htc-egpio.o
 obj-$(CONFIG_GPIO_ICH)		+= gpio-ich.o
 obj-$(CONFIG_GPIO_INGENIC)	+= gpio-ingenic.o
diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
new file mode 100644
index 000000000000..a63136a68ba3
--- /dev/null
+++ b/drivers/gpio/gpio-hlwd.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (C) 2008-2009 The GameCube Linux Team
+// Copyright (C) 2008,2009 Albert Herranz
+// Copyright (C) 2017-2018 Jonathan Neuschäfer
+//
+// Nintendo Wii (Hollywood) GPIO driver
+
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+/*
+ * Register names and offsets courtesy of WiiBrew:
+ * https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
+ *
+ * Note that for most registers, there are two versions:
+ * - HW_GPIOB_* Is always accessible by the Broadway PowerPC core, but does
+ *   always give access to all GPIO lines
+ * - HW_GPIO_* Is only accessible by the Broadway PowerPC code if the memory
+ *   firewall (AHBPROT) in the Hollywood chipset has been configured to allow
+ *   such access.
+ *
+ * The ownership of each GPIO line can be configured in the HW_GPIO_OWNER
+ * register: A one bit configures the line for access via the HW_GPIOB_*
+ * registers, a zero bit indicates access via HW_GPIO_*. This driver uses
+ * HW_GPIOB_*.
+ */
+#define HW_GPIOB_OUT		0x00
+#define HW_GPIOB_DIR		0x04
+#define HW_GPIOB_IN		0x08
+#define HW_GPIOB_INTLVL		0x0c
+#define HW_GPIOB_INTFLAG	0x10
+#define HW_GPIOB_INTMASK	0x14
+#define HW_GPIOB_INMIR		0x18
+#define HW_GPIO_ENABLE		0x1c
+#define HW_GPIO_OUT		0x20
+#define HW_GPIO_DIR		0x24
+#define HW_GPIO_IN		0x28
+#define HW_GPIO_INTLVL		0x2c
+#define HW_GPIO_INTFLAG		0x30
+#define HW_GPIO_INTMASK		0x34
+#define HW_GPIO_INMIR		0x38
+#define HW_GPIO_OWNER		0x3c
+
+struct hlwd_gpio {
+	struct gpio_chip gpioc;
+	void __iomem *regs;
+};
+
+static int hlwd_gpio_probe(struct platform_device *pdev)
+{
+	struct hlwd_gpio *hlwd;
+	struct resource *regs_resource;
+	u32 ngpios;
+	int res;
+
+	hlwd = devm_kzalloc(&pdev->dev, sizeof(*hlwd), GFP_KERNEL);
+	if (!hlwd)
+		return -ENOMEM;
+
+	regs_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	hlwd->regs = devm_ioremap_resource(&pdev->dev, regs_resource);
+	if (IS_ERR(hlwd->regs))
+		return PTR_ERR(hlwd->regs);
+
+	/*
+	 * Claim all GPIOs using the OWNER register. This will not work on
+	 * systems where the AHBPROT memory firewall hasn't been configured to
+	 * permit PPC access to HW_GPIO_*.
+	 *
+	 * Note that this has to happen before bgpio_init reads the
+	 * HW_GPIOB_OUT and HW_GPIOB_DIR, because otherwise it reads the wrong
+	 * values.
+	 */
+	iowrite32be(0xffffffff, hlwd->regs + HW_GPIO_OWNER);
+
+	res = bgpio_init(&hlwd->gpioc, &pdev->dev, 4,
+			hlwd->regs + HW_GPIOB_IN, hlwd->regs + HW_GPIOB_OUT,
+			NULL, hlwd->regs + HW_GPIOB_DIR, NULL,
+			BGPIOF_BIG_ENDIAN_BYTE_ORDER);
+	if (res < 0) {
+		dev_warn(&pdev->dev, "bgpio_init failed: %d\n", res);
+		return res;
+	}
+
+	res = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios);
+	if (res)
+		ngpios = 32;
+	hlwd->gpioc.ngpio = ngpios;
+
+	return devm_gpiochip_add_data(&pdev->dev, &hlwd->gpioc, hlwd);
+}
+
+static const struct of_device_id hlwd_gpio_match[] = {
+	{ .compatible = "nintendo,hollywood-gpio", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hlwd_gpio_match);
+
+static struct platform_driver hlwd_gpio_driver = {
+	.driver	= {
+		.name		= "gpio-hlwd",
+		.of_match_table	= hlwd_gpio_match,
+	},
+	.probe	= hlwd_gpio_probe,
+};
+module_platform_driver(hlwd_gpio_driver);
+
+MODULE_AUTHOR("Jonathan Neuschäfer <j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>");
+MODULE_DESCRIPTION("Nintendo Wii GPIO driver");
+MODULE_LICENSE("GPL");
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Segher Boessenkool]

On Fri, Feb 09, 2018 at 01:07:29PM +0100, Jonathan Neuschäfer wrote:
> The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> that supports a configurable number of pins (up to 32), interrupts, and
> some special mechanisms to share the controller between the system's
> security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> not supported.
> 
> This patch adds a basic driver for this GPIO controller. Interrupt
> support will come in a later patch.
> 
> This patch is based on code developed by Albert Herranz and the GameCube
> Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> has grown quite dissimilar.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>
> Cc: Albert Herranz <albert_herranz-mRCrAkd8dF0@public.gmane.org>
> Cc: Segher Boessenkool <segher-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>

Reviewed-by: Segher Boessenkool <segher-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>

Looks just fine to me :-)


Segher
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Andy Shevchenko]

On Fri, Feb 9, 2018 at 2:07 PM, Jonathan Neuschäfer
<j.neuschaefer@gmx.net> wrote:
> The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> that supports a configurable number of pins (up to 32), interrupts, and
> some special mechanisms to share the controller between the system's
> security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> not supported.
>
> This patch adds a basic driver for this GPIO controller. Interrupt
> support will come in a later patch.
>
> This patch is based on code developed by Albert Herranz and the GameCube
> Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> has grown quite dissimilar.
>

Fine to me, though one comment below.
In any case,

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> Cc: Albert Herranz <albert_herranz@yahoo.es>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> <---
>
> v3:
> - Do some style cleanups, as suggest by Andy Shevchenko
>
> v2:
> - Change hlwd_gpio_driver.driver.name to "gpio-hlwd" to match the
>   filename (was "hlwd_gpio")
> - Remove unnecessary include of linux/of_gpio.h, as suggested by Linus
>   Walleij.
> - Add struct device pointer to context struct to make it possible to use
>   dev_info(hlwd->dev, "..."), as suggested by Linus Walleij
> - Use the GPIO_GENERIC library to reduce code size, as suggested by
>   Linus Walleij
> - Use iowrite32be instead of __raw_writel for big-endian MMIO access, as
>   suggested by Linus Walleij
> - Remove commit message paragraph suggesting to diff against the
>   original driver, because it's so different now
> ---
>  drivers/gpio/Kconfig     |   9 ++++
>  drivers/gpio/Makefile    |   1 +
>  drivers/gpio/gpio-hlwd.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 125 insertions(+)
>  create mode 100644 drivers/gpio/gpio-hlwd.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d6a8e851ad13..47606dfe06cc 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -229,6 +229,15 @@ config GPIO_GRGPIO
>           Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
>           VHDL IP core library.
>
> +config GPIO_HLWD
> +       tristate "Nintendo Wii (Hollywood) GPIO"

> +       depends on OF_GPIO

You may get rid of it if...

> +       select GPIO_GENERIC
> +       help
> +         Select this to support the GPIO controller of the Nintendo Wii.
> +
> +         If unsure, say N.
> +
>  config GPIO_ICH
>         tristate "Intel ICH GPIO"
>         depends on PCI && X86
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 4bc24febb889..492f62d0eb59 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_FTGPIO010)  += gpio-ftgpio010.o
>  obj-$(CONFIG_GPIO_GE_FPGA)     += gpio-ge.o
>  obj-$(CONFIG_GPIO_GPIO_MM)     += gpio-gpio-mm.o
>  obj-$(CONFIG_GPIO_GRGPIO)      += gpio-grgpio.o
> +obj-$(CONFIG_GPIO_HLWD)                += gpio-hlwd.o
>  obj-$(CONFIG_HTC_EGPIO)                += gpio-htc-egpio.o
>  obj-$(CONFIG_GPIO_ICH)         += gpio-ich.o
>  obj-$(CONFIG_GPIO_INGENIC)     += gpio-ingenic.o
> diff --git a/drivers/gpio/gpio-hlwd.c b/drivers/gpio/gpio-hlwd.c
> new file mode 100644
> index 000000000000..a63136a68ba3
> --- /dev/null
> +++ b/drivers/gpio/gpio-hlwd.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (C) 2008-2009 The GameCube Linux Team
> +// Copyright (C) 2008,2009 Albert Herranz
> +// Copyright (C) 2017-2018 Jonathan Neuschäfer
> +//
> +// Nintendo Wii (Hollywood) GPIO driver
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>

> +#include <linux/of.h>
> +#include <linux/of_platform.h>

...(and using platform device header I suppose)...

> +#include <linux/slab.h>
> +
> +/*
> + * Register names and offsets courtesy of WiiBrew:
> + * https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
> + *
> + * Note that for most registers, there are two versions:
> + * - HW_GPIOB_* Is always accessible by the Broadway PowerPC core, but does
> + *   always give access to all GPIO lines
> + * - HW_GPIO_* Is only accessible by the Broadway PowerPC code if the memory
> + *   firewall (AHBPROT) in the Hollywood chipset has been configured to allow
> + *   such access.
> + *
> + * The ownership of each GPIO line can be configured in the HW_GPIO_OWNER
> + * register: A one bit configures the line for access via the HW_GPIOB_*
> + * registers, a zero bit indicates access via HW_GPIO_*. This driver uses
> + * HW_GPIOB_*.
> + */
> +#define HW_GPIOB_OUT           0x00
> +#define HW_GPIOB_DIR           0x04
> +#define HW_GPIOB_IN            0x08
> +#define HW_GPIOB_INTLVL                0x0c
> +#define HW_GPIOB_INTFLAG       0x10
> +#define HW_GPIOB_INTMASK       0x14
> +#define HW_GPIOB_INMIR         0x18
> +#define HW_GPIO_ENABLE         0x1c
> +#define HW_GPIO_OUT            0x20
> +#define HW_GPIO_DIR            0x24
> +#define HW_GPIO_IN             0x28
> +#define HW_GPIO_INTLVL         0x2c
> +#define HW_GPIO_INTFLAG                0x30
> +#define HW_GPIO_INTMASK                0x34
> +#define HW_GPIO_INMIR          0x38
> +#define HW_GPIO_OWNER          0x3c
> +
> +struct hlwd_gpio {
> +       struct gpio_chip gpioc;
> +       void __iomem *regs;
> +};
> +
> +static int hlwd_gpio_probe(struct platform_device *pdev)
> +{
> +       struct hlwd_gpio *hlwd;
> +       struct resource *regs_resource;
> +       u32 ngpios;
> +       int res;
> +
> +       hlwd = devm_kzalloc(&pdev->dev, sizeof(*hlwd), GFP_KERNEL);
> +       if (!hlwd)
> +               return -ENOMEM;
> +
> +       regs_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       hlwd->regs = devm_ioremap_resource(&pdev->dev, regs_resource);
> +       if (IS_ERR(hlwd->regs))
> +               return PTR_ERR(hlwd->regs);
> +
> +       /*
> +        * Claim all GPIOs using the OWNER register. This will not work on
> +        * systems where the AHBPROT memory firewall hasn't been configured to
> +        * permit PPC access to HW_GPIO_*.
> +        *
> +        * Note that this has to happen before bgpio_init reads the
> +        * HW_GPIOB_OUT and HW_GPIOB_DIR, because otherwise it reads the wrong
> +        * values.
> +        */
> +       iowrite32be(0xffffffff, hlwd->regs + HW_GPIO_OWNER);
> +
> +       res = bgpio_init(&hlwd->gpioc, &pdev->dev, 4,
> +                       hlwd->regs + HW_GPIOB_IN, hlwd->regs + HW_GPIOB_OUT,
> +                       NULL, hlwd->regs + HW_GPIOB_DIR, NULL,
> +                       BGPIOF_BIG_ENDIAN_BYTE_ORDER);
> +       if (res < 0) {
> +               dev_warn(&pdev->dev, "bgpio_init failed: %d\n", res);
> +               return res;
> +       }
> +

> +       res = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios);

...if you switch to unified device property API.

> +       if (res)
> +               ngpios = 32;
> +       hlwd->gpioc.ngpio = ngpios;
> +
> +       return devm_gpiochip_add_data(&pdev->dev, &hlwd->gpioc, hlwd);
> +}
> +
> +static const struct of_device_id hlwd_gpio_match[] = {
> +       { .compatible = "nintendo,hollywood-gpio", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, hlwd_gpio_match);
> +
> +static struct platform_driver hlwd_gpio_driver = {
> +       .driver = {
> +               .name           = "gpio-hlwd",
> +               .of_match_table = hlwd_gpio_match,
> +       },
> +       .probe  = hlwd_gpio_probe,
> +};
> +module_platform_driver(hlwd_gpio_driver);
> +
> +MODULE_AUTHOR("Jonathan Neuschäfer <j.neuschaefer@gmx.net>");
> +MODULE_DESCRIPTION("Nintendo Wii GPIO driver");
> +MODULE_LICENSE("GPL");
> --
> 2.15.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

On Fri, Feb 09, 2018 at 05:30:55PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 9, 2018 at 2:07 PM, Jonathan Neuschäfer
> <j.neuschaefer@gmx.net> wrote:
> > The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> > that supports a configurable number of pins (up to 32), interrupts, and
> > some special mechanisms to share the controller between the system's
> > security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> > not supported.
> >
> > This patch adds a basic driver for this GPIO controller. Interrupt
> > support will come in a later patch.
> >
> > This patch is based on code developed by Albert Herranz and the GameCube
> > Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> > available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> > has grown quite dissimilar.
> >
> 
> Fine to me, though one comment below.
> In any case,
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you.


[...]
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index d6a8e851ad13..47606dfe06cc 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -229,6 +229,15 @@ config GPIO_GRGPIO
> >           Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
> >           VHDL IP core library.
> >
> > +config GPIO_HLWD
> > +       tristate "Nintendo Wii (Hollywood) GPIO"
> 
> > +       depends on OF_GPIO
> 
> You may get rid of it if...

[ Even if this driver isn't switched to the unified device property API,
  I think "depends on OF" would be enough here, because it doesn't use
  the code that's guarded by CONFIG_OF_GPIO (gpiolib-of.c), but this
  applies to other drivers (e.g. gpio-aspeed, gpio-bcm-kona) as well, so
  this would ideally be a bigger cleanup patch. ]


> > +       res = of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios);
> 
> ...if you switch to unified device property API.

I don't think this change is worth making, unless/until the of_property
API is deprecated. I'm rather sure this GPIO controller won't appear in
an ACPI-based system.


Thanks,
Jonathan Neuschäfer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On Fri, Feb 09, 2018 at 01:07:29PM +0100, Jonathan Neuschäfer wrote:
> The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> that supports a configurable number of pins (up to 32), interrupts, and
> some special mechanisms to share the controller between the system's
> security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> not supported.
> 
> This patch adds a basic driver for this GPIO controller. Interrupt
> support will come in a later patch.
> 
> This patch is based on code developed by Albert Herranz and the GameCube
> Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> has grown quite dissimilar.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> Cc: Albert Herranz <albert_herranz@yahoo.es>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> <---

Ooops, I just noticed that I broke the separator here. This should be a
normal --- line, obviously.


Jonathan Neuschäfer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Linus Walleij]

On Fri, Feb 9, 2018 at 1:07 PM, Jonathan Neuschäfer
<j.neuschaefer@gmx.net> wrote:

> The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> that supports a configurable number of pins (up to 32), interrupts, and
> some special mechanisms to share the controller between the system's
> security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> not supported.
>
> This patch adds a basic driver for this GPIO controller. Interrupt
> support will come in a later patch.
>
> This patch is based on code developed by Albert Herranz and the GameCube
> Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> has grown quite dissimilar.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> Cc: Albert Herranz <albert_herranz@yahoo.es>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> <---
>
> v3:
> - Do some style cleanups, as suggest by Andy Shevchenko

Patch applied to the GPIO tree for v4.17 with all the review tags.

I just folded the changelog into the commit message, for new
drivers it is sometimes useful to keep these around in
git actually.

If any further changes are needed we can just patch on top
of this.

It's a very pretty driver, good work!

Yours,
Linus Walleij

Re: [PATCH v3 2/4] gpio: Add GPIO driver for Nintendo Wii

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]

On Thu, Feb 22, 2018 at 01:57:07PM +0100, Linus Walleij wrote:
> On Fri, Feb 9, 2018 at 1:07 PM, Jonathan Neuschäfer
> <j.neuschaefer@gmx.net> wrote:
> 
> > The Nintendo Wii's chipset (called "Hollywood") has a GPIO controller
> > that supports a configurable number of pins (up to 32), interrupts, and
> > some special mechanisms to share the controller between the system's
> > security processor (an ARM926) and the PowerPC CPU. Pin multiplexing is
> > not supported.
> >
> > This patch adds a basic driver for this GPIO controller. Interrupt
> > support will come in a later patch.
> >
> > This patch is based on code developed by Albert Herranz and the GameCube
> > Linux Team, file arch/powerpc/platforms/embedded6xx/hlwd-gpio.c,
> > available at https://github.com/DeltaResero/GC-Wii-Linux-Kernels, but
> > has grown quite dissimilar.
> >
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > Cc: Albert Herranz <albert_herranz@yahoo.es>
> > Cc: Segher Boessenkool <segher@kernel.crashing.org>
> > <---
> >
> > v3:
> > - Do some style cleanups, as suggest by Andy Shevchenko
> 
> Patch applied to the GPIO tree for v4.17 with all the review tags.
> 
> I just folded the changelog into the commit message, for new
> drivers it is sometimes useful to keep these around in
> git actually.
> 
> If any further changes are needed we can just patch on top
> of this.
> 
> It's a very pretty driver, good work!

Thanks! :-)


Jonathan Neuschäfer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 3/4] powerpc: wii.dts: Add ngpios property

[Jonathan Neuschäfer]

The Hollywood GPIO controller supports 32 GPIOs, but on the Wii, only 24
are used.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---

v2, v3:
- no change
---
 arch/powerpc/boot/dts/wii.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 40b324b6391e..7235e375919c 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -176,6 +176,7 @@
 			compatible = "nintendo,hollywood-gpio";
 			reg = <0x0d8000c0 0x40>;
 			gpio-controller;
+			ngpios = <24>;
 
 			/*
 			 * This is commented out while a standard binding
-- 
2.15.1

[PATCH v3 4/4] powerpc: wii.dts: Add GPIO line names

[Jonathan Neuschäfer]

These are the GPIO line names on a Nintendo Wii, as documented in:
https://wiibrew.org/wiki/Hardware/Hollywood_GPIOs

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---

v2, v3:
- no change
---
 arch/powerpc/boot/dts/wii.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index 7235e375919c..07d5e84e98b1 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -178,6 +178,14 @@
 			gpio-controller;
 			ngpios = <24>;
 
+			gpio-line-names =
+				"POWER", "SHUTDOWN", "FAN", "DC_DC",
+				"DI_SPIN", "SLOT_LED", "EJECT_BTN", "SLOT_IN",
+				"SENSOR_BAR", "DO_EJECT", "EEP_CS", "EEP_CLK",
+				"EEP_MOSI", "EEP_MISO", "AVE_SCL", "AVE_SDA",
+				"DEBUG0", "DEBUG1", "DEBUG2", "DEBUG3",
+				"DEBUG4", "DEBUG5", "DEBUG6", "DEBUG7";
+
 			/*
 			 * This is commented out while a standard binding
 			 * for i2c over gpio is defined.
-- 
2.15.1