Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1240214 > unrolled thread

Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller Linus Walleij <linus.walleij@linaro.org> - 2015-10-06 09:40 +0200
    Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller Andreas Werner <andy@wernerandy.de> - 2015-10-06 20:00 +0200
    Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller Andreas Werner <andy@wernerandy.de> - 2015-10-08 19:00 +0200
      Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller Linus Walleij <linus.walleij@linaro.org> - 2015-10-16 22:00 +0200
    Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller Andreas Werner <andy@wernerandy.de> - 2015-10-08 21:50 +0200
      Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller Linus Walleij <linus.walleij@linaro.org> - 2015-10-16 22:00 +0200

#1240214 — Re: [PATCH] gpio: Add driver for MEN 16Z127 GPIO controller

On Mon, Oct 5, 2015 at 8:09 PM, Andreas Werner <andy@wernerandy.de> wrote:

> The 16Z127 is a GPIO controller on a MCB FPGA and has 32
> configurable GPIOs.
> The GPIOs can be configured as inputs and outputs
>
> Signed-off-by: Andreas Werner <andy@wernerandy.de>

This driver looks like it can use the generic MMIO library.

select GPIO_GENERIC
#include <linux/basic_mmio_gpio.h>

Then look at example for how to do this, e.g.
drivers/gpio/gpio-74xx-mmio.c

> +config GPIO_MENZ127
> +       tristate "MEN 16Z127 GPIO support"
> +       depends on MCB

Is this "MCB" symbol already upstream? It seems a bit short.

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/mcb.h>
> +#include <linux/gpio.h>

Just
#include <linux/gpio/driver.h>

> +#define MEN_Z127_CTRL  0x00
> +#define MEN_Z127_PSR   0x04
> +#define MEN_Z127_IRQR  0x08
> +#define MEN_Z127_GPIODR        0x0c
> +#define MEN_Z127_IER1  0x10
> +#define MEN_Z127_IER2  0x14

It looks like it has interrupt support?

> +#define MEN_Z127_DBER  0x18

Debounce? In that case, maybe implement .set_debounce from day 1?

> +#define MEN_Z127_ODER  0x1C

And Open Drain? Maybe you should support that from day 1?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1240820

On Tue, Oct 06, 2015 at 09:35:51AM +0200, Linus Walleij wrote:
> On Mon, Oct 5, 2015 at 8:09 PM, Andreas Werner <andy@wernerandy.de> wrote:
> 
> > The 16Z127 is a GPIO controller on a MCB FPGA and has 32
> > configurable GPIOs.
> > The GPIOs can be configured as inputs and outputs
> >
> > Signed-off-by: Andreas Werner <andy@wernerandy.de>
> 
> This driver looks like it can use the generic MMIO library.

Yes you are right, did not know about the gener MMIO lib but seems to be 
the right think for this driver. I will change that.

> 
> select GPIO_GENERIC
> #include <linux/basic_mmio_gpio.h>
> 
> Then look at example for how to do this, e.g.
> drivers/gpio/gpio-74xx-mmio.c
> 
> > +config GPIO_MENZ127
> > +       tristate "MEN 16Z127 GPIO support"
> > +       depends on MCB
> 
> Is this "MCB" symbol already upstream? It seems a bit short.
>

Yes MCB is already upstream and there is alreay a driver upstream named 
men_Z135_uart. MCB is called "MEN Chameleon Bus" which is out FPGA.
The FPGA implements a table named "Chameleon Table" which describes the IPs
in the FPGA. 
 
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/io.h>
> > +#include <linux/mcb.h>
> > +#include <linux/gpio.h>
> 
> Just
> #include <linux/gpio/driver.h>
> 
OK, will change that.

> > +#define MEN_Z127_CTRL  0x00
> > +#define MEN_Z127_PSR   0x04
> > +#define MEN_Z127_IRQR  0x08
> > +#define MEN_Z127_GPIODR        0x0c
> > +#define MEN_Z127_IER1  0x10
> > +#define MEN_Z127_IER2  0x14
> 
> It looks like it has interrupt support?
> 
Yes there is an Interrupt support but not completly implemented in HW.
My IC designer is currently in holiday and he is planned to do the IRQ work
in a few weeks (month).
The plan was to start without IRQ support and get the driver upstream.

> > +#define MEN_Z127_DBER  0x18
> 
> Debounce? In that case, maybe implement .set_debounce from day 1?

Yes did not want to implement this from day 1, but i think you are right
I should do that, its not a complicated thing :-)
> 
> > +#define MEN_Z127_ODER  0x1C
> 
> And Open Drain? Maybe you should support that from day 1?
> 
Need to check if this is current implementation is working, but should be no 
problem to implement this.

> Yours,
> Linus Walleij

Thanks for your comments. I will send a v2 today or tomorrow.

Regards
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242605

Hi,
i have an additional question regarding the Open Drain setting.

The register is currently impelemented as a read/write register
which means the pin mode is configurable by software to
Push Pull or Open Drain.

There is also the possiblity (normal way) that the HW (FPGA) configures
each pin to the correct mode.

Is there actually a way to set an output mode from userland or by the gpio API?
I did not find anything about that.


If there is no way, i will implement it without software control. I just
read out the mode configuration and handle the pins as PP or Open Drain.

Regards
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249121

On Thu, Oct 8, 2015 at 6:55 PM, Andreas Werner <andy@wernerandy.de> wrote:

> The register is currently impelemented as a read/write register
> which means the pin mode is configurable by software to
> Push Pull or Open Drain.

OK then you can set it in accordance to what clients request.

> There is also the possiblity (normal way) that the HW (FPGA) configures
> each pin to the correct mode.
>
> Is there actually a way to set an output mode from userland or by the gpio API?

As per <linux/gpio/machine.h> you can specify per-line if it should be
open drain using GPIO_OPEN_DRAIN, this will push through to the
gpio descriptor and those lines with this flag set will be handled specially.
We have merged device tree bindings for OD/OS but not code to
handle these, but I might have some code from Laurent Pinchart cooking
for the DT in-kernel backend too.

> I did not find anything about that.

git grep OPEN_DRAIN

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1242705

Yet another thing.

if i will implement the open drain support, does it really makes sense to go with the
generic gpio lib?

I need to replace the direction_input and direction_outpu functions
because if the userland set an open drain pin to input i need to
set the pin to High-Z (driver "1") instead of setting the input bit
in the direction register.

So at the end there are just the "set" and "dat"
functions used to drive an output and to get an input state.
Alle other functions needs to be implemented in the driver.

Regards
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249120

On Thu, Oct 8, 2015 at 9:49 PM, Andreas Werner <andy@wernerandy.de> wrote:

> if i will implement the open drain support, does it really makes sense to go with the
> generic gpio lib?

Open Drain yes, for more esoterin pin config you may need to
also implement pin control, see Documentation/pinctrl.txt

> I need to replace the direction_input and direction_outpu functions
> because if the userland set an open drain pin to input i need to
> set the pin to High-Z (driver "1") instead of setting the input bit
> in the direction register.

Laurent has the same usecase I think, we need some fixes but
then we can detect (in the driver) that the gpio descriptor has
the open drain flag set in the .request() callback so you
can set it up there.

> So at the end there are just the "set" and "dat"
> functions used to drive an output and to get an input state.
> Alle other functions needs to be implemented in the driver.

I don't understand this question.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web