Re: [PATCH] pinctrl: sh-pfc: fix warnings by include core.h
On Tue, Jun 7, 2016 at 7:33 PM, Ben Dooks <ben.do...@codethink.co.uk> wrote: > Fix warnings about emev2_pinmux_info and r8a7779_pinmux_info > by using core.h instead of sh_pfc.h in these files. This gives > the declarations of the two structures and removes the following > warnings: > > drivers/pinctrl/sh-pfc/pfc-emev2.c:1695:30: warning: symbol > 'emev2_pinmux_info' was not declared. Should it be static? > drivers/pinctrl/sh-pfc/pfc-r8a7779.c:3888:30: warning: symbol > 'r8a7779_pinmux_info' was not declared. Should it be static? > > Signed-off-by: Ben Dooks <ben.do...@codethink.co.uk> Geert/Laurent: - take a look at this patch - shall I apply this directly? Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: fix property name in bindings doc
On Fri, Jun 10, 2016 at 5:11 PM, Wolfram Sang <w...@the-dreams.de> wrote: > From: Wolfram Sang <wsa+rene...@sang-engineering.com> > > It must be "drive-strength", with a hyphen. > > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> Geert are you queuing this too? Yours, Linus Walleij
Re: [PATCH 0/3] pinctrl: sh-pfc: Cleanups
On Fri, Jun 10, 2016 at 12:05 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > This series contains various cleanups for the Renesas Pin Function > Controller driver subsystem. Acked-by: Linus Walleij <linus.wall...@linaro.org> I guess I will get them by pull request from you. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7795: use PINMUX_SINGLE() for I2C
On Tue, Jun 21, 2016 at 9:21 AM, Kuninori Morimoto <kuninori.morimoto...@renesas.com> wrote: > From: Kuninori Morimoto <kuninori.morimoto...@renesas.com> > > Now we have PINMUX_SINGLE(). Let's use it instead of PINMUX_IPSR_NOGP() > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto...@renesas.com> > Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be> Acked-by: Linus Walleij <linus.wall...@linaro.org> Geert, are you queuing this? Yours, Linus Walleij
Re: [Bug]:LAGER: GPIO-KEYS: Warning occurs after unbinding the e6051000.gpio
On Tue, Jun 28, 2016 at 3:41 AM, Hiep Cao Minh <cm-h...@jinso.co.jp> wrote: > As I have reported you the issue that related to your released patch. > Could you please consider this issue and tell me how to resolve this > problem? > > This issue still happens on Rcar-H2 and Rcar-M2 (Renesas's SOC) at the Linux > upstream version v4.7-rc2. I think the patches that Torvalds merged for v4.7-rc3 http://lwn.net/Articles/690987/ especially these two: Ricardo Ribalda Delgado (2): gpiolib: Fix NULL pointer deference gpiolib: Fix unaligned used of reference counters Solves the issue. Can you verify? Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.8
On Thu, Jun 23, 2016 at 3:39 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 1a695a905c18548062509178b98bc91e67510864: > > Linux 4.7-rc1 (2016-05-29 09:29:24 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.8-tag1 > > for you to fetch changes up to 2d7758319889bf9def19e0a7a5daf1f87c9a9116: > > pinctrl: sh-pfc: r8a7795: Add DRIF support (2016-06-23 11:01:21 +0200) > > > pinctrl: sh-pfc: Updates for v4.8 Pulled into the pin control devel branch for v4.8. Thanks for your efforts to keep the PFC development together! Yours, Linus Walleij
Re: [PATCH/RFC v2] gpio: rcar: Add Runtime PM handling for interrupts
On Thu, Feb 18, 2016 at 5:06 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > The R-Car GPIO driver handles Runtime PM for requested GPIOs only. > > When using a GPIO purely as an interrupt source, no Runtime PM handling > is done, and the GPIO module's clock may not be enabled. > > To fix this: > - Add .irq_request_resources() and .irq_release_resources() callbacks > to handle Runtime PM when an interrupt is requested, > - Add irq_bus_lock() and sync_unlock() callbacks to handle Runtime PM > when e.g. disabling/enabling an interrupt, or configuring the > interrupt type. > > Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be> Patch applied with Marc's ACK. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: Use ARCH_RENESAS
On Thu, Feb 25, 2016 at 1:51 AM, Simon Horman <horms+rene...@verge.net.au> wrote: > Make use of ARCH_RENESAS in place of ARCH_SHMOBILE. > > This is part of an ongoing process to migrate from ARCH_SHMOBILE to > ARCH_RENESAS the motivation for which being that RENESAS seems to be a more > appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs. > > Signed-off-by: Simon Horman <horms+rene...@verge.net.au> Acked-by: Linus Walleij <linus.wall...@linaro.org> Expecting Geert to queue this for me. Yours, Linus Walleij
Re: [PATCH] gpio: rcar: Use ARCH_RENESAS
On Tue, Feb 23, 2016 at 2:36 AM, Simon Horman <horms+rene...@verge.net.au> wrote: > Make use of ARCH_RENESAS in place of ARCH_SHMOBILE. > > This is part of an ongoing process to migrate from ARCH_SHMOBILE to > ARCH_RENESAS the motivation for which being that RENESAS seems to be a more > appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs. > > Signed-off-by: Simon Horman <horms+rene...@verge.net.au> OK sounds like a good cause, patch applied. Yours, Linus Walleij
Re: [PATCH/RFC] gpio: rcar: Add Runtime PM handling for interrupts
r_priv *p,
> gpio_rcar_write(p, INTCLR, BIT(hwirq));
>
> spin_unlock_irqrestore(>lock, flags);
> +
> + pm_runtime_put(>pdev->dev);
> }
>
> static int gpio_rcar_irq_set_type(struct irq_data *d, unsigned int type)
> @@ -196,6 +215,27 @@ static int gpio_rcar_irq_set_wake(struct irq_data *d,
> unsigned int on)
> return 0;
> }
>
> +static int gpio_rcar_irq_request_resources(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct gpio_rcar_priv *p = gpiochip_get_data(gc);
> + int error;
> +
> + error = pm_runtime_get_sync(>pdev->dev);
> + if (error < 0)
> + return error;
> +
> + return 0;
> +}
> +
> +static void gpio_rcar_irq_release_resources(struct irq_data *d)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct gpio_rcar_priv *p = gpiochip_get_data(gc);
> +
> + pm_runtime_put(>pdev->dev);
> +}
> +
> static irqreturn_t gpio_rcar_irq_handler(int irq, void *dev_id)
> {
> struct gpio_rcar_priv *p = dev_id;
> @@ -450,6 +490,8 @@ static int gpio_rcar_probe(struct platform_device *pdev)
> irq_chip->irq_unmask = gpio_rcar_irq_enable;
> irq_chip->irq_set_type = gpio_rcar_irq_set_type;
> irq_chip->irq_set_wake = gpio_rcar_irq_set_wake;
> + irq_chip->irq_request_resources = gpio_rcar_irq_request_resources;
> + irq_chip->irq_release_resources = gpio_rcar_irq_release_resources;
> irq_chip->flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_MASK_ON_SUSPEND;
>
> ret = gpiochip_add_data(gpio_chip, p);
> --
> 1.9.1
>
Yours,
Linus Walleij
Re: [PATCH 1/2] pinctrl: sh-pfc: r8a7794: add SSI pin groups
On Wed, Feb 10, 2016 at 11:38 PM, Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> wrote: > From: Ryo Kataoka <ryo.kataoka...@renesas.com> > > Add the SSI pin groups to the R8A7794 PFC driver. > > [Sergei: fixed inconsistent alternate pin group naming, split SSI5/6 pin > groups into data/control ones, moved SSI7 data B group to its proper place, > fixed pin names in the comments to *_pins[], extended Cogent Embedded's > copyright, added the changelog, renamed the patch.] > > Signed-off-by: Ryo Kataoka <ryo.kataoka...@renesas.com> > Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> These look fine to me, Acked-by for both. Geert, will you & Laurent review and queue this please. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7794: add EtherAVB pin groups
On Thu, Feb 18, 2016 at 9:36 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Wed, Feb 17, 2016 at 11:32 PM, Sergei Shtylyov > <sergei.shtyl...@cogentembedded.com> wrote: >> Add the EtherAVB pin groups to the R8A7794 PFC driver. >> >> Based on the patches by Mitsuhiro Kimura <mitsuhiro.kimura...@renesas.com>. > > Thank you for your patch! > >> Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> > > Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be> I guess this means you're queueing it too? Yours, Linus Walleij
Re: [PATCH -next] gpio: Use kzalloc() to allocate struct gpio_device to fix crash
On Tue, Feb 16, 2016 at 11:22 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > gpiochip_add_data() allocates the struct gpio_device using kmalloc(), > which doesn't zero the returned memory. > > Hence when calling dev_set_name(), it may try to free a bogus old name, > causing a crash: Ooops got two patches to this independently and applied the other one, I tagged your name onto the Reported-by now. Thanks! Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.6
On Fri, Feb 19, 2016 at 9:54 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 92e963f50fc74041b5e9e744c330dca48e04f08d: > > Linux 4.5-rc1 (2016-01-24 13:06:47 -0800) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > sh-pfc-for-v4.6 > > for you to fetch changes up to 4c96cb027be5ceb2c7c0d4dc086d35fd0cfaf14b: Pulled to my devel branch. Pushing to the build servers soon. Thanks for your excellent comaintenance efforts! Yours, Linus Walleij
Re: [PATCH/RFC] gpio: rcar: Add Runtime PM handling for interrupts
On Tue, Feb 16, 2016 at 10:28 AM, Jon Hunter <jonath...@nvidia.com> wrote: > As I mentioned I do plan to get back to the series for adding runtime-pm > support for IRQ chips, in the next week or two. It sounds like the two of you need to coordinate your work. We're breaking new ground here so keep me, Ulf and tglx in the loop! Yours, Linus Walleij
Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
On Mon, Mar 7, 2016 at 7:40 PM, Wolfram Sang <w...@the-dreams.de> wrote: > From: Wolfram Sang <wsa+rene...@sang-engineering.com> > > If pinctrl_provide_dummies() is used unconditionally, then the dummy > state will be used even on DT platforms when the "init" state was > intentionally left out. Instead of "default", the dummy "init" state > will then be used during probe. Thus, when probing an I2C controller on > cold boot, communication triggered by bus notifiers broke because the > pins were not initialized. > > Do it like OMAP2: use the dummy state only for non-DT platforms. > > Reported-by: Geert Uytterhoeven <geert+rene...@glider.be> > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> Patch applied for fixes with all the ACKs etc. This unfortiunately coincide with the merge window so was slow to pick it up, but it will be in the first round of fixes to Torvalds, possibly at -rc1 possibly earlier. > - pinctrl_provide_dummies(); > + /* Enable dummy states for those platforms without pinctrl support */ > + if (!of_have_populated_dt()) > + pinctrl_provide_dummies(); So remind we: what Renesas platforms are still not using DT? arch/sh? Yours, Linus Walleij
Re: [PATCH 4/8] drivers/pinctrl: make sh-pfc/core.c explicitly non-modular
On Tue, Mar 1, 2016 at 3:48 AM, Paul Gortmaker <paul.gortma...@windriver.com> wrote: > The Kconfig / Makefile currently controlling compilation of this code is: > > drivers/pinctrl/sh-pfc/Makefile:obj-$(CONFIG_PINCTRL_SH_PFC)+= sh-pfc.o > drivers/pinctrl/sh-pfc/Makefile:sh-pfc-objs = core.o > pinctrl.o > > drivers/pinctrl/sh-pfc/Kconfig:config PINCTRL_SH_PFC > drivers/pinctrl/sh-pfc/Kconfig: def_bool y > > ...meaning that it currently is not being built as a module by anyone. > > Lets remove the modular code that is essentially orphaned, so that > when reading the driver there is no doubt it is builtin-only. > > Since module_init already wasn't being used in this code, the init > ordering remains unchanged with this commit. > > Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code. > > We also delete the MODULE_LICENSE tag etc. since all that information > was (or is now) contained at the top of the file in the comments. > > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > Cc: Geert Uytterhoeven <geert+rene...@glider.be> > Cc: Linus Walleij <linus.wall...@linaro.org> > Cc: linux-renesas-soc@vger.kernel.org > Cc: linux-g...@vger.kernel.org > Signed-off-by: Paul Gortmaker <paul.gortma...@windriver.com> Patch applied with Laurent's and Geert's ACKs. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: core: don't open code of_device_get_match_data()
On Tue, Mar 1, 2016 at 11:38 PM, Wolfram Sang <w...@the-dreams.de> wrote: > From: Wolfram Sang <wsa+rene...@sang-engineering.com> > > This change will also make Coverity happy by avoiding a theoretical NULL > pointer dereference; yet another reason is to use the above helper function > to tighten the code and make it more readable. > > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> Patch applied with Geert's ACK. Yours, Linus Walleij
Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
On Wed, Mar 9, 2016 at 4:58 PM, Wolfram Sang <w...@the-dreams.de> wrote: > On Mon, Mar 07, 2016 at 10:00:37PM +0100, Geert Uytterhoeven wrote: >> >This approach is better then, won't have to fix again whenever SH gets >> > DT >> > support. >> >> Perhaps the of_have_populated_dt() check should be moved inside >> pinctrl_provide_dummies()? > > I suggest we pick up this patch now even with stable tag (the irq storm > fixup doesn't work correctly because of this) and consider the > refactoring as a second step? Sergei, Geert, do you ACK this? Yours, Linus Walleij
Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
On Tue, Mar 8, 2016 at 5:21 AM, Wolfram Sang <w...@the-dreams.de> wrote: > >> Perhaps the of_have_populated_dt() check should be moved inside >> pinctrl_provide_dummies()? > > Hmm, the kernel-doc for pinctrl_provide_dummies() says "Usually this > function is called by platforms without pinctrl driver support...". > Is "without pintctrl" == "no DT"? Linus? The primary example is a platform with one pin controller and two identical GPIO controllers, one of the GPIO controllers is using a pin control backend. Since the GPIO driver has to call pinctrl_request_gpio() etc, it needs something to hold on to if there is no backing pin controller for one of them. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: core: only call pinctrl_provide_dummies() on SuperH
On Sat, Mar 5, 2016 at 5:58 AM, Sergei Shtylyov
<sergei.shtyl...@cogentembedded.com> wrote:
> The 'ravb' Ethernet driver couldn't connect to the PHY as the MDIO bus
> appeared empty on the Renesas R-Car boards. The bug hunt finally pointed
> at the commit adding the "init" pintcrl state: it tries to switch to non-
> default state before the driver probe which should fail but doesn't as the
> PFC pinctrl driver happens to call pinctrl_provide_dummies() which makes
> all state lookups succeed, even though the state doesn't really exist.
> That feature is only relevant to non-DT systems and all the ARM boards
> that use the PFC driver have been converted to the DT boot, so limiting
> it to the SuperH architecture seems The Right Thing...
>
> Fixes: ef0eebc05130 ("drivers/pinctrl: Add the concept of an "init" state")
> Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com>
> CC: sta...@vger.kernel.org
Putting this on hold because another patch is being discussed as a
more proper fix.
Yours,
Linus Walleij
Re: [PATCH v2 0/2] r8a7795 pinctrl: Add drive strength support
On Thu, Mar 31, 2016 at 10:41 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Thu, Mar 31, 2016 at 10:34 AM, Linus Walleij >> Geert, are you queuing a pull request to me or do you think >> Renesas activity is down to a level where I can return to applying >> PFC patches directly to my tree? > > Already queued up in sh-pfc-for-v4.7. Thanks! > It depends on how many pull requests you want to see during a release > cycle ;-) > Do you already want a pull request now? Nah that's up to you, when you feel that you have a bunch of stuff that needs some rotation in linux-next feel free to send a pull request for that. Yours, Linus Walleij
Re: [PATCH] MAINTAINERS: gpio: add DT bindings directory
On Tue, Apr 12, 2016 at 5:57 PM, Wolfram Sang <w...@the-dreams.de> wrote: > From: Wolfram Sang <wsa+rene...@sang-engineering.com> > > Helps get_maintainer.pl to find the right people. > > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> OK patch applied. Maybe we should move the ACPI specs into the kernel as well as that stuff is taking my time as well now, apart from DT :/ Yours, Linus Walleij
Re: [RFD] Sharing GPIOs for input (buttons) and output (LEDs)
On Mon, Feb 22, 2016 at 1:14 PM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> On the Renesas Salvator-X development board, 3 GPIO pins are connected to both
> push buttons and LEDs.
> - If the GPIO is configured for output, it can control the LED,
> - If the GPIO is configured for input, the push button status can be
> read. Note that the LED is on if the push button is not pressed; it is
> turned off while holding the button.
Have you asked the hardware engineer who did this construction
what s/he was thinking? And I mean seriously: what was the
usecase? Did they really design the LEDs to be used to flicker
with or just as an indication as to whether the button was being
pushed or not?
Your approach seems dedicated to use it for both usecases (also
as a stand-alone heartbeat or whatever) but was that really
intended?
> keyboard {
> compatible = "gpio-keys";
>
> key-a {
> gpios = < 11 GPIO_ACTIVE_LOW>;
> label = "SW20";
> wakeup-source;
> linux,code = ;
> };
> key-b {
> gpios = < 12 GPIO_ACTIVE_LOW>;
> label = "SW21";
> wakeup-source;
> linux,code = ;
> };
> key-c {
> gpios = < 13 GPIO_ACTIVE_LOW>;
> label = "SW22";
> wakeup-source;
> linux,code = ;
> };
> };
I suspect that in this usecase, the GPIO should be flagged
GPIO_OPEN_DRAIN rather than GPIO_ACTIVE_LOW,
as the construction with the LED draws current fron the line.
> There exist device tree bindings for LEDs connected to GPIOs, and the
> following also works:
>
> leds {
> compatible = "gpio-leds";
>
> led4 {
> gpios = < 11 GPIO_ACTIVE_HIGH>;
> label = "LED4";
> };
> led5 {
> gpios = < 12 GPIO_ACTIVE_HIGH>;
> label = "LED5";
> };
> led6 {
> gpios = < 13 GPIO_ACTIVE_HIGH>;
> label = "LED6";
> };
> };
OK
> If a GPIO is found busy during initialization, and if it's already in
> use by the other half of the driver, the driver switches to a special
> "polled-key-and-LED" mode. I.e. during polling, it does:
> - Save the GPIO output state,
> - Switch the GPIO to input mode,
> - Wait 5 ms (else it will read the old output state, depending on
> e.g. hardware capacitance),
> - Read the GPIO input state,
> - Switch the GPIO to output mode,
> - Restore the GPIO output state.
>
> And it works, the LEDs can be controlled, and the push button states can
> be read!
Why go to such troubles of switching the line from output to
input to read it?
Especially when a line is OPEN_DRAIN it should be perfectly legal
to read it's value even if it is set as output, but AFAICT that is
always working no matter whether the line is set as output.
> However, due to the 5 ms delay, there's a visible flickering of LEDs
> that are supposed to be turned off (remember, when the GPIO is
> configured for input and the button is not pressed, the LED is lit).
>
> If we go this route, adding support for non-polled GPIOs (if the GPIO is
> not shared with an LED) and wake-up should be doable.
I think this actually implies OPEN_DRAIN and if you flag it as such
the core should be happy using it as input and output at the same
time.
If the hardware has a problem with reading the value from a line
that is set to output, it needs a workaround hack in the driver to
support reading and output line, *NOT* changes to gpiolib,
because the lib assumes this is always possible, i.e. it will
call the driver .get() callback no matter what.
Yours,
Linus Walleij
Re: [PATCH 2/2] pinctrl: sh-pfc: IPSRx and MOD_SELx should be set before GPSRx
On Wed, Mar 16, 2016 at 1:48 AM, Kuninori Morimoto <kuninori.morimoto...@renesas.com> wrote: > From: Kuninori Morimoto <kuninori.morimoto...@renesas.com> > > Gen2 / Gen3 datasheet will have below note in next version. > This patch follows this note. > > IPSRx and MOD_SELx registers shall be set before setting GPSRx > registers in case that they need to be configured. > MOD_SELx registers can be set either earlier or later than setting > IPSRx registers. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto...@renesas.com> I wait for Geert to either queue this for his first v4.7 pull request or tell me to apply it for fixes. Is it a regression? Yours, Linus Walleij
Re: [RFD] Sharing GPIOs for input (buttons) and output (LEDs)
On Tue, Mar 22, 2016 at 2:59 PM, Vaibhav Hiremath <vaibhav.hirem...@linaro.org> wrote: > On Thursday 17 March 2016 03:48 PM, Linus Walleij wrote: > but recently I came across another usecase of shared gpio, > > Say, for example, we have multiple external I2C peripheral which share > interrupt line over gpio (ofcourse irq line is muxed onto single gpio). > > Now in kernel I would have multiple instances of driver supporting each > peripheral but gpio can not be shared for registering irq. > > Do you suggest MFD driver for such simple usecases ? Should gpiolib > support SHARED gpios, and gpiolib can define all the policies over > configuration I don't see why you would want to use MFD at all. Not shared GPIOs but shared interrupt lines. If you're using device tree is is pretty straight forward: - Make sure the GPIOlib and irqchip portions of the driver are totally orthogonal. (See Documentation/gpio/driver.txt section "GPIO drivers providing IRQs") - Just use the irqchip side of the GPIO - Make sure your consumers check if the IRQ is theirs and return IRQ_HANDLED vs NO_IRQ depending on whether it's theirs or not. It is quite obvious from context that in this case all consumers are using an open drain (if active low) scheme. However since the GPIO is then used as input it has no effect, it is more a question for the consumers to assure their IRQ logic is not using push-pull but rather open drain or you will get a disaster in the totempole... > (input only, what about conflicts, bidirectional mode?) We thought about this, see Documentation/gpio/driver.txt section Locking IRQ usage". When the line is locked for IRQ usage, it cannot be switched to output. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.6 (take two)
On Fri, Feb 26, 2016 at 10:59 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 4c96cb027be5ceb2c7c0d4dc086d35fd0cfaf14b > (my previous pull request, which is not yet visible in pinctrl/for-next): > > pinctrl: sh-pfc: r8a7794: Add EtherAVB pin groups (2016-02-18 09:30:59 > +0100) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > sh-pfc-for-v4.6 > > for you to fetch changes up to 4412bb5db6068855b8ad30a058c9d039d3e4f7bd: > > pinctrl: sh-pfc: r8a7795: Add CAN FD support (2016-02-26 13:59:41 +0100) Pulled to my devel branch and pushed to the autobuilders. Yours, Linus Walleij
Re: [PATCH] MAINTAINERS: gpio: add DT bindings directory
On Wed, Apr 13, 2016 at 10:11 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Wed, Apr 13, 2016 at 9:23 AM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> On Tue, Apr 12, 2016 at 5:57 PM, Wolfram Sang <w...@the-dreams.de> wrote: >> >>> From: Wolfram Sang <wsa+rene...@sang-engineering.com> >>> >>> Helps get_maintainer.pl to find the right people. >>> >>> Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> >> >> OK patch applied. >> >> Maybe we should move the ACPI specs into the kernel >> as well as that stuff is taking my time as well now, apart >> from DT :/ > > Wasn't the plan to move the DT bindings out of the kernel tree instead? That never seems to happen. Who would dare to try even. The effect of doing that would probably not be desireable: the kernel maintainers would maybe stop caring about DT bindings and not providing any help at all, and noone would be able to sensibly enforce that drivers and bindings go hand-in-hand. I would think it was a good idea if there was an entity outside the kernel community with the right manpower and time budget to actually just do DT reviews, participating on bindings and kernel driver reviews alike. But that needs to come first, then *maybe* this entity can maintain their own tree, with a copy in the kernel for a trial period and then move it out. Yours, Linus Walleij
Re: [Bug]:LAGER: GPIO-KEYS: Warning occurs after unbinding the e6051000.gpio
On Mon, Apr 25, 2016 at 11:32 AM, Cao Minh Hiep <cm-h...@jinso.co.jp> wrote:
> Hello Linus Walleij-san
>
> We have tested Linux upstream v4.6-rc2 on Renesas's Lager board.
> When we tried to unbind the e6051000.gpio, the following warning messages
> occurs:
>
> "root@linaro-nano:/sys/bus/platform/drivers/gpio_rcar# echo e6051000.gpio >
> unbind
> [ 241.511034] [ cut here ]
> [ 241.525054] WARNING: CPU: 0 PID: 2104 at fs/proc/generic.c:575
> remove_proc_entry+0x13c/0x160
> [ 241.550456] remove_proc_entry: removing non-empty directory 'irq/169',
> leaking at least '6-0039'
Do you mean that you set up a handler in userspace, using the
deprecated sysfs ABI and then unbind the module providing the
IRQ resource?
> And we found a patch between v4.5 and v4.6-rc2 that causing of this issue.
> The patch is "ff2b1359 gpio: make the gpiochip a real device"
It seems the issue is not a bug in the kernel, the issue is that the
kernel is warning you about something that was wrong also before
but you didn't get a warning for it until now.
It is not OK to unbind a driver providing IRQs.
It is even unclear if we should even allow modules to provide
IRQs because there is no way to handle irqchips going away
when it has consumers.
Maybe we should just do this?
diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 3fe8e773d95c..ae5e81358ec5 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -562,6 +562,7 @@ static struct platform_driver gpio_rcar_device_driver = {
.driver = {
.name = "gpio_rcar",
.of_match_table = of_match_ptr(gpio_rcar_of_table),
+ .suppress_bind_attrs = true,
}
};
Yours,
Linus Walleij
Re: [PATCH v2] pinctrl: sh-pfc: Let gpio_chip.to_irq() return zero on error
On Wed, May 4, 2016 at 10:21 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Currrently the gpio_chip.to_irq() callback returns -ENOSYS on error, > which causes bad interactions with the serial_mctrl_gpio helpers. > > mctrl_gpio_init() returns -ENOSYS if GPIOLIB is not enabled, which is > intended to be ignored by its callers. However, ignoring -ENOSYS when it > was caused by a gpiod_to_irq() failure will lead to a crash later: > > Unable to handle kernel paging request at virtual address ffde > ... > PC is at mctrl_gpio_set+0x14/0x78 > > Fix this by returning zero instead, like gpiochip_to_irq() does. > > Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be> > --- > This depends on "gpio: make gpiod_to_irq() return negative for NO_IRQ" > for proper handling by callers of mctrl_gpio_init(). > Hence I think it's best to take this through the GPIO tree, after the > above has been applied. > > v2: > - Return zero instead of -ENXIO. Patch applied. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: Let gpio_chip.to_irq() return -ENXIO on error
On Fri, Apr 29, 2016 at 9:24 AM, Geert Uytterhoeven
<geert+rene...@glider.be> wrote:
> Currrently the gpio_chip.to_irq() callback returns -ENOSYS on error,
> which causes bad interactions with the serial_mctrl_gpio helpers.
>
> mctrl_gpio_init() returns -ENOSYS if GPIOLIB is not enabled, which is
> intended to be ignored by its callers. However, ignoring -ENOSYS when it
> was caused by a gpiod_to_irq() failure will lead to a crash later:
>
> Unable to handle kernel paging request at virtual address ffde
> ...
> PC is at mctrl_gpio_set+0x14/0x78
>
> Fix this by returning -ENXIO instead.
>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---
> Is -ENXIO the right error code?
I would say that whatever the generic helpers in drivers/gpio/gpiolib.c
returns should be the norm. However the guy who wrote them (me)
isn't very smart either. It looks like so:
static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
{
return irq_find_mapping(chip->irqdomain, offset);
}
That returns 0 (NO_IRQ) on failure.
And as you say:
> - Drivers that call irq_find_mapping(), irq_create_mapping(), or
> irq_create_fwspec_mapping() return zero! This also applies to the
> core helper gpiochip_to_irq().
Zero means NO_IRQ.
Reminder:
http://lwn.net/Articles/470820/
What we should do is patch all drivers to return 0 on failure, and
patch any consumers like mctrl_gpio_init() to handle that correctly.
Yours,
Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: Let gpio_chip.to_irq() return -ENXIO on error
On Mon, May 2, 2016 at 11:04 AM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> [silly response deleted]
>
> Scrap it.
:D
> The only annoying thing is that 0 cannot easily be propagated upstream as
> an error code, so it has to be tested for explicitly.
Well with the Big Penguin's clear opinion on the matter there is not
much we can do.
What about this?
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
b/drivers/tty/serial/serial_mctrl_gpio.c
index 02147361eaa9..2297ec781681 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -172,6 +172,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct
uart_port *port, unsigned int idx)
dev_err(port->dev,
"failed to find corresponding irq for
%s (idx=%d, err=%d)\n",
mctrl_gpios_desc[i].name, idx, ret);
+ /*
+* Satisfy the error code semantics for a missing IRQ,
+* 0 means NO_IRQ, but the framework needs to return
+* a negative to deal with the error.
+*/
+ if (!ret)
+ ret = -ENOSYS;
return ERR_PTR(ret);
}
gpios->irq[i] = ret;
Yours,
Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: Let gpio_chip.to_irq() return -ENXIO on error
On Mon, May 2, 2016 at 10:03 AM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> On Sun, May 1, 2016 at 10:48 AM, Linus Walleij <linus.wall...@linaro.org>
> wrote:
>>> - Drivers that call irq_find_mapping(), irq_create_mapping(), or
>>> irq_create_fwspec_mapping() return zero! This also applies to the
>>> core helper gpiochip_to_irq().
>>
>> Zero means NO_IRQ.
>>
>> Reminder:
>> http://lwn.net/Articles/470820/
>>
>> What we should do is patch all drivers to return 0 on failure, and
>> patch any consumers like mctrl_gpio_init() to handle that correctly.
>
> That's the Long Term Plan. There are still too many non-zero NO_IRQ
> definitions in use...
>
> Is -ENXIO acceptable for the short term?
I don't understand. You say you have a problem with
mctrl_gpio_init() which looks like this:
ret = gpiod_to_irq(gpios->gpio[i]);
if (ret <= 0) {
dev_err(port->dev, (...)
This function is already *correctly* handling zero as NO_IRQ
i.e. an error.
Just make your driver return 0/NO_IRQ and it is fixed.
Or are there other problems that you're not telling about?
Yours,
Linus Walleij
Re: [PATCH] Documentation: power: reset: move gpio-{poweroff|restart} to proper place
On Tue, Apr 12, 2016 at 6:00 PM, Wolfram Sang <w...@the-dreams.de> wrote: > From: Wolfram Sang <wsa+rene...@sang-engineering.com> > > I did only find them after a fuzzy search, so let them be where one > would expect them. > > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> I saw this the other day too I agree. Sebastian: are you OK that I move this by committing the patch to the GPIO tree? DT maintainers: OK? Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.7
On Wed, Apr 13, 2016 at 10:45 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca: > > Linux 4.6-rc1 (2016-03-26 16:03:24 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > sh-pfc-for-v4.7 Pulled into the devel branch for v4.7. Thanks Geert! Yours, Linus Walleij
Re: [PATCH] Documentation: power: reset: move gpio-{poweroff|restart} to proper place
On Fri, Apr 15, 2016 at 6:53 PM, Rob Herring <r...@kernel.org> wrote: > On Fri, Apr 15, 2016 at 2:48 AM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> On Tue, Apr 12, 2016 at 6:00 PM, Wolfram Sang <w...@the-dreams.de> wrote: >> >>> From: Wolfram Sang <wsa+rene...@sang-engineering.com> >>> >>> I did only find them after a fuzzy search, so let them be where one >>> would expect them. >>> >>> Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com> >> >> I saw this the other day too I agree. >> Sebastian: are you OK that I move this by committing the patch to the GPIO >> tree? >> DT maintainers: OK? > > Acked-by: Rob Herring <r...@kernel.org> > > Seems like this should go thru my tree though, but I don't have the patch. I have it merged in my tree, is is OK or shall I extract it and send it to you? Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7795: Correct header from R-Car Gen3 to R8A7795
On Tue, Jul 19, 2016 at 9:29 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > This source file handles r8a7795 only, which is not the sole member of > the R-Car Gen3 family. > > Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be> > --- > To be queued up in sh-pfc-for-v4.9. OK! Acked-by. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7792: add SDHI pin groups
On Tue, Jul 12, 2016 at 11:40 PM, Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> wrote: > Add SDHI0 pin groups to the R8A7792 PFC driver. > > Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> Acked-by: Linus Walleij <linus.wall...@linaro.org> I expect Geert to queue this and send it with the first pull request for the v4.9 development cycle. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7792: add missing pinmux data
On Fri, Jul 22, 2016 at 3:51 PM, Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> wrote: > The patch I've based my R8A7792 PFC work on had some VIN pinmux data missing > and I just noticed that while adding the VIN pin groups... > > Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> This looks like a fix that should go in ASAP? Geert, do you want me to apply this directly for fixes with your ACK? Yours, Linus Walleij
Re: [PATCH] gpio: rcar: add R8A7792 support
On Thu, Jul 7, 2016 at 4:11 PM, Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> wrote: > Renesas R8A7792 SoC is a member of the R-Car gen2 family, add support for > its GPIO controllers. > > Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> Patch applied. Yours, Linus Walleij
Re: [PATCH -next] sh-pfc: Use PTR_ERR_OR_ZERO() to simplify the code
On Wed, Jul 6, 2016 at 2:03 PM, <weiyj...@163.com> wrote: > From: Wei Yongjun <yongjun_...@trendmicro.com.cn> > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR. > > Signed-off-by: Wei Yongjun <yongjun_...@trendmicro.com.cn> Patch applied with Laurent's ACK. (Not expecting Geert to collect patches for me this late in the merge cycle.) Yours, Linus Walleij
Re: [PATCH v2] gpio: convince line to become input in irq helper
I sent a patch for the direction setter to be more careful, but
it's no silver bullet for strange semantics.
On Tue, Jul 5, 2016 at 12:07 PM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> [1] gpio_rcar e6052000.gpio: sense irq = 11, type = 8
> ravb e680.ethernet eth0: Base address at 0xe680,
> 2e:09:0a:00:83:1e, IRQ 131.
> ...
> [2] gpiochip_irq_reqres: gpiochip e6052000.gpio
> [3] gpio_rcar e6052000.gpio: gpio_rcar_direction_input: 11
> [4] gpiochip_irq_reqres: desc->flags = 0x0
(...)
> This configures the GPIO for plain input mode, cfr. [3] above, basically
> undoing the configuration from [1]. Hence interrupts no longer come through,
> and Ethernet fails.
The driver is a bit fragile in that it relies on a certain call semantic,
I guess it is not a widespread problem so we should be able to make
a local fix if necessary.
The .set_direction() call should
set the direction. Why is it turning off interrupts as a side effect?
What happens if you apply this, making the .request() function handle
the pin setup and .set_direction() really just setting the
direction?
diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 681c93fb9e70..68fb0147caf4 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -221,7 +221,20 @@ static void
gpio_rcar_config_general_input_output_mode(struct gpio_chip *chip,
struct gpio_rcar_priv *p = gpiochip_get_data(chip);
unsigned long flags;
-/* follow steps in the GPIO documentation for
+spin_lock_irqsave(>lock, flags);
+
+/* Select Input Mode or Output Mode in INOUTSEL */
+gpio_rcar_modify_bit(p, INOUTSEL, gpio, output);
+
+spin_unlock_irqrestore(>lock, flags);
+}
+
+static int gpio_rcar_request(struct gpio_chip *chip, unsigned offset)
+{
+struct gpio_rcar_priv *p = gpiochip_get_data(chip);
+
+/*
+ * follow steps in the GPIO documentation for
* "Setting General Output Mode" and
* "Setting General Input Mode"
*/
@@ -234,14 +247,8 @@ static void
gpio_rcar_config_general_input_output_mode(struct gpio_chip *chip,
/* Select "General Input/Output Mode" in IOINTSEL */
gpio_rcar_modify_bit(p, IOINTSEL, gpio, false);
-/* Select Input Mode or Output Mode in INOUTSEL */
-gpio_rcar_modify_bit(p, INOUTSEL, gpio, output);
-
spin_unlock_irqrestore(>lock, flags);
-}
-static int gpio_rcar_request(struct gpio_chip *chip, unsigned offset)
-{
return pinctrl_request_gpio(chip->base + offset);
}
.request() is always called before .set_direction() when issuing
gpiod_get() anyways, so the order required according to the comment
will be satisfied when the GPIO is first requested, but if we're just
using the interrupt, we still will be able to set the direction right.
Yours,
Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7792: add EtherAVB pin groups
On Tue, Jul 5, 2016 at 5:46 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Tue, Jul 5, 2016 at 4:54 PM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> On Tue, Jul 5, 2016 at 8:57 AM, Geert Uytterhoeven <ge...@linux-m68k.org> >> wrote: >>> Hi Sergei, >>> >>> On Mon, Jul 4, 2016 at 9:52 PM, Sergei Shtylyov >>> <sergei.shtyl...@cogentembedded.com> wrote: >>>> Add the EtherAVB pin groups to the R8A7792 PFC driver. >>>> >>>> Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> >>> >>> Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be> >> >> Should I just apply it? >> >> It's late in the kernel cycle but I do not mind stuff like >> this so much. > > You can't, as I haven't send a pull request for the initial r8a7792 support > yet, > so this patch won't apply ;-) Bring it on. None of your stuff has ever bit me so I'm still happy to pull in some more Renesas stuff. Yours, Linus Walleij
Re: [PATCH 3/7] arm: mach-u300: regulator: add missing of_node_put after calling of_parse_phandle
On Fri, Jul 1, 2016 at 11:41 AM, Peter Chen <peter.c...@nxp.com> wrote: > of_node_put needs to be called when the device node which is got > from of_parse_phandle has finished using. > > Cc: Linus Walleij <linus.wall...@linaro.org> > Signed-off-by: Peter Chen <peter.c...@nxp.com> Acked-by: Linus Walleij <linus.wall...@linaro.org> Yours, Linus Walleij
Re: [RFC 0/5] Renesas RZ series pinctrl driver
On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: >after having discussed in great detail the RZ series per-pin PFC hardware > peculiarities, this is a proposal for a possible pin-based pin controller > driver for SoC devices of Renesas RZ family. > > This RFC series adds a minimal driver infrastructure which supports pin > multiplexing via explicit per-pin settings performed in device tree sources. I think this is what Laurent had in mind when he said something about that he should have taken the per-pin approach from day 1. I would especially like to see Laurent's review and ACK on this series for that reason so we don't create something less than what he is expecting for the future of Renesas pin control. The series as such seems fine and is reusing Tony's work to generalize pin controllers putting functions and groups into the device tree in a nice way, which makes it relevant for him to have a look as well, if he has time. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.11 (take two)
On Fri, Jan 27, 2017 at 10:21 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > The following changes since commit 0e4e4999aac16641f47699e8929693b83a7a4d64: > > pinctrl: sh-pfc: r8a7796: Add HSCIF pins, groups, and functions (2016-12-27 > 10:57:39 +0100) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.11-tag2 > > for you to fetch changes up to 07254d835dfc1e06a8cdfb565e7371176a4b93f9: > > pinctrl: sh-pfc: r8a7791: Add ADI pinconf support (2017-01-20 14:23:40 > +0100) Pulled into the pinctrl devel branch for v4.11. Yours, Linus Walleij
Re: [PATCH v3 3/8] gpio: gpio-rz: GPIO driver for Renesas RZ series
On Thu, Jan 19, 2017 at 10:36 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Thu, Jan 19, 2017 at 10:27 AM, Linus Walleij > <linus.wall...@linaro.org> wrote: >> On Wed, Jan 18, 2017 at 3:06 PM, Geert Uytterhoeven >> <ge...@linux-m68k.org> wrote: >>> On Wed, Jan 18, 2017 at 2:58 PM, Linus Walleij <linus.wall...@linaro.org> >>> wrote: >>>>> + gpio_chip->request = rz_gpio_request; >>>>> + gpio_chip->free = rz_gpio_free; >>>>> + gpio_chip->label = dev_name(>dev); >>>>> + gpio_chip->parent = >dev; >>>>> + gpio_chip->owner = THIS_MODULE; >>>>> + gpio_chip->base = -1; >>>>> + gpio_chip->ngpio = ret == 0 ? args.args[2] : RZ_GPIOS_PER_PORT; >>>> >>>> bgpio_init() will have already set this up to 16 (RZ_GPIOS_PER_PORT) >>>> as we pass width 2 bytes. >>> >>> Note that some banks have less than 16 GPIOs, cfr. the last value of the >>> gpio-ranges tuple being less than 16. >> >> Aha OK then it is fine to override this default value calculate from >> the register size. >> >> But for that case we should use the standard DT property >> ngpios described in >> Documentation/devicetree/bindings/gpio/gpio.txt >> It is for exactly this purpose. > > IC. > > Note that gpio-rcar uses the same method, switching to "ngpios" would > break backwards compatibility. Can we support both? Yours, Linus Walleij
Re: [PATCH v3 3/8] gpio: gpio-rz: GPIO driver for Renesas RZ series
On Wed, Jan 18, 2017 at 3:06 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > Hi Linus, > > On Wed, Jan 18, 2017 at 2:58 PM, Linus Walleij <linus.wall...@linaro.org> > wrote: >>> + gpio_chip->request = rz_gpio_request; >>> + gpio_chip->free = rz_gpio_free; >>> + gpio_chip->label = dev_name(>dev); >>> + gpio_chip->parent = >dev; >>> + gpio_chip->owner = THIS_MODULE; >>> + gpio_chip->base = -1; >>> + gpio_chip->ngpio = ret == 0 ? args.args[2] : RZ_GPIOS_PER_PORT; >> >> bgpio_init() will have already set this up to 16 (RZ_GPIOS_PER_PORT) >> as we pass width 2 bytes. > > Note that some banks have less than 16 GPIOs, cfr. the last value of the > gpio-ranges tuple being less than 16. Aha OK then it is fine to override this default value calculate from the register size. But for that case we should use the standard DT property ngpios described in Documentation/devicetree/bindings/gpio/gpio.txt It is for exactly this purpose. Yours, Linus Walleij
Re: [PATCH v3 3/8] gpio: gpio-rz: GPIO driver for Renesas RZ series
p->io[REG_PPR] = p->io[REG_P]; > + p->io[REG_P] = p->io[REG_PM] = NULL; > + } > + > + ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, > ); > + > + gpio_chip = >gpio_chip; Replace from here: > + gpio_chip->get = rz_gpio_get; > + gpio_chip->set = rz_gpio_set; > + gpio_chip->direction_input = rz_gpio_direction_input; > + gpio_chip->direction_output = rz_gpio_direction_output; > + gpio_chip->get_direction = rz_gpio_get_direction; To here with: ret = bgpio_init(gpio_chip, >dev, 2, p->io[REG_PPR], p->io[REG_P], NULL, NULL, p->io[REG_PM], 0); if (ret) return ret; This might need some flags or I screwed something up, but I'm convinced you can use GENERIC_GPIO like this. The generic accessors also sets the value before switching direction. If you're uncertain about the sematics, read drivers/gpio/gpio-mmio.c. > + gpio_chip->request = rz_gpio_request; > + gpio_chip->free = rz_gpio_free; > + gpio_chip->label = dev_name(>dev); > + gpio_chip->parent = >dev; > + gpio_chip->owner = THIS_MODULE; > + gpio_chip->base = -1; > + gpio_chip->ngpio = ret == 0 ? args.args[2] : RZ_GPIOS_PER_PORT; bgpio_init() will have already set this up to 16 (RZ_GPIOS_PER_PORT) as we pass width 2 bytes. Yours, Linus Walleij
Re: [PATCH] pinctrl: sh-pfc: r8a7795: Add DU support
On Mon, Aug 22, 2016 at 2:23 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Mon, Aug 22, 2016 at 2:10 PM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> Geert, I assume you will queue this? > > Sure, it's part of the pull request I sent last week ;-) Gah good that you remind me, because in some magic way I managed to miss that. Pulled it now, sorry. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.9 (take two)
On Wed, Aug 24, 2016 at 9:24 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > From: Geert Uytterhoeven <geert+rene...@glider.be> > > Hi Linus, > > The following changes since commit 7955dac1b7efab7ae1e7522ea9af3fdc18a257d0: > > pinctrl: sh-pfc: r8a7795: Add DU support (2016-08-16 10:30:43 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.9-tag2 > > for you to fetch changes up to 374cf6992de89fc38d4d923c89b40816d341d678: > > pinctrl: sh-pfc: r8a7796: Add SDHI pins, groups and functions (2016-08-19 > 09:37:20 +0200) Pulled into the pinctrl development branch, thanks! Yours, Linus Walleij
Re: [PATCH resend] pinctrl: sh-pfc: r8a7792: add QSPI pin groups
On Fri, Sep 2, 2016 at 11:50 PM, Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> wrote: > Add QSPI pin groups to the R8A7792 PFC driver. > > Based on the original (and large) patch by Vladimir Barinov > <vladimir.bari...@cogentembedded.com>. > > Signed-off-by: Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> Acked-by: Linus Walleij <linus.wall...@linaro.org> Expecting Geert to queue it. Yours, Linus Walleij
Re: [PATCH] gpio: rcar: Add r8a7796 (R-Car M3-W) support
On Tue, Sep 6, 2016 at 12:35 PM, Simon Horman <horms+rene...@verge.net.au> wrote: > R-Car Gen3's GPIO blocks are identical to Gen2's in every respect. > > Based on work for the r8a7795 (R-Car H3) by Ulrich Hecht. > > Cc: Ulrich Hecht <ulrich.hecht+rene...@gmail.com> > Signed-off-by: Simon Horman <horms+rene...@verge.net.au> > Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be> > Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > Tested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Patch applied. Yours, Linus Walleij
Re: [PATCH 0/3] pinctrl: sh-pfc: r8a779[14]: Implement voltage switching for SDHI
On Thu, Sep 8, 2016 at 1:36 PM, Simon Horman <horms+rene...@verge.net.au> wrote: > this series adds voltage switching for SDHI to sh-pfc for the r8a7791 and > r8a794 SoCs. It is based on work for the r8a7790 SoC by Wolfram Sang. > > The r8a7793 should be able to be trivially supported using the > same mechanism as r8a7791. I intend to provide a follow-up patch > once testing is completed. Acked-by: Linus Walleij <linus.wall...@linaro.org> I expect these to be merged by Geert who send the patches to me. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.9 (take three)
On Wed, Sep 14, 2016 at 9:59 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 374cf6992de89fc38d4d923c89b40816d341d678: > > pinctrl: sh-pfc: r8a7796: Add SDHI pins, groups and functions (2016-08-19 > 09:37:20 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.9-tag3 > > for you to fetch changes up to 77fd4136e58c642482f03d293d5717658a569b4b: > > pinctrl: sh-pfc: r8a7794: Implement voltage switching for SDHI (2016-09-14 > 09:26:54 +0200) Thanks, pulled in and pushed to the autobuilders for test. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.10
On Mon, Nov 7, 2016 at 4:35 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 1001354ca34179f3db924eb66672442a173147dc: > > Linux 4.9-rc1 (2016-10-15 12:17:50 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.10-tag1 Pulled to my devel branch. Thanks! Yours, Linus Walleij
Re: [PATCH v2] gpio: em: depnd on ARCH_SHMOBILE
On Thu, Nov 24, 2016 at 10:38 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Sun, Nov 20, 2016 at 6:48 PM, Peter Robinson <pbrobin...@gmail.com> wrote: >> The GPIO_EM is part of the Renesas SoCs so depend on the arch. >> config GPIO_EM >> tristate "Emma Mobile GPIO" >> - depends on ARM && OF_GPIO >> + depends on (ARCH_SHMOBILE || COMPILE_TEST) && OF_GPIO > > For ARM SoCs, ARCH_SHMOBILE is being phased out in favor of ARCH_RENESAS. > Note that ARCH_SHMOBILE is also set for some SuperH SoCs. > > However, ARCH_EMEV2 is even more suitable here. I fixed the patch in-tree to use ARCH_EMEV2 instead. Thanks! Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.10 (take two)
On Wed, Nov 16, 2016 at 10:47 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 0f866a9679215838328e1c0ed1892224672bb396: > > pinctrl: sh-pfc: r8a7796: Fix GPSR definitions for SDHI2/3 (2016-11-07 > 10:39:11 +0100) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.10-tag2 Pulled into the pin control tree, thanks a lot! Yours, Linus Walleij
Re: [PATCH 0/2] Salvator-X: Add GPIO keys support
On Tue, Nov 15, 2016 at 3:28 PM, Laurent Pinchart <laurent.pinch...@ideasonboard.com> wrote: >> >>> Apart from that, there's also the discrepancy between hardware >> >>> description (the GPIO is connected to both buttons and LEDs, hence it >> >>> should be described that way in DT) >> >> >> >> Correct, this is where a framework change is needed if we want to allow >> >> both the GPIO keyboard and LED drivers to request the GPIO. >> >> Linus, would this be acceptable to you ? Yes, if: - It is solved in a generic way so that there is a call like gpiod_get_nonexclusive() or similar to get a second handle on a GPIOd someone else is already using. - It is cross-coordinated with the regulator people who have exactly the same problem for fixed GPIO regulators and which is why that code looks like hell. Apart from that, we generally need a way to access LEDs as resources with foo-leds = <_foo>; in the device tree just like we can get GPIOs and I hope someone is working on that for this thing... triggers is such a hack. Yours, Linus Walleij
Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
On Fri, Dec 9, 2016 at 12:21 AM, Laurent Pinchart <laurent.pinch...@ideasonboard.com> wrote: > On Friday 09 Dec 2016 00:15:51 Niklas Söderlund wrote: >> This was changed for the whole driver after the original patch was >> applied (at the time of change the patch was not yet reverted), see [1]. >> I needed to update this when I resurrected the patch, maybe I could have >> been more clever and reverted the revert patch but this felt cleaner, if >> it's better to do it the other way around and revert a revert please let >> me know so I can do so in the future. > > No, this is fine. I'm a bit dubious about [1] given that it consumes more CPU > cycles without any benefit as far as I can see. Maybe I can't see far enough > though, Linus Walleij could prove me wrong :-) This was changed mainly for maintainability and code readability. Lots of subsystems have the option to carry around a data poiner and/or allocate an extra memory chunk for state containers. I am relaxed on it, if someone very much want to use container_of() because of $REASON I am not really bothered. Yours, Linus Walleij
Re: [PATCH 2/3] gpio: gpio-rz: GPIO driver for Renesas RZ series
On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote:
> From: Magnus Damm <d...@opensource.se>
>
> This commit combines Magnus' original driver and minor fixes to
> forward-port it to a more recent kernel version (v4.10)
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
>
> ---
> gpio: Renesas RZ GPIO driver V3
>
> This patch adds a GPIO driver for the RZ series of SoCs from
> Renesas. The V3 of the driver requires DT to be used.
>
> The hardware allows control of GPIOs in blocks of up to 16 pins.
> Interrupts are not included in this hardware block, if interrupts
> are needed then the PFC needs to be configured to a IRQ pin function
> which is handled by the GIC hardware.
>
> Tested with yet-to-be-posted DT devices on r7s72100 and Genmai using
> LEDs, DIP switches and I2C bitbang.
>
> Signed-off-by: Magnus Damm <d...@opensource.se>
>
> gpio: gpio-rz: Port to v4.10-rc1
>
> Fix invalid return value in gpio remove function and change gpiochip's
> "dev" field name to "parent" to compile driver against v4.10
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
This commit message looks seriously mangled, please make it look like a regular
one with all signed offs at the end.
> +config GPIO_RZ
> + tristate "Renesas RZ GPIO"
> + depends on ARM
ARM really? Not some Renesas or so?
Include || COMPILE_TEST?
> +#include
> +#include
> +#include
No. Use only
#include
> +static inline struct rz_gpio_priv *gpio_to_priv(struct gpio_chip *chip)
> +{
> + return container_of(chip, struct rz_gpio_priv, gpio_chip);
> +}
Please get rid of this and use gpiochip_get_data()
after adding the chip with devm_gpiochip_add_data()
supplying the data you need.
Look at other GPIO drivers for examples.
> +static int rz_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + /* Get bit from PPR register to determine pin state */
> + return rz_gpio_read_ppr(gpio_to_priv(chip), offset);
return !!rz_gpio_read_ppr(gpio_to_priv(chip), offset);
To clamp to bool.
> +static void rz_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + /* Set bit in P register via PSR to control output */
> + rz_gpio_write(gpio_to_priv(chip), REG_PSR, offset, !!value);
> +}
Ironically you can trust the value to be 0/1 here. Check the
gpiolib.
> +static int rz_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> + return pinctrl_request_gpio(chip->base + offset);
> +}
What about just using gpiochip_generic_request instead?
> +static void rz_gpio_free(struct gpio_chip *chip, unsigned offset)
> +{
> + pinctrl_free_gpio(chip->base + offset);
> +
> + /* Set the GPIO as an input to ensure that the next GPIO request won't
> + * drive the GPIO pin as an output.
> + */
> + rz_gpio_direction_input(chip, offset);
> +}
Very close to gpiochip_generic_free()
Maybe we should actually set lines as input in the
generic routine!
> + gpio_chip = >gpio_chip;
> + gpio_chip->direction_input = rz_gpio_direction_input;
> + gpio_chip->get = rz_gpio_get;
> + gpio_chip->direction_output = rz_gpio_direction_output;
Please implement .get_direction() as well.
> + gpio_chip->set = rz_gpio_set;
> + gpio_chip->request = rz_gpio_request;
> + gpio_chip->free = rz_gpio_free;
> + gpio_chip->label = dev_name(>dev);
> + gpio_chip->parent = >dev;
> + gpio_chip->owner = THIS_MODULE;
> + gpio_chip->base = -1;
> + gpio_chip->ngpio = ret == 0 ? args.args[2] : RZ_GPIOS_PER_PORT;
> +
> + ret = gpiochip_add(gpio_chip);
Use devm_gpiochip_add_data() providing struct rz_gpio_priv * as
data.
> +static int rz_gpio_remove(struct platform_device *pdev)
> +{
> + struct rz_gpio_priv *p = platform_get_drvdata(pdev);
> +
> + gpiochip_remove(>gpio_chip);
> + return 0;
> +}
And with the devm_gpiochip_add_data() you don't even need
this remove().
Yours,
Linus Walleij
Re: [PATCH v2 2/7] gpio: gpio-rz: GPIO driver for Renesas RZ series
On Wed, Jan 11, 2017 at 10:57 AM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: > From: Magnus Damm <d...@opensource.se> > > This commit combines Magnus' original driver and minor fixes to > forward-port it to a more recent kernel version (v4.10) > > Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> A new version appeared while I was reviewing the old version. Oh well. The comments still apply. Yours, Linus Walleij
Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs
On Tue, Jan 10, 2017 at 8:19 PM, Tony Lindgren <t...@atomide.com> wrote: > Below is an experimental fix to intorduce pinctrl_start() that I've > tested with pinctrl-single. Then we should probably make all pin controller > drivers call pinctrl_start() to properly fix the issue of struct pinctrl_dev > handle not being initialized before driver functions are called. Hm I guess that could work, but can we keep pinctrl_register() with the old semantics and add a separate pinctrl_register_and_defer() for those who just wanna start it later by a separate call? Then we don't need any special flags. > Or do you guys have any better ideas? Not really. So you mean revert the previous patch and apply something like this instead? Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.11
On Fri, Jan 13, 2017 at 3:00 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77: > > Linux 4.10-rc1 (2016-12-25 16:13:08 -0800) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.11-tag1 > > for you to fetch changes up to 0e4e4999aac16641f47699e8929693b83a7a4d64: Pulled this in and pushed the branch to the build servers, thanks! Yours, Linus Walleij
Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
On Thu, Dec 8, 2016 at 6:32 PM, Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se> wrote: > From: Geert Uytterhoeven <geert+rene...@glider.be> > > Currently gpio modules are runtime-resumed at probe time. This means the > gpio module will be active all the time (except during system suspend, > if not configured as a wake-up source). > > While an R-Car Gen2 gpio module retains pins configured for output at > the requested level while put in standby mode, gpio register cannot be > accessed while suspended. Unfortunately pm_runtime_get_sync() cannot be > called from all contexts where gpio register access is needed. Hence > move the Runtime PM handling from probe/remove time to gpio request/free > time, which is probably the best we can do. > > On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during > normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6 > for SD-Card CD & WP, gpio7 for keys and regulators). > > Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be> > [Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/] > Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se> Patch applied. Yours, Linus Walleij
Re: [PATCH 1/2] gpio: rcar: set IRQ chip parent_device
On Thu, Dec 8, 2016 at 6:32 PM, Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se> wrote: > This enables Runtime PM handling for interrupts. > > By setting the parent_device in struct irq_chip genirq will call the > pm_runtime_get/put APIs when an IRQ is requested/freed. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se> Patch applied with the ACKs. Yours, Linus Walleij
Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
On Thu, Mar 23, 2017 at 5:02 PM, jacopo <jac...@jmondi.org> wrote: >> > + Required properties: >> > +- renesas,pins >> >> Just "pins"? >> > > You know, I've been thinking about this, bu the "pins" property > definition in pinctrl-bidings is the following one: > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt > --- > - pins takes a list of pin names or IDs as a required argument. The > specific binding for the hardware defines: > - Whether the entries are integers or strings, and their > meaning. > --- > > And all examples there assume one "pin name" or "ID" per pin. > > Now, we use 2 values per each pin (the pin ID and the alternate > function number), so to me this is different from what the generic > binding describes. > Also, pinctrl-single, and pinctrl-imx which have and ABI similar to > the one this driver define, use "pinctrl-single,pins" and "fsl,pins" > respectively as property names. > So either they have to be updated yet, or we should keep using > "renesas,pins" for our own defined ABI. > > Maybe Linus or other pinctrl people can give some suggestion here. To me as subsystem maintainer any "necessarily different" bindings are just a big confusion for the head. Since you're adding a new driver, try to stick to the generic bindings even if it deviates from what you are used to for Renesas, because even if it may be more work for you guys or make you annoyed that now a certain Renesas is different from all other Renesas platforms, for the community this makes things easier to maintain because we can look at the driver and its bindings and say "ah I know this". The fact that historically all the early adopters of pinctrl in device tree have these funky custom bindings is unfortunate but just something that we need to live with. Yours, Linus Walleij
Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header
ith the muxing, so you can do what the Qualcomm driver >> does and add custom pin configurations extending the generic >> pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c >> qcom,pull-up-strength etc. >> > > I see, but that custom pin configuration flag can be applied > independently from pin muxing procedure and it can be applied to pins > while they're configured in GPIO mode. See "GPIO mode pitfalls" in Documentation/pinctrl.txt I've been over this hardware lingo so many times already. It is not about "GPIO" at all, it is about pin configuration. The generic pin config was not invented for GPIO, it was just recently that we started to provide pin config back-ends for GPIO. Only Intel do that so far. > Our "flags" are not of that nature, and only apply to some register > setting during pinmux, as I hopefully tried to clarify above. And that is a driver semantic. Or even a subsystem semantic. No big deal, accumulate such writes in the driver and apply it all when you have it all available no matter if pin multiplexing or pin config happens first? Surely this is just a hardware pecularity, then it warrants some special driver code. If you definately feel you must get a call from the pin control core setting up muxing and config at the same time we need to think of a way to augment the pin control core if necessary? The fact that Linux pin control subsystem semantics you don't like does not affect the relevant device tree bindings. Yours, Linus Walleij
Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header
On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: > Add dt-bindings for Renesas r7s72100 pin controller header file. > > Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> > +/* > + * Pin is bi-directional. > + * An alternate function that needs both input/output functionalities shall > + * be configured as bidirectional. > + * Eg. SDA/SCL pins of an I2c interface. > + */ > +#define BI_DIR (1 << 3) Any specific reason why this should not simply be added to include/linux/pinctrl/pinconf-generic.h as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf-generic.c from the (new) DT property "bidirectional" simply? > +/* > + * Flags used to ask software to drive the pin I/O direction overriding the > + * alternate function configuration. > + * Some alternate functions require software to force I/O direction of a pin, > + * overriding the designated one. > + * Refer to the HW manual to know when this flag shall be used. > + */ > +#define SWIO_IN(1 << 4) > +#define SWIO_OUT (1 << 5) What is wrong in doing this with generic pin config using PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT (ignoring the argument)? In the device tree use input-enable and add a new output-enable (with unspecified value) with proper description and DT bindings? And if you think these have no general applicability, by the end of the day they are *still* pin config, not magic flags we can choose to toss in with the muxing, so you can do what the Qualcomm driver does and add custom pin configurations extending the generic pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c qcom,pull-up-strength etc. Yours, Linus Walleij
Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header
On Wed, Mar 29, 2017 at 4:55 PM, Chris Brandt <chris.bra...@renesas.com> wrote: > On Wednesday, March 29, 2017, Linus Walleij wrote: >> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> >> wrote: >> >> > Add dt-bindings for Renesas r7s72100 pin controller header file. >> > >> > Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> >> >> > +/* >> > + * Pin is bi-directional. >> > + * An alternate function that needs both input/output functionalities >> > +shall >> > + * be configured as bidirectional. >> > + * Eg. SDA/SCL pins of an I2c interface. >> > + */ >> > +#define BI_DIR (1 << 3) >> >> Any specific reason why this should not simply be added to >> include/linux/pinctrl/pinconf-generic.h >> as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf- >> generic.c from the (new) DT property "bidirectional" simply? > > I see your point. It would cut down from every driver out there > inventing some new property or config instead of everyone just sharing > a fixed set. > Maybe someone else out there will end up having a need for a > "bidirectional" option. I was thinking about that one. It is a bit weird electrically, like what kind of electronics is really bidirectional? It seems like a fancy name for open drain/open source, what we call "single ended" configuration. (See docs in Documentation/gpio/driver.txt) It would be great if you could check if that is what they mean, actually. > But, what do we do for Ethernet? All the pins are "normal" except just > the MDIO pin needs to be bidirectional. I see Geert clarified what we could do here. Yours, Linus Walleij
Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
On Wed, Mar 29, 2017 at 1:20 PM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
>> I do not understand the notion of "flags" here. I hope that is not referring
>
> Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from
> https://patchwork.kernel.org/patch/9643047/
Aha I will go in and review that closer because it doesn't seem right.
Sorry for missing it.
>> i2c1_pins_a: i2c1@0 {
>> pins {
>> pinmux = ,
>> ;
>
> If we follow this example, then we can list all combinations in
> include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value
> by combining the bits using a macro where we need it in the DTS.
>
> It's gonna be a long list, though...
Size is not the issue, readability is the issue. I don't see why you would
need to list "all combinations" since the trees go through the C preprocessor
so you can use macros and bit | OR to build them?
Yours,
Linus Walleij
Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
On Wed, Mar 29, 2017 at 2:38 PM, Chris Brandt <chris.bra...@renesas.com> wrote: >> I'm strongly in favour of something like >> pinmux = <PIN(1, 4) | FUNC# | FLAGS>, ; >> >> opposed to >> pinmux = <PIN1_4_FUNC#_FLAGS>, ... ; > > > I agree. I like "<PIN(1, 4) | FUNC# | FLAGS>". I don't know if you have seen the Mediatek things really. include/dt-bindings/pinctrl/mt6397-pinfunc.h Example: #define MT6397_PIN_2_SRCLKEN_PERI__FUNC_GPIO2 (MTK_PIN_NO(2) | 0) If you prefer to use preprocessor macros or whatever to make the bitmasks or how you want to organize the constants in your include files is not my concern, do whatever you seem fit, just pack it into a 32bit thing somehow which makes sense from a maintenance point of view. >> Not only because it will save use from having a loong list(*) of macros >> that has to be kept up to date when/if new RZ hardware will arrive, but >> also because of readability and simplicity for down-stream and BSP users. >> Speaking of which, I would like to know what does Chris think of this. > > The list of macros would be very long, especially against the different > packaging version of the RZ/A1 series. 11 ports, 16-pins for each port, 8 > different > function options for each pin2 different package/pin variations. > > And at the end of the daythere is no benefit for the user over just using > a macro. I don't know who has this idea that you could not use macros, certainly not me. Some misunderstanding must be going on. For what I'm concerned you can write hex numbers in the pinmux = <0x12345678>; > The reason for the "FLAGS" is to work around a quirky hardware design (in my > opinion). The flags I don't like at all, and think they should be converted to generic pin config because they have nothing to do with muxing. But I will point that out in the specific patch adding them. Yours, Linus Walleij
Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
On Tue, Mar 28, 2017 at 4:38 PM, <jac...@jmondi.org> wrote: >> The fact that historically all the early adopters of pinctrl in device >> tree >> have these funky custom bindings is unfortunate but just something >> that we need to live with. >> > > To avoid any confusion, please bear with me and clarify this once and for > all, > since I'm not certain I fully got you here. > > Are you suggesting: > > 1) Use "pins" property with the currently implemented ABI (which slightly > differs >from the standard documented one as explained above. Not sure it is fine > overriding >it or not) Correction: you should be using the property "pinmux", because you are setting group and function at the same time. See for example: include/dt-bindings/pinctrl/mt65xx.h And how that is used in: arch/arm/boot/dts/mt2701-pinfunc.h arch/arm/boot/dts/mt2701-evb.dts The docs are here: Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt I'm sorry that "pinmux" is not part of the generic documentation, it'd be great if you would like to add it with a patch. Yours, Linus Walleij
Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller
On Fri, Mar 24, 2017 at 5:45 PM, jacopo <jac...@jmondi.org> wrote: > I initially created a whole new sub-directory where all the Renesas > devices with this kind of pin controller would have gone > (drivers/pinctrl/rz-pfc) but since as of now the only available > hardware of that type is RZ/A1 and some of its variants, we decided to go > with a single driver supporting that platform only. > If more will come, we'll think about supporting them through this > driver if possible, or create a dedicated directory. > > Are you ok with this? Yep it's OK. It's no big deal to move it into a new subdir if many new drivers start popping in anyway. Right now I see the use of renesas,pins as the only big blocker, I would much like it to use just pins = <>; Yours, Linus Walleij
Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller
On Wed, Mar 29, 2017 at 9:30 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Fri, Mar 24, 2017 at 4:42 PM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> >> wrote: >> >> I assume Geert will queue this driver even if it is outside of sh-pfc? > > OK for me, thanks. I was actually wondering about that ;-) Thanks Geert, much appreciated. Yours, Linus Walleij
Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
On Wed, Mar 29, 2017 at 9:35 AM, Geert Uytterhoeven
<ge...@linux-m68k.org> wrote:
> Me:
>> See for example:
>> include/dt-bindings/pinctrl/mt65xx.h
>>
>> And how that is used in:
>> arch/arm/boot/dts/mt2701-pinfunc.h
>> arch/arm/boot/dts/mt2701-evb.dts
>>
>> The docs are here:
>> Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
>
> All of the above pack the information for a pin into a single 32-bit integer.
> Is that what you want us to use, too?
> Currently we use two integers: 1) pin index, and 2) function/flags combo.
I don't really know what you need, sorry. But some kind of figure, yes.
I would say whatever makes sense. 16+16 bits makes sense in most
combinatorial spaces does it not? If you split 32 bits in 16 bits for
pin and 16 bits for function, do you have more than 2^16 pins or 2^16
functions?
If you really do we may need to go for u64 but ... really? Is there
a rational reason for that other than "we did it like this first"?
I do not understand the notion of "flags" here. I hope that is not referring
to pin config, because I expect that to use the standard pin config
bindings outside of the pinmux value which should just define the
pin+function combo:
node {
pinmux = ;
GENERIC_PINCONFIG;
};
Example from Mediatek:
i2c1_pins_a: i2c1@0 {
pins {
pinmux = ,
;
bias-pull-up = <55>;
};
};
So this allows for a combine pin+function number but pull ups,
bias etc are not baked into the thing, they have to be added on
separately with the generic bindings, which is nice and very readable.
Yours,
Linus Walleij
Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller
On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote:
I assume Geert will queue this driver even if it is outside of sh-pfc?
> Add combined gpio and pin controller driver for Renesas RZ/A1
> r7s72100 SoC.
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
> drivers/pinctrl/Kconfig| 10 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-rza1.c | 961
> +
So this is very different from the SH-PFC family and should not
be in drivers/pinctrl/sh-pfc?
> +config PINCTRL_RZA1
> + bool "Renesas RZ/A1 gpio and pinctrl driver"
> + depends on OF
> + depends on ARCH_R7S72100 || COMPILE_TEST
> + select GENERIC_PINCTRL_GROUPS
> + select GENERIC_PINMUX_FUNCTIONS
> + select GENERIC_PINCONF
If it is also a GPIO driver I guess it should
select GPIOLIB as well.
This was not possible in the past, but it is possible nowadays.
> +struct gpio_chip rza1_gpiochip_template = {
> + .request= rza1_gpio_request,
> + .free = rza1_gpio_free,
> + .get_direction = rza1_gpio_get_direction,
> + .direction_input= rza1_gpio_direction_input,
> + .direction_output = rza1_gpio_direction_output,
> + .get= rza1_gpio_get,
> + .set= rza1_gpio_set,
> +};
We now also have .set_multiple() and more interestingly
.set_config() which can be backed by pinctrl if you want
to e.g. support debouncing and/or open drain/open source.
Maybe this is stuff your pin controller can do, but not needed
in the initial submission for sure.
> +static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int
> selector,
> + unsigned int group)
Please name it rza1_set_mux() to correspond with the ops field.
Yours,
Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.12
On Fri, Mar 24, 2017 at 10:39 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201: > > Linux 4.11-rc1 (2017-03-05 12:59:56 -0800) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.12-tag1 > > for you to fetch changes up to 3091ae775fae17084013021d01513bc1ad274e6a: > > pinctrl: sh-pfc: Update info pointer after SoC-specific init (2017-03-21 > 11:21:55 +0100) Pulled into my devel branch in the pinctrl tree. Thanks! Linus Walleij
Re: [PATCH 0/2] pinctrl: sh-pfc: r8a7795: Misc fixes and cleanups
On Mon, Mar 13, 2017 at 5:41 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > I plan to queue these up in sh-pfc-for-v4.12. Thanks, looking forward to pull request. I realized I needed a maintainer collecting Samsung patches as well this week, so we may get more people maintaining subdirs in pinctrl... Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.12 (take two)
On Fri, Mar 31, 2017 at 11:26 AM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > The following changes since commit 3091ae775fae17084013021d01513bc1ad274e6a: > > pinctrl: sh-pfc: Update info pointer after SoC-specific init (2017-03-21 > 11:21:55 +0100) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.12-tag2 > > for you to fetch changes up to d14a39edf757f5bdd73cf25d0155d7cfb271e782: > > pinctrl: sh-pfc: r8a7795: Add SCIF_CLK support (2017-03-30 13:43:55 +0200) Pulled into the pinctrl devel branch, thanks! Yours, Linus Walleij
Re: [RFC] Documentation: pinctrl: Add "pinmux" property
On Thu, Apr 6, 2017 at 12:35 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: > Document "pinmux" property as part of generic pin controller > documentation. > Fix 2 minor typos in documentation while at there. > > Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> Patch applied. This is documenting what is already the case and is as such very helpful. > Sending this one as RFC to collect feedback. I just applied it. It is already much better than what we have. If more adjustments are needed, people can send additional patches. > If the change is not rejected is it > worth adding to pin controller core helper functions to parse the newly > documented property, as this commit > <https://patchwork.kernel.org/patch/9411231/> > did for "pinctrl-pin-array" one? > > All drivers using "pinmux" exhibit the same behavior which is fine as long as > "pinmux" only accepts a list of u32 parameters. > > ... > pins = of_find_property(node, "pinmux", NULL); > ... > npins = pins->length / sizeof(u32); > ... > of_property_read_u32_index(node, "pinmux", > i, ); > ... It makes sense. Just make sure to move users over to use the helpers. Yours, Linus Walleij
Re: [PATCH v4 3/9] dt-bindings: pinctrl: Add RZ/A1 bindings doc
On Mon, Apr 10, 2017 at 8:12 PM, Rob Herring <r...@kernel.org> wrote:
> On Wed, Apr 05, 2017 at 04:07:21PM +0200, Jacopo Mondi wrote:
>> + The allowed generic formats for a pin multiplexing sub-node are the
>> + following ones:
>> +
>> + node-1 {
>> + pinmux = , , ... ;
>> + GENERIC_PINCONFIG;
>
> What's GENERIC_PINCONFIG? I see this in some other binding docs, but not
> used anywhere. If this is a boolean property then get rid of the all
> caps. If this is a define, then don't use complex defines that expand to
> dts source.
I guess it is a wildcard for everything under the heading in
"Generic pin configuration node content"
in Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
I'm all for documenting it properly.
It's kind of useful, but I don't know the recent ambtions about being
formal with DT bindings. The GPIO bindings are just over the top
with BNF notation in its formalism. Dunno what is best here :/
Yours,
Linus Walleij
Re: [PATCH v4 1/9] pinctrl: generic: Add bi-directional and output-enable
On Wed, Apr 5, 2017 at 4:07 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: > Add bi-directional and output-enable pin configuration properties. > > bi-directional allows to specify when a pin shall operate in input and > output mode at the same time. This is particularly useful in platforms > where input and output buffers have to be manually enabled. > > output-enable is just syntactic sugar to specify that a pin shall > operate in output mode, ignoring the provided argument. > This pairs with input-enable pin configuration option. > > Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> Patch applied with Rob's ACK. Yours, Linus Walleij
Re: [PATCH v4 0/9] Renesas RZ/A1 pin and gpio controller
On Wed, Apr 5, 2017 at 4:07 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: > Hi Linus, >this is 4th round of gpio/pincontroller for RZ/A1 devices. > > As you suggested in v3 review, I have now added what we called pinmux flags > to the list of standard pinconf generic properties, and we're now using > generic parsing routines to collect them and apply them when multiplexing > pins. I have merged patch 1/9 so you have the necessary infrastructure in place. If Geert want to send a pull request based on my devel branch that is fine (but a bit late) else the requirements are there for a merge in the early v4.12 kernel cycle. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.12 (take three)
On Mon, Apr 24, 2017 at 1:06 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > The following changes since commit d14a39edf757f5bdd73cf25d0155d7cfb271e782: > > pinctrl: sh-pfc: r8a7795: Add SCIF_CLK support (2017-03-30 13:43:55 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.12-tag3 > > for you to fetch changes up to 5f4c8cafe1148f8a91287072815df8f0b66f0e5c: > > pinctrl: sh-pfc: r8a7794: Swap ATA signals (2017-04-05 09:41:29 +0200) Thanks, pulled into my devel branch for v4.12. Yours, Linus Walleij
Re: [PATCH 0/5] pinctrl: sh-pfc: r8a7795/r8a7796: MSIOF updates
On Wed, Jul 12, 2017 at 12:31 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, Laurent, > > This patch series fixes MSIOF-related bugs in the R-Car H3 ES2.0 and M3-W > pin control drivers, and enables support for MSIOF on R-Car H3 ES2.0. > > I plan to queue these up in sh-pfc-for-v4.14. All look good to me. Yours, Linus Walleij
Re: [PATCH] gpio: rcar: reinstate generic compat string
On Thu, Aug 10, 2017 at 10:06 AM, Simon Horman <ho...@verge.net.au> wrote:
> On Wed, Aug 09, 2017 at 10:21:06AM +0200, Simon Horman wrote:
>> commit d10bbd156926 ("gpio: rcar: add gen[123] fallback compatibility
>> strings") deprecated the generic compat string, renesas,gpio-rcar. After
>> further discussion this appears not to have been desirable as that compat
>> string is compatible with all R-Car SoCs supported in upstream.
>>
>> This commit partially reverts that commit, and updates related
>> documentation and examples.
>>
>> Fixes: d10bbd156926 ("gpio: rcar: add gen[123] fallback compatibility
>> strings")
>> Signed-off-by: Simon Horman <horms+rene...@verge.net.au>
>
> Hi Linus,
>
> I believe Geert is on holidays at the moment (maybe you are to‽).
Nah neither of us are as it seems :)
I wait for the two of you to figure this out before applying anything.
Yours,
Linus Walleij
Re: [PATCH 2/2] gpio: make regmap_irq_chip const
On Sat, Aug 12, 2017 at 8:09 AM, Bhumika Goyal <bhumi...@gmail.com> wrote: > Make the structure const as it is only passed to the function > devm_regmap_add_irq_chip having the corresponding argument as const. > Done using Coccinelle. > > Signed-off-by: Bhumika Goyal <bhumi...@gmail.com> Patch applied. Yours, Linus Walleij
Re: [PATCH] pinctrl: rza1: Remove unneeded wrong check for wrong variable
On Fri, Jun 30, 2017 at 8:57 AM, Geert Uytterhoeven
<geert+rene...@glider.be> wrote:
> Depending on compiler version:
>
> drivers/pinctrl/pinctrl-rza1.c: In function ‘rza1_pinctrl_probe’:
> drivers/pinctrl/pinctrl-rza1.c:1260:5: warning: ‘ret’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> if (ret)
> ^
>
> Indeed, the result returned by platform_get_resource() was stored in
> "res", not "ret". In addition, the correct error check would be
> "if (!res)", as platform_get_resource() does not return an error code,
> but returns NULL on failure.
>
> However, as devm_ioremap_resource() verifies the validity of the passed
> resource pointer anyway, the check can just be removed.
>
> Reported-by: Stephen Rothwell <s...@canb.auug.org.au>
> Fixes: 5a49b644b3075f88 ("pinctrl: Renesas RZ/A1 pin and gpio controller")
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
Patch applied.
Yours,
Linus Walleij
Re: [PATCH v2] pinctrl: generic: Add output-enable property
On Thu, Jun 22, 2017 at 12:00 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> wrote: > Add output-enable generic pin configuration property. > This properties allows enabling/disabling pin's output capabilities > without actually driving any value on the line. > > --- > v1->v2: > - Expand the property description as suggested by Laurent. I ended up >mentioning the in-famous output buffer :) I have applied the patch adding a few elaborations in the binding that the situation where you may use this is typically when enabling/disabling input or output buffers irrespective of the mode of the line as a whole. We might need even more documentation because this is really confusing. But for now it lets drivers work, which is nice. Rough consensus and running code should be our guide. Yours, Linus Walleij
Re: [git pull] pinctrl: sh-pfc: Updates for v4.13 (take two)
On Wed, Jun 28, 2017 at 7:10 PM, Geert Uytterhoeven <geert+rene...@glider.be> wrote: > Hi Linus, > > The following changes since commit c8bac70f079bb3ecaf9a716f141f3d85cef27231: > > pinctrl: sh-pfc: r8a7794: Add R8A7745 support (2017-05-16 13:53:15 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git > tags/sh-pfc-for-v4.13-tag2 > > for you to fetch changes up to c03a133bba8662daecd7db09819fa369663d0ea7: > > pinctrl: sh-pfc: r8a7795: Add PWM support (2017-06-26 10:21:38 +0200) Pulled in for devel. It's late, but you all worked hard to make this happen so we should be liberal, also it is a new driver. Yours, Linus Walleij
Re: [PATCH v2] gpio: rcar: Add R8A7743 (RZ/G1M) support
On Wed, Jun 21, 2017 at 4:27 PM, Biju Das <biju@bp.renesas.com> wrote: > Renesas RZ/G1M (R8A7743) SoC GPIO blocks are identical to the R-Car Gen2 > family. Add support for its GPIO controllers. > > Signed-off-by: Biju Das <biju@bp.renesas.com> > Reviewed-by: Chris Paterson <chris.paters...@renesas.com> > --- > v1->v2 > * Modified the text "RZ-G1M" to "RZ/G1M" Patch applied with the ACKs. Yours, Linus Walleij
Re: [PATCH 3/4] gpio: Add ROHM BD9571MWV-M PMIC GPIO driver
On Sun, Apr 16, 2017 at 8:08 PM, Marek Vasut <marek.va...@gmail.com> wrote: > Add driver for the GPIO block in the ROHM BD9571MWV-W MFD PMIC. > This block is pretty trivial and supports setting GPIO direction > as Input/Output and in case of Output, supports setting value. > > Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> > Cc: linux-g...@vger.kernel.org > Cc: Geert Uytterhoeven <geert+rene...@glider.be> > Cc: Linus Walleij <linus.wall...@linaro.org> (...) > +#include Only use #include It should be enough. With that: Reviewed-by: Linus Walleij <linus.wall...@linaro.org> I guess you plan to merge this through MFD? Yours, Linus Walleij
Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable
On Thu, Apr 27, 2017 at 4:56 PM, Andy Shevchenko <andy.shevche...@gmail.com> wrote: > On Thu, Apr 27, 2017 at 11:19 AM, Jacopo Mondi > <jacopo+rene...@jmondi.org> wrote: >> Add bi-directional and output-enable pin configuration properties. >> >> bi-directional allows to specify when a pin shall operate in input and >> output mode at the same time. This is particularly useful in platforms >> where input and output buffers have to be manually enabled. >> >> output-enable is just syntactic sugar to specify that a pin shall >> operate in output mode, ignoring the provided argument. >> This pairs with input-enable pin configuration option. > > For me it looks like you are trying to alias open-drain + bias or > alike. Don't actually see the benefit of it. Andy is bringing up a valid point. And I remember asking about this before. What does "bi-directional" really mean, electrically speaking? Does is just mean open drain and/or open source actually? (See Documentation/gpio/driver.txt for an explanation of how open drain/source works.) When you set an output without setting a value, what happens electrically? Isn't this bias-high-impedance / High-Z? Hopefully you can find the answer from Renesas hardware dept. You can certainly call it whatever the datasheet calls it in your driver #defines but for the DT bindings we would ideally have the physical world things. Yours, Linus Walleij
Re: [PATCH v5 10/10] arm: dts: genmai: Add ethernet pin group
On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi
<jacopo+rene...@jmondi.org> wrote:
> Add pin configuration subnode for ETHER ethernet controller.
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
(...)
> + pins_bidir {
> + pinmux = <RZA1_PINMUX(3, 3, 2)>;/* P3_3 = ET_MDIO */
> + bi-directional;
> + };
So I'm against merging this until someone explains what "bi-directional"
actually means, electrically speaking. What happens physically on this pin?
I think this just means open drain.
It is dangerous to merge things we don't understand.
Surely someone inside Renesas can answer this question.
Yours,
Linus Walleij
Re: [RESEND][PATCH V2 3/4] gpio: Add ROHM BD9571MWV-M PMIC GPIO driver
On Tue, Apr 25, 2017 at 8:32 PM, Marek Vasut <marek.va...@gmail.com> wrote: > Add driver for the GPIO block in the ROHM BD9571MWV-W MFD PMIC. > This block is pretty trivial and supports setting GPIO direction > as Input/Output and in case of Output, supports setting value. > > Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> > Cc: linux-g...@vger.kernel.org > Cc: Geert Uytterhoeven <geert+rene...@glider.be> > Cc: Linus Walleij <linus.wall...@linaro.org> > Reviewed-by: Linus Walleij <linus.wall...@linaro.org> > --- > V2: Use linux/gpio/driver.h instead of linux/gpio.h I applied this for v4.12 to bring down the number of deps in v4.13. The guarding symbol makes it not compile until the MFD patch is there, the MFD_BD9571MWV is unlikely to change, and Marek will certainly go through with the driver submission. Yours, Linus Walleij
Re: [PATCH v4 2/9] pinctrl: Renesas RZ/A1 pin and gpio controller
On Wed, Apr 26, 2017 at 2:21 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Wed, Apr 5, 2017 at 4:07 PM, Jacopo Mondi <jacopo+rene...@jmondi.org> > wrote: >> Add combined gpio and pin controller driver for Renesas RZ/A1 >> r7s72100 SoC. >> >> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org> > >> --- /dev/null >> +++ b/drivers/pinctrl/pinctrl-rza1.c > >> +/* >> + * Keep this up-to-date with pinconf-generic.h: it performs packing of >> + * pin conf flags and argument during pinconf_generic_parse_dt_config(); >> + * we simply discard pinconf argument here >> + */ >> +#define PIN_CONF_UNPACK(pinconf) ((pinconf) & 0xffUL) > > Perhaps this should be moved to pinconf-generic.h, to make sure it stays > up-to-date? I agree. Use the generic macros. If further processing is needed, make a static inline to discard config flags etc. Yours, Linus Walleij
Re: [PATCH v5 02/10] pinctrl: generic: Add macros to unpack properties
On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi
<jacopo+rene...@jmondi.org> wrote:
> Add PIN_CONF_UNPACK_PARAM and PIN_CONF_UNPACK_ARGS macros useful to
> unpack generic properties and their arguments
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
(...)
/*
* Helpful configuration macro to be used in tables etc.
Then this should say "macros" rather than "macro".
> -#define PIN_CONF_PACKED(p, a) ((a << 8) | ((unsigned long) p & 0xffUL))
> +#define PIN_CONF_PACKED(p, a) (((a) << 8) | ((unsigned long) (p) & 0xffUL))
Also adding some extra parantheses I see.
> +#define PIN_CONF_UNPACK_PARAM(c) ((c) & 0xffUL)
> +#define PIN_CONF_UNPACK_ARGS(c) ((c) >> 8)
But why.
I have these two static inlines just below your new macros:
static inline enum pin_config_param pinconf_to_config_param(unsigned
long config)
{
return (enum pin_config_param) (config & 0xffUL);
}
static inline u32 pinconf_to_config_argument(unsigned long config)
{
return (u32) ((config >> 8) & 0xffUL);
}
Why can't you use this in your code instead of macros?
We generally prefer static inlines over macros because they are easier
to read.
Yours,
Linus Walleij
Re: [PATCH V2 3/4] gpio: Add ROHM BD9571MWV-M PMIC GPIO driver
On Mon, Apr 24, 2017 at 5:21 PM, Marek Vasut <marek.va...@gmail.com> wrote: > Add driver for the GPIO block in the ROHM BD9571MWV-W MFD PMIC. > This block is pretty trivial and supports setting GPIO direction > as Input/Output and in case of Output, supports setting value. > > Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> > Cc: linux-g...@vger.kernel.org > Cc: Geert Uytterhoeven <geert+rene...@glider.be> > Cc: Linus Walleij <linus.wall...@linaro.org> > Reviewed-by: Linus Walleij <linus.wall...@linaro.org> > --- > V2: Use linux/gpio/driver.h instead of linux/gpio.h Reviewed-by: Linus Walleij <linus.wall...@linaro.org> Are you merging this through MFD? Yours, Linus Walleij
Re: [PATCH 00/14] pinctrl: sh-pfc: r8a7796: Fix pin assignment definitions
On Wed, Jul 12, 2017 at 6:55 PM, Yoshihiro Kaneko <ykaneko0...@gmail.com> wrote: > This series fixes pin assignment definitions for R8A7796 SoC. > This series is based on the for-next branch of linux-pinctrl tree. I'm relying on Geert to review and/or queue these changes. He will forward them to me when he's happy with them. Yours, Linus Walleij
