From: Linus Walleij <linus.walleij@linaro.org> To: Vincent Yang <vincent.yang.fujitsu@gmail.com>, Kamlakant Patel <kamlakant.patel@linaro.org> Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "Arnd Bergmann" <arnd@arndb.de>, "Olof Johansson" <olof@lixom.net>, "Rob Herring" <robh+dt@kernel.org>, "Paweł Moll" <pawel.moll@arm.com>, "Mark Rutland" <mark.rutland@arm.com>, "ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>, "Kumar Gala" <galak@codeaurora.org>, "Grant Likely" <grant.likely@linaro.org>, "Alexandre Courbot" <gnurou@gmail.com>, "Andy Green" <andy.green@linaro.org>, "Patch Tracking" <patches@linaro.org>, "Jaswinder Singh" <jaswinder.singh@linaro.org>, "Vincent Yang" <Vincent.Yang@tw.fujitsu.com>, "Tetsuya Nuriya" <nuriya.tetsuya@jp.fujitsu.com> Subject: Re: [PATCH v3 5/8] gpio: Add Fujitsu MB86S7x GPIO driver Date: Sun, 11 Jan 2015 23:40:46 +0100 [thread overview] Message-ID: <CACRpkdZk3sLN43zmuWM+VYLB7vJdNCSq_goTx6mDSnzHrO=UHw@mail.gmail.com> (raw) In-Reply-To: <1420803212-4350-1-git-send-email-Vincent.Yang@tw.fujitsu.com> On Fri, Jan 9, 2015 at 12:33 PM, Vincent Yang <vincent.yang.fujitsu@gmail.com> wrote: > From: Jassi Brar <jaswinder.singh@linaro.org> > > Driver for Fujitsu MB86S7x SoCs that have a memory mapped GPIO controller. > > Signed-off-by: Andy Green <andy.green@linaro.org> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> > Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com> > Signed-off-by: Tetsuya Nuriya <nuriya.tetsuya@jp.fujitsu.com> (....) > +++ b/Documentation/devicetree/bindings/gpio/fujitsu,mb86s70-gpio.txt Binding looks good to me. > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 633ec21..699e629 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -197,6 +197,12 @@ config GPIO_F7188X > To compile this driver as a module, choose M here: the module will > be called f7188x-gpio. > > +config GPIO_MB86S7X > + bool "GPIO support for Fujitsu MB86S7x Platforms" > + depends on ARCH_MB86S7X || COMPILE_TEST? So we can test it on a x86_64 compile? > diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c > new file mode 100644 > index 0000000..c912585 > --- /dev/null > +++ b/drivers/gpio/gpio-mb86s7x.c > @@ -0,0 +1,231 @@ > +/* > + * linux/drivers/gpio/gpio-mb86s7x.c > + * > + * Copyright (C) 2015 Fujitsu Semiconductor Limited > + * Copyright (C) 2015 Linaro Ltd. > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/io.h> > +#include <linux/init.h> > +#include <linux/clk.h> > +#include <linux/gpio.h> Nominally a modern driver should just #include <linux/gpio/driver.h> instead of <linux/gpio.h> > +#include <linux/module.h> > +#include <linux/err.h> > +#include <linux/errno.h> > +#include <linux/ioport.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/spinlock.h> > +#include <linux/slab.h> > + > +/* > + * Only first 8bits of a register correspond to each pin, > + * so there are 4 registers for 32 pins. > + */ > +#define PDR(x) (0x0 + x / 8 * 4) > +#define DDR(x) (0x10 + x / 8 * 4) > +#define PFR(x) (0x20 + x / 8 * 4) Hm pins ... is this actually a pin controller hardware? Usually I encourage the concept of "line" over "pin" to distinguish GPIO lines from pin controller pins. > +static int mb86s70_gpio_direction_output(struct gpio_chip *gc, > + unsigned gpio, int value) > +{ > + struct mb86s70_gpio_chip *gchip = chip_to_mb86s70(gc); > + unsigned long flags; > + unsigned char val; > + > + spin_lock_irqsave(&gchip->lock, flags); > + > + val = readl(gchip->base + PDR(gpio)); > + if (value) > + val |= OFFSET(gpio); > + else > + val &= ~OFFSET(gpio); > + writel(val, gchip->base + PDR(gpio)); > + > + val = readl(gchip->base + DDR(gpio)); > + val |= OFFSET(gpio); > + writel(val, gchip->base + DDR(gpio)); > + > + spin_unlock_irqrestore(&gchip->lock, flags); > + > + return 0; > +} First I thought maybe this could use the generic MMIO driver but it seems you need this double register access for setting the direction so it won't work. Otherwise it's sort of close ... have you looks at the option? Kamlakant did a lot of interesting attempts to migrate some current drivers to GPIO MMIO (generic GPIO) recently, e.g. this: http://marc.info/?l=linux-gpio&m=141743574511640&w=2 or this: http://marc.info/?l=linux-gpio&m=141743574211639&w=2 > +static int __init mb86s70_gpio_init(void) > +{ > + return platform_driver_register(&mb86s70_gpio_driver); > +} > +subsys_initcall(mb86s70_gpio_init); We are trying to move away from sybsys_initcall() and rely on deferred probe. Why do you need this here? Yours, Linus Walleij
WARNING: multiple messages have this Message-ID (diff)
From: linus.walleij@linaro.org (Linus Walleij) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v3 5/8] gpio: Add Fujitsu MB86S7x GPIO driver Date: Sun, 11 Jan 2015 23:40:46 +0100 [thread overview] Message-ID: <CACRpkdZk3sLN43zmuWM+VYLB7vJdNCSq_goTx6mDSnzHrO=UHw@mail.gmail.com> (raw) In-Reply-To: <1420803212-4350-1-git-send-email-Vincent.Yang@tw.fujitsu.com> On Fri, Jan 9, 2015 at 12:33 PM, Vincent Yang <vincent.yang.fujitsu@gmail.com> wrote: > From: Jassi Brar <jaswinder.singh@linaro.org> > > Driver for Fujitsu MB86S7x SoCs that have a memory mapped GPIO controller. > > Signed-off-by: Andy Green <andy.green@linaro.org> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> > Signed-off-by: Vincent Yang <Vincent.Yang@tw.fujitsu.com> > Signed-off-by: Tetsuya Nuriya <nuriya.tetsuya@jp.fujitsu.com> (....) > +++ b/Documentation/devicetree/bindings/gpio/fujitsu,mb86s70-gpio.txt Binding looks good to me. > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 633ec21..699e629 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -197,6 +197,12 @@ config GPIO_F7188X > To compile this driver as a module, choose M here: the module will > be called f7188x-gpio. > > +config GPIO_MB86S7X > + bool "GPIO support for Fujitsu MB86S7x Platforms" > + depends on ARCH_MB86S7X || COMPILE_TEST? So we can test it on a x86_64 compile? > diff --git a/drivers/gpio/gpio-mb86s7x.c b/drivers/gpio/gpio-mb86s7x.c > new file mode 100644 > index 0000000..c912585 > --- /dev/null > +++ b/drivers/gpio/gpio-mb86s7x.c > @@ -0,0 +1,231 @@ > +/* > + * linux/drivers/gpio/gpio-mb86s7x.c > + * > + * Copyright (C) 2015 Fujitsu Semiconductor Limited > + * Copyright (C) 2015 Linaro Ltd. > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/io.h> > +#include <linux/init.h> > +#include <linux/clk.h> > +#include <linux/gpio.h> Nominally a modern driver should just #include <linux/gpio/driver.h> instead of <linux/gpio.h> > +#include <linux/module.h> > +#include <linux/err.h> > +#include <linux/errno.h> > +#include <linux/ioport.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/spinlock.h> > +#include <linux/slab.h> > + > +/* > + * Only first 8bits of a register correspond to each pin, > + * so there are 4 registers for 32 pins. > + */ > +#define PDR(x) (0x0 + x / 8 * 4) > +#define DDR(x) (0x10 + x / 8 * 4) > +#define PFR(x) (0x20 + x / 8 * 4) Hm pins ... is this actually a pin controller hardware? Usually I encourage the concept of "line" over "pin" to distinguish GPIO lines from pin controller pins. > +static int mb86s70_gpio_direction_output(struct gpio_chip *gc, > + unsigned gpio, int value) > +{ > + struct mb86s70_gpio_chip *gchip = chip_to_mb86s70(gc); > + unsigned long flags; > + unsigned char val; > + > + spin_lock_irqsave(&gchip->lock, flags); > + > + val = readl(gchip->base + PDR(gpio)); > + if (value) > + val |= OFFSET(gpio); > + else > + val &= ~OFFSET(gpio); > + writel(val, gchip->base + PDR(gpio)); > + > + val = readl(gchip->base + DDR(gpio)); > + val |= OFFSET(gpio); > + writel(val, gchip->base + DDR(gpio)); > + > + spin_unlock_irqrestore(&gchip->lock, flags); > + > + return 0; > +} First I thought maybe this could use the generic MMIO driver but it seems you need this double register access for setting the direction so it won't work. Otherwise it's sort of close ... have you looks at the option? Kamlakant did a lot of interesting attempts to migrate some current drivers to GPIO MMIO (generic GPIO) recently, e.g. this: http://marc.info/?l=linux-gpio&m=141743574511640&w=2 or this: http://marc.info/?l=linux-gpio&m=141743574211639&w=2 > +static int __init mb86s70_gpio_init(void) > +{ > + return platform_driver_register(&mb86s70_gpio_driver); > +} > +subsys_initcall(mb86s70_gpio_init); We are trying to move away from sybsys_initcall() and rely on deferred probe. Why do you need this here? Yours, Linus Walleij
next prev parent reply other threads:[~2015-01-11 22:40 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-09 11:19 [PATCH v3 0/8] Support for Fujitsu MB86S7X SoCs Vincent Yang 2015-01-09 11:19 ` Vincent Yang [not found] ` <1420802369-3840-1-git-send-email-Vincent.Yang-l16TxrwUIHTQFUHtdCDX3A@public.gmane.org> 2015-01-09 11:24 ` [PATCH v3 1/8] ARM: Add platform support " Vincent Yang 2015-01-09 11:24 ` Vincent Yang 2015-01-09 11:28 ` [PATCH v3 2/8] mailbox: arm_mhu: add driver for ARM MHU controller Vincent Yang 2015-01-09 11:28 ` Vincent Yang [not found] ` <1420802889-4041-1-git-send-email-Vincent.Yang-l16TxrwUIHTQFUHtdCDX3A@public.gmane.org> 2015-01-09 12:51 ` Russell King - ARM Linux 2015-01-09 12:51 ` Russell King - ARM Linux [not found] ` <20150109125102.GL12302-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2015-01-09 13:19 ` Jassi Brar 2015-01-09 13:19 ` Jassi Brar 2015-01-09 15:24 ` Russell King - ARM Linux 2015-01-09 15:24 ` Russell King - ARM Linux [not found] ` <20150109152402.GQ12302-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2015-01-09 15:29 ` Jassi Brar 2015-01-09 15:29 ` Jassi Brar 2015-01-09 11:29 ` [PATCH v3 4/8] clk: Add clock driver for mb86s7x Vincent Yang 2015-01-09 11:29 ` Vincent Yang [not found] ` <1420802977-4126-1-git-send-email-Vincent.Yang-l16TxrwUIHTQFUHtdCDX3A@public.gmane.org> 2015-01-09 12:39 ` Russell King - ARM Linux 2015-01-09 12:39 ` Russell King - ARM Linux [not found] ` <20150109123958.GJ12302-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2015-01-09 13:03 ` Jassi Brar 2015-01-09 13:03 ` Jassi Brar 2015-01-09 11:35 ` [PATCH v3 7/8] of: add Fujitsu vendor prefix Vincent Yang 2015-01-09 11:35 ` Vincent Yang 2015-01-09 11:28 ` [PATCH v3 3/8] ARM: MB86S7X: Add MCPM support Vincent Yang 2015-01-09 12:41 ` Russell King - ARM Linux 2015-01-09 13:23 ` Jassi Brar 2015-01-09 20:04 ` Nicolas Pitre 2015-01-09 11:33 ` [PATCH v3 5/8] gpio: Add Fujitsu MB86S7x GPIO driver Vincent Yang 2015-01-09 11:33 ` Vincent Yang [not found] ` <1420803212-4350-1-git-send-email-Vincent.Yang-l16TxrwUIHTQFUHtdCDX3A@public.gmane.org> 2015-01-09 12:52 ` Russell King - ARM Linux 2015-01-09 12:52 ` Russell King - ARM Linux 2015-01-09 13:20 ` Jassi Brar 2015-01-09 13:20 ` Jassi Brar 2015-01-11 22:40 ` Linus Walleij [this message] 2015-01-11 22:40 ` Linus Walleij 2015-01-12 0:04 ` Linus Walleij 2015-01-12 0:04 ` Linus Walleij 2015-01-09 11:34 ` [PATCH v3 6/8] dt: mb86s7x: add dt files for MB86S7x evbs Vincent Yang 2015-01-09 11:34 ` Vincent Yang 2015-01-09 11:36 ` [PATCH v3 8/8] ARM: MB86S7x: Add configs Vincent Yang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CACRpkdZk3sLN43zmuWM+VYLB7vJdNCSq_goTx6mDSnzHrO=UHw@mail.gmail.com' \ --to=linus.walleij@linaro.org \ --cc=Vincent.Yang@tw.fujitsu.com \ --cc=andy.green@linaro.org \ --cc=arnd@arndb.de \ --cc=devicetree@vger.kernel.org \ --cc=galak@codeaurora.org \ --cc=gnurou@gmail.com \ --cc=grant.likely@linaro.org \ --cc=ijc+devicetree@hellion.org.uk \ --cc=jaswinder.singh@linaro.org \ --cc=kamlakant.patel@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-gpio@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=nuriya.tetsuya@jp.fujitsu.com \ --cc=olof@lixom.net \ --cc=patches@linaro.org \ --cc=pawel.moll@arm.com \ --cc=robh+dt@kernel.org \ --cc=vincent.yang.fujitsu@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.