[RFC RESEND] GPIO: gpio-generic: Add DT support

[RFC RESEND] GPIO: gpio-generic: Add DT support

[Alexander Shiyan]

This patch adds DT support for generic (MMIO) GPIO driver.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/gpio/gpio-generic.txt      | 25 ++++++++++++++
 drivers/gpio/gpio-generic.c                        | 40 +++++++++++++++-------
 2 files changed, 53 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-generic.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-generic.txt b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
new file mode 100644
index 0000000..a9417ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
@@ -0,0 +1,25 @@
+Generic memory-mapped GPIO controller
+
+Required properties:
+- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be".
+- reg: Physical base GPIO controller registers location and length.
+- reg-names: Should be the names of reg resources. Each register uses
+  its own reg name, so there should be as many reg names as referenced
+  registers:
+  "dat"		: Input/output register (Required),
+  "set"		: Register for set output bits (Optional),
+  "clr"		: Register for clear output bits (Optional),
+  "dirout"	: Register for setup direction as output (Optional),
+  "dirin"	: Register for setup direction as input (Optional).
+- gpio-controller: Marks the device node as a gpio controller.
+- #gpio-cells: Should be two.
+
+Example (Simple 8-bit memory cell):
+
+bgpio: gpio@20000000 {
+	compatible = "basic-mmio-gpio";
+	reg = <0x20000000 0x1>;
+	reg-names = "dat";
+	gpio-controller;
+	#gpio-cells = <2>;
+};
diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index d2196bf..055c7f5 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -57,6 +57,7 @@ o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
 #include <linux/ioport.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
 #include <linux/mod_devicetable.h>
@@ -440,6 +441,26 @@ EXPORT_SYMBOL_GPL(bgpio_init);
 
 #ifdef CONFIG_GPIO_GENERIC_PLATFORM
 
+static const struct platform_device_id bgpio_id_table[] = {
+	{ .name = "basic-mmio-gpio",	.driver_data = 0, },
+	{ .name = "basic-mmio-gpio-be",	.driver_data = BGPIOF_BIG_ENDIAN, },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, bgpio_id_table);
+
+static const struct of_device_id bgpio_id_dt_table[] = {
+	{
+		.compatible	= "basic-mmio-gpio",
+		.data		= (const void *)0,
+	},
+	{
+		.compatible	= "basic-mmio-gpio-be",
+		.data		= (const void *)BGPIOF_BIG_ENDIAN,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bgpio_id_dt_table);
+
 static void __iomem *bgpio_map(struct platform_device *pdev,
 			       const char *name,
 			       resource_size_t sane_sz,
@@ -487,11 +508,12 @@ static int bgpio_pdev_probe(struct platform_device *pdev)
 	void __iomem *clr;
 	void __iomem *dirout;
 	void __iomem *dirin;
-	unsigned long sz;
-	unsigned long flags = 0;
+	unsigned long sz, flags;
 	int err;
 	struct bgpio_chip *bgc;
 	struct bgpio_pdata *pdata = dev_get_platdata(dev);
+	const struct of_device_id *of_id =
+		of_match_device(bgpio_id_dt_table, &pdev->dev);
 
 	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
 	if (!r)
@@ -519,8 +541,7 @@ static int bgpio_pdev_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	if (!strcmp(platform_get_device_id(pdev)->name, "basic-mmio-gpio-be"))
-		flags |= BGPIOF_BIG_ENDIAN;
+	flags = of_id ? (ulong)of_id->data : pdev->id_entry->driver_data;
 
 	bgc = devm_kzalloc(&pdev->dev, sizeof(*bgc), GFP_KERNEL);
 	if (!bgc)
@@ -548,16 +569,11 @@ static int bgpio_pdev_remove(struct platform_device *pdev)
 	return bgpio_remove(bgc);
 }
 
-static const struct platform_device_id bgpio_id_table[] = {
-	{ "basic-mmio-gpio", },
-	{ "basic-mmio-gpio-be", },
-	{},
-};
-MODULE_DEVICE_TABLE(platform, bgpio_id_table);
-
 static struct platform_driver bgpio_driver = {
 	.driver = {
-		.name = "basic-mmio-gpio",
+		.name		= "basic-mmio-gpio",
+		.owner		= THIS_MODULE,
+		.of_match_table	= bgpio_id_dt_table,
 	},
 	.id_table = bgpio_id_table,
 	.probe = bgpio_pdev_probe,
-- 
1.8.1.5

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Olof Johansson]

On Tue, Jul 30, 2013 at 03:18:35PM +0400, Alexander Shiyan wrote:
> This patch adds DT support for generic (MMIO) GPIO driver.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../devicetree/bindings/gpio/gpio-generic.txt      | 25 ++++++++++++++
>  drivers/gpio/gpio-generic.c                        | 40 +++++++++++++++-------
>  2 files changed, 53 insertions(+), 12 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-generic.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-generic.txt b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
> new file mode 100644
> index 0000000..a9417ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
> @@ -0,0 +1,25 @@
> +Generic memory-mapped GPIO controller
> +
> +Required properties:
> +- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be".
> +- reg: Physical base GPIO controller registers location and length.
> +- reg-names: Should be the names of reg resources. Each register uses
> +  its own reg name, so there should be as many reg names as referenced
> +  registers:
> +  "dat"		: Input/output register (Required),
> +  "set"		: Register for set output bits (Optional),
> +  "clr"		: Register for clear output bits (Optional),
> +  "dirout"	: Register for setup direction as output (Optional),
> +  "dirin"	: Register for setup direction as input (Optional).
> +- gpio-controller: Marks the device node as a gpio controller.
> +- #gpio-cells: Should be two.
> +
> +Example (Simple 8-bit memory cell):
> +
> +bgpio: gpio@20000000 {
> +	compatible = "basic-mmio-gpio";
> +	reg = <0x20000000 0x1>;
> +	reg-names = "dat";
> +	gpio-controller;
> +	#gpio-cells = <2>;
> +};

I'm trying to figure out what to say about this binding. It's not really
describing hardware, instead it's strongly tied to how the basic-mmio-gpio
driver expects the platform data to look.

It makes more sense to actually describe the hardware in question,
and then have the driver handle that as expected. I.e. either have a
small conversion layer that binds to the actual hardware compatible
value and registers a basic-mmio-gpio device from there, or extend the
basic-mmio-gpio driver to do it by itself.


-Olof

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Stephen Warren]

On 07/30/2013 10:22 AM, Olof Johansson wrote:
> On Tue, Jul 30, 2013 at 03:18:35PM +0400, Alexander Shiyan wrote:
>> This patch adds DT support for generic (MMIO) GPIO driver.

>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-generic.txt b/Documentation/devicetree/bindings/gpio/gpio-generic.txt

>> +Generic memory-mapped GPIO controller
>> +
>> +Required properties:
>> +- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be".

I think naming this "simple-gpio" would match other simple/basic/generic
bindings better.

>> +- reg: Physical base GPIO controller registers location and length.
>> +- reg-names: Should be the names of reg resources. Each register uses
>> +  its own reg name, so there should be as many reg names as referenced
>> +  registers:
>> +  "dat"		: Input/output register (Required),
>> +  "set"		: Register for set output bits (Optional),
>> +  "clr"		: Register for clear output bits (Optional),
>> +  "dirout"	: Register for setup direction as output (Optional),
>> +  "dirin"	: Register for setup direction as input (Optional).
>> +- gpio-controller: Marks the device node as a gpio controller.
>> +- #gpio-cells: Should be two.
...
> 
> I'm trying to figure out what to say about this binding. It's not really
> describing hardware, instead it's strongly tied to how the basic-mmio-gpio
> driver expects the platform data to look.

I'm not sure; the binding seems to only contain a direct representation
of pure HW properties; the addresses/offsets of various registers in the
HW block.

> It makes more sense to actually describe the hardware in question,
> and then have the driver handle that as expected. I.e. either have a
> small conversion layer that binds to the actual hardware compatible
> value and registers a basic-mmio-gpio device from there, or extend the
> basic-mmio-gpio driver to do it by itself.

Ah, I guess the question more: Do we want generic bindings that describe
the low-level details of the HW in a completely generic fashion so that
new HW can be supported with zero kernel changes, or do we want a simple
driver with a lookup table that maps from compatible value to the HW
configuration? One of the potential benefits of DT is to be able to
support new HW without code changes, although perhaps that's more
typically considered in the context of new boards rather than new IP
blocks or SoCs.

If we reject this driver, we surely have to get rid of pinctrl-single,
and perhaps some others?

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Mark Rutland]

On Tue, Jul 30, 2013 at 06:59:01PM +0100, Stephen Warren wrote:
> On 07/30/2013 10:22 AM, Olof Johansson wrote:
> > On Tue, Jul 30, 2013 at 03:18:35PM +0400, Alexander Shiyan wrote:
> >> This patch adds DT support for generic (MMIO) GPIO driver.
> 
> >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-generic.txt b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
> 
> >> +Generic memory-mapped GPIO controller
> >> +
> >> +Required properties:
> >> +- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be".
> 
> I think naming this "simple-gpio" would match other simple/basic/generic
> bindings better.
> 
> >> +- reg: Physical base GPIO controller registers location and length.
> >> +- reg-names: Should be the names of reg resources. Each register uses
> >> +  its own reg name, so there should be as many reg names as referenced
> >> +  registers:
> >> +  "dat"		: Input/output register (Required),
> >> +  "set"		: Register for set output bits (Optional),
> >> +  "clr"		: Register for clear output bits (Optional),
> >> +  "dirout"	: Register for setup direction as output (Optional),
> >> +  "dirin"	: Register for setup direction as input (Optional).
> >> +- gpio-controller: Marks the device node as a gpio controller.
> >> +- #gpio-cells: Should be two.
> ...
> > 
> > I'm trying to figure out what to say about this binding. It's not really
> > describing hardware, instead it's strongly tied to how the basic-mmio-gpio
> > driver expects the platform data to look.
> 
> I'm not sure; the binding seems to only contain a direct representation
> of pure HW properties; the addresses/offsets of various registers in the
> HW block.
> 
> > It makes more sense to actually describe the hardware in question,
> > and then have the driver handle that as expected. I.e. either have a
> > small conversion layer that binds to the actual hardware compatible
> > value and registers a basic-mmio-gpio device from there, or extend the
> > basic-mmio-gpio driver to do it by itself.
> 
> Ah, I guess the question more: Do we want generic bindings that describe
> the low-level details of the HW in a completely generic fashion so that
> new HW can be supported with zero kernel changes, or do we want a simple
> driver with a lookup table that maps from compatible value to the HW
> configuration? One of the potential benefits of DT is to be able to
> support new HW without code changes, although perhaps that's more
> typically considered in the context of new boards rather than new IP
> blocks or SoCs.

I think that going forward it would be better to have have a compatible
string per different device. As Olof pointed out, we're leaking the way
we currently handle things in Linux into the binding, rather than
precisely describing the hardware (with a unique compatible string). I'm
not sure if this is much better than embedding a bytecode describing how
to poke devices.

Certainly there should be more-specific bindings for each device, so we
can later improve support for them. If we have that anyway, I think it
would be nicer to have the mapping from that compatible string to some
internal data passed to the simple-gpio driver, rather than explicitly
stating that the current version of the Linux simple-gpio driver is
capable of driving the device.

I think the issue is that we're describing a hardware block in general,
rather than the instance of the hardware block, and that limits how
flexibly we can use the data in the description.

> 
> If we reject this driver, we surely have to get rid of pinctrl-single,
> and perhaps some others?
> 

That's certainly something we need to consider. However, those bindings
are in active use, and this is not yet. I don't think that we should
necessarily follow that style of binding.

Thanks,
Mark.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Alexander Shiyan]

On Wed, 31 Jul 2013 16:56:08 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> On Tue, Jul 30, 2013 at 06:59:01PM +0100, Stephen Warren wrote:
> > On 07/30/2013 10:22 AM, Olof Johansson wrote:
> > > On Tue, Jul 30, 2013 at 03:18:35PM +0400, Alexander Shiyan wrote:
> > >> This patch adds DT support for generic (MMIO) GPIO driver.
> > 
> > >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-generic.txt b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
> > 
> > >> +Generic memory-mapped GPIO controller
> > >> +
> > >> +Required properties:
> > >> +- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be".
> > 
> > I think naming this "simple-gpio" would match other simple/basic/generic
> > bindings better.
> > 
> > >> +- reg: Physical base GPIO controller registers location and length.
> > >> +- reg-names: Should be the names of reg resources. Each register uses
> > >> +  its own reg name, so there should be as many reg names as referenced
> > >> +  registers:
> > >> +  "dat"		: Input/output register (Required),
> > >> +  "set"		: Register for set output bits (Optional),
> > >> +  "clr"		: Register for clear output bits (Optional),
> > >> +  "dirout"	: Register for setup direction as output (Optional),
> > >> +  "dirin"	: Register for setup direction as input (Optional).
> > >> +- gpio-controller: Marks the device node as a gpio controller.
> > >> +- #gpio-cells: Should be two.
> > ...
> > > 
> > > I'm trying to figure out what to say about this binding. It's not really
> > > describing hardware, instead it's strongly tied to how the basic-mmio-gpio
> > > driver expects the platform data to look.
> > 
> > I'm not sure; the binding seems to only contain a direct representation
> > of pure HW properties; the addresses/offsets of various registers in the
> > HW block.
> > 
> > > It makes more sense to actually describe the hardware in question,
> > > and then have the driver handle that as expected. I.e. either have a
> > > small conversion layer that binds to the actual hardware compatible
> > > value and registers a basic-mmio-gpio device from there, or extend the
> > > basic-mmio-gpio driver to do it by itself.
> > 
> > Ah, I guess the question more: Do we want generic bindings that describe
> > the low-level details of the HW in a completely generic fashion so that
> > new HW can be supported with zero kernel changes, or do we want a simple
> > driver with a lookup table that maps from compatible value to the HW
> > configuration? One of the potential benefits of DT is to be able to
> > support new HW without code changes, although perhaps that's more
> > typically considered in the context of new boards rather than new IP
> > blocks or SoCs.
> 
> I think that going forward it would be better to have have a compatible
> string per different device. As Olof pointed out, we're leaking the way
> we currently handle things in Linux into the binding, rather than
> precisely describing the hardware (with a unique compatible string). I'm
> not sure if this is much better than embedding a bytecode describing how
> to poke devices.
> 
> Certainly there should be more-specific bindings for each device, so we
> can later improve support for them. If we have that anyway, I think it
> would be nicer to have the mapping from that compatible string to some
> internal data passed to the simple-gpio driver, rather than explicitly
> stating that the current version of the Linux simple-gpio driver is
> capable of driving the device.
> 
> I think the issue is that we're describing a hardware block in general,
> rather than the instance of the hardware block, and that limits how
> flexibly we can use the data in the description.

A lot of people - a lot of opinions.
So, what the final resolution for this?

-- 
Alexander Shiyan <shc_work@mail.ru>

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Pawel Moll]

On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
> > Ah, I guess the question more: Do we want generic bindings that describe
> > the low-level details of the HW in a completely generic fashion so that
> > new HW can be supported with zero kernel changes, or do we want a simple
> > driver with a lookup table that maps from compatible value to the HW
> > configuration? One of the potential benefits of DT is to be able to
> > support new HW without code changes, although perhaps that's more
> > typically considered in the context of new boards rather than new IP
> > blocks or SoCs.

... or FPGAs that can be synthesized with random collection of standard
IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
simpler and simpler...

> I think that going forward it would be better to have have a compatible
> string per different device. As Olof pointed out, we're leaking the way
> we currently handle things in Linux into the binding, rather than
> precisely describing the hardware (with a unique compatible string). I'm
> not sure if this is much better than embedding a bytecode describing how
> to poke devices.
> 
> Certainly there should be more-specific bindings for each device, so we
> can later improve support for them. If we have that anyway, I think it
> would be nicer to have the mapping from that compatible string to some
> internal data passed to the simple-gpio driver, rather than explicitly
> stating that the current version of the Linux simple-gpio driver is
> capable of driving the device.

This is one of the important decisions we may have to make going
forward... Do we only accept bindings for "real" blocks of IP? (and how
we define "real"?) If so, why does the "simple-bus"?

Frankly speaking I don't know where to draw the line, but I feel that in
this particular case - a "generic" GPIO binding - is worth the effort.
SOCs are literally littered with control registers driving random bits.
My favourite example - Versatile Express ;-) - have random registers
representing things like LEDs or MMC status lines. Depending on the
motherboard/FPGA version they can live in different places. And yes, I
can have a Versatile Express "platform" driver registering different set
of them depending on the particular variant of the FPGA bitfile. Or try
to represent them in the tree...

And yes, I've actually came with a patch almost identical to Alexander's
one. No, I won't feel depressed if it doesn't get in :-)

> I think the issue is that we're describing a hardware block in general,
> rather than the instance of the hardware block, and that limits how
> flexibly we can use the data in the description.

So if I went and designed a parametrized, synthesizeble, memory-mapped
GPIO "controller" matching the binding in question, would it change the
situation?

> > If we reject this driver, we surely have to get rid of pinctrl-single,
> > and perhaps some others?
>
> That's certainly something we need to consider. However, those bindings
> are in active use, and this is not yet. I don't think that we should
> necessarily follow that style of binding.

I think we should tell Mike Turquette about this, as his bindings for
basic clock components definitely fall into the same category:

http://thread.gmane.org/gmane.linux.kernel/1513049/

Pawel

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Mark Rutland]

On Tue, Aug 06, 2013 at 12:00:50PM +0100, Pawel Moll wrote:
> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
> > > Ah, I guess the question more: Do we want generic bindings that describe
> > > the low-level details of the HW in a completely generic fashion so that
> > > new HW can be supported with zero kernel changes, or do we want a simple
> > > driver with a lookup table that maps from compatible value to the HW
> > > configuration? One of the potential benefits of DT is to be able to
> > > support new HW without code changes, although perhaps that's more
> > > typically considered in the context of new boards rather than new IP
> > > blocks or SoCs.
> 
> ... or FPGAs that can be synthesized with random collection of standard
> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
> simpler and simpler...
> 
> > I think that going forward it would be better to have have a compatible
> > string per different device. As Olof pointed out, we're leaking the way
> > we currently handle things in Linux into the binding, rather than
> > precisely describing the hardware (with a unique compatible string). I'm
> > not sure if this is much better than embedding a bytecode describing how
> > to poke devices.
> > 
> > Certainly there should be more-specific bindings for each device, so we
> > can later improve support for them. If we have that anyway, I think it
> > would be nicer to have the mapping from that compatible string to some
> > internal data passed to the simple-gpio driver, rather than explicitly
> > stating that the current version of the Linux simple-gpio driver is
> > capable of driving the device.
> 
> This is one of the important decisions we may have to make going
> forward... Do we only accept bindings for "real" blocks of IP? (and how
> we define "real"?) If so, why does the "simple-bus"?

Agreed. I suspect this is going to be a bit messy.

I'd argue that "simple-bus" is at least special in that compatibility
implies nothing: It's more an annotation that the configuration logic
for some bus is ignorable, and can be used without being poked in any
way whatsoever. It's also useful for sharing blocks of components that
might be mapped in different areas in different dts using ranges, but
that's probably another point of contention ;)

> 
> Frankly speaking I don't know where to draw the line, but I feel that in
> this particular case - a "generic" GPIO binding - is worth the effort.
> SOCs are literally littered with control registers driving random bits.
> My favourite example - Versatile Express ;-) - have random registers
> representing things like LEDs or MMC status lines. Depending on the
> motherboard/FPGA version they can live in different places. And yes, I
> can have a Versatile Express "platform" driver registering different set
> of them depending on the particular variant of the FPGA bitfile. Or try
> to represent them in the tree...

I worry that going down that route encourages bindings to describe a
single way to use a given device, rather than describing the device
itself and allowing the OS to decide how to use it. This limits what we
can do in future, and I worry about how we can handle quirks sanely if
we describe devices in this way.

> 
> And yes, I've actually came with a patch almost identical to Alexander's
> one. No, I won't feel depressed if it doesn't get in :-)
> 
> > I think the issue is that we're describing a hardware block in general,
> > rather than the instance of the hardware block, and that limits how
> > flexibly we can use the data in the description.
> 
> So if I went and designed a parametrized, synthesizeble, memory-mapped
> GPIO "controller" matching the binding in question, would it change the
> situation?

It would certainly muddy the waters a bit.

> 
> > > If we reject this driver, we surely have to get rid of pinctrl-single,
> > > and perhaps some others?
> >
> > That's certainly something we need to consider. However, those bindings
> > are in active use, and this is not yet. I don't think that we should
> > necessarily follow that style of binding.
> 
> I think we should tell Mike Turquette about this, as his bindings for
> basic clock components definitely fall into the same category:
> 
> http://thread.gmane.org/gmane.linux.kernel/1513049/

Definitely.

It would be useful for the other maintainers to share their opinions
here. This is a rather important policy decision.

Thanks,
Mark.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Stephen Warren]

On 08/07/2013 08:07 AM, Mark Rutland wrote:
> On Tue, Aug 06, 2013 at 12:00:50PM +0100, Pawel Moll wrote:
>> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
>>>> Ah, I guess the question more: Do we want generic bindings that describe
>>>> the low-level details of the HW in a completely generic fashion so that
>>>> new HW can be supported with zero kernel changes, or do we want a simple
>>>> driver with a lookup table that maps from compatible value to the HW
>>>> configuration? One of the potential benefits of DT is to be able to
>>>> support new HW without code changes, although perhaps that's more
>>>> typically considered in the context of new boards rather than new IP
>>>> blocks or SoCs.
>>
>> ... or FPGAs that can be synthesized with random collection of standard
>> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
>> simpler and simpler...
>>
>>> I think that going forward it would be better to have have a compatible
>>> string per different device. As Olof pointed out, we're leaking the way
>>> we currently handle things in Linux into the binding, rather than
>>> precisely describing the hardware (with a unique compatible string). I'm
>>> not sure if this is much better than embedding a bytecode describing how
>>> to poke devices.

This really isn't leaking information about how Linux handles the
device. It's simply saying that there is a GPIO controller whose HW is
able to be described by a simple/generic binding, and that binding
provides full details of the register layout. Other OSs can handle this
differently; see below ...

...
>> Frankly speaking I don't know where to draw the line, but I feel that in
>> this particular case - a "generic" GPIO binding - is worth the effort.
>> SOCs are literally littered with control registers driving random bits.
>> My favourite example - Versatile Express ;-) - have random registers
>> representing things like LEDs or MMC status lines. Depending on the
>> motherboard/FPGA version they can live in different places. And yes, I
>> can have a Versatile Express "platform" driver registering different set
>> of them depending on the particular variant of the FPGA bitfile. Or try
>> to represent them in the tree...
> 
> I worry that going down that route encourages bindings to describe a
> single way to use a given device, rather than describing the device
> itself and allowing the OS to decide how to use it. This limits what we
> can do in future, and I worry about how we can handle quirks sanely if
> we describe devices in this way.

Well, each DT node that uses this binding must still have a compatible
property that fully defines the exact instance of the HW. In other
words, assuming this binding worked fine for Tegra, the DT must contain:

compatible = "nvidia,tegra20-gpio", "simple-gpio";

and not just:

compatible = "simple-gpio";

In that case, an OS could choose to match on "nvidia,tegra20-gpio" and
ignore most of the other properties to instantiate a more "custom"
driver, or to enable HW-specific quirks.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Mark Rutland]

On Wed, Aug 07, 2013 at 05:12:12PM +0100, Stephen Warren wrote:
> On 08/07/2013 08:07 AM, Mark Rutland wrote:
> > On Tue, Aug 06, 2013 at 12:00:50PM +0100, Pawel Moll wrote:
> >> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
> >>>> Ah, I guess the question more: Do we want generic bindings that describe
> >>>> the low-level details of the HW in a completely generic fashion so that
> >>>> new HW can be supported with zero kernel changes, or do we want a simple
> >>>> driver with a lookup table that maps from compatible value to the HW
> >>>> configuration? One of the potential benefits of DT is to be able to
> >>>> support new HW without code changes, although perhaps that's more
> >>>> typically considered in the context of new boards rather than new IP
> >>>> blocks or SoCs.
> >>
> >> ... or FPGAs that can be synthesized with random collection of standard
> >> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
> >> simpler and simpler...
> >>
> >>> I think that going forward it would be better to have have a compatible
> >>> string per different device. As Olof pointed out, we're leaking the way
> >>> we currently handle things in Linux into the binding, rather than
> >>> precisely describing the hardware (with a unique compatible string). I'm
> >>> not sure if this is much better than embedding a bytecode describing how
> >>> to poke devices.
> 
> This really isn't leaking information about how Linux handles the
> device. It's simply saying that there is a GPIO controller whose HW is
> able to be described by a simple/generic binding, and that binding
> provides full details of the register layout. Other OSs can handle this
> differently; see below ...

I worry that it doesn't provide a full description, but rather a
description of the subset of the hardware used by the driver.

> 
> ...
> >> Frankly speaking I don't know where to draw the line, but I feel that in
> >> this particular case - a "generic" GPIO binding - is worth the effort.
> >> SOCs are literally littered with control registers driving random bits.
> >> My favourite example - Versatile Express ;-) - have random registers
> >> representing things like LEDs or MMC status lines. Depending on the
> >> motherboard/FPGA version they can live in different places. And yes, I
> >> can have a Versatile Express "platform" driver registering different set
> >> of them depending on the particular variant of the FPGA bitfile. Or try
> >> to represent them in the tree...
> > 
> > I worry that going down that route encourages bindings to describe a
> > single way to use a given device, rather than describing the device
> > itself and allowing the OS to decide how to use it. This limits what we
> > can do in future, and I worry about how we can handle quirks sanely if
> > we describe devices in this way.
> 
> Well, each DT node that uses this binding must still have a compatible
> property that fully defines the exact instance of the HW. In other
> words, assuming this binding worked fine for Tegra, the DT must contain:
> 
> compatible = "nvidia,tegra20-gpio", "simple-gpio";
> 
> and not just:
> 
> compatible = "simple-gpio";
> 
> In that case, an OS could choose to match on "nvidia,tegra20-gpio" and
> ignore most of the other properties to instantiate a more "custom"
> driver, or to enable HW-specific quirks.

In that case, does the "nvidia,tegra20-gpio" require any extra reg
fields for registers not used by the "simple-gpio" binding? If peopel
are given a shortcut, I don't see why they'd bother to describe the
hardware they're not using.

What about the case where some mfd IP block can act as a gpio
controller, compatbile with simple-gpio, and also provides some other
functionality requiring a separate driver? I suspect people will
describe this as two devices, mirroring the Linux driver model, rather
than describing the hardware itself.

As I see it, a "simple-gpio" compatible string says "I can be driven by
the Linux simple-gpio driver", and the rest of the description is a
reflection of the structure of the simple-gpio driver rather than the
device.

Thanks,
Mark.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Stephen Warren]

On 08/08/2013 03:11 AM, Mark Rutland wrote:
> On Wed, Aug 07, 2013 at 05:12:12PM +0100, Stephen Warren wrote:
>> On 08/07/2013 08:07 AM, Mark Rutland wrote:
>>> On Tue, Aug 06, 2013 at 12:00:50PM +0100, Pawel Moll wrote:
>>>> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
>>>>>> Ah, I guess the question more: Do we want generic bindings that describe
>>>>>> the low-level details of the HW in a completely generic fashion so that
>>>>>> new HW can be supported with zero kernel changes, or do we want a simple
>>>>>> driver with a lookup table that maps from compatible value to the HW
>>>>>> configuration? One of the potential benefits of DT is to be able to
>>>>>> support new HW without code changes, although perhaps that's more
>>>>>> typically considered in the context of new boards rather than new IP
>>>>>> blocks or SoCs.
>>>>
>>>> ... or FPGAs that can be synthesized with random collection of standard
>>>> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
>>>> simpler and simpler...
>>>>
>>>>> I think that going forward it would be better to have have a compatible
>>>>> string per different device. As Olof pointed out, we're leaking the way
>>>>> we currently handle things in Linux into the binding, rather than
>>>>> precisely describing the hardware (with a unique compatible string). I'm
>>>>> not sure if this is much better than embedding a bytecode describing how
>>>>> to poke devices.
>>
>> This really isn't leaking information about how Linux handles the
>> device. It's simply saying that there is a GPIO controller whose HW is
>> able to be described by a simple/generic binding, and that binding
>> provides full details of the register layout. Other OSs can handle this
>> differently; see below ...
> 
> I worry that it doesn't provide a full description, but rather a
> description of the subset of the hardware used by the driver.

I don't see that as a problem.

A particular DT file provides a description of an interface to HW. To my
mind, if that particular DT doesn't describe everything about a
particular HW module (e.g. some advanced feature can't be enabled),
that's basically equivalent to not describing aspect of the board/system
(so e.g. some I2C device isn't represented at all, and hence some
temperature probe can't be monitored).

I think both the simple(!) simple-gpio interface to HW, and the more
complex tegra20-gpio interface are both equally valid interfaces; they
simply allow a different set of features to be accessed.

>> ...
>>>> Frankly speaking I don't know where to draw the line, but I feel that in
>>>> this particular case - a "generic" GPIO binding - is worth the effort.
>>>> SOCs are literally littered with control registers driving random bits.
>>>> My favourite example - Versatile Express ;-) - have random registers
>>>> representing things like LEDs or MMC status lines. Depending on the
>>>> motherboard/FPGA version they can live in different places. And yes, I
>>>> can have a Versatile Express "platform" driver registering different set
>>>> of them depending on the particular variant of the FPGA bitfile. Or try
>>>> to represent them in the tree...
>>>
>>> I worry that going down that route encourages bindings to describe a
>>> single way to use a given device, rather than describing the device
>>> itself and allowing the OS to decide how to use it. This limits what we
>>> can do in future, and I worry about how we can handle quirks sanely if
>>> we describe devices in this way.
>>
>> Well, each DT node that uses this binding must still have a compatible
>> property that fully defines the exact instance of the HW. In other
>> words, assuming this binding worked fine for Tegra, the DT must contain:
>>
>> compatible = "nvidia,tegra20-gpio", "simple-gpio";
>>
>> and not just:
>>
>> compatible = "simple-gpio";
>>
>> In that case, an OS could choose to match on "nvidia,tegra20-gpio" and
>> ignore most of the other properties to instantiate a more "custom"
>> driver, or to enable HW-specific quirks.
> 
> In that case, does the "nvidia,tegra20-gpio" require any extra reg
> fields for registers not used by the "simple-gpio" binding? If peopel
> are given a shortcut, I don't see why they'd bother to describe the
> hardware they're not using.

Do you mean entries in the reg field itself, or the various
dat/set/clr/... properties that describe the register layout?

I would expect the reg property for a DT node to be entirely complete in
all cases, even if writing a simple-gpio node when the HW could instead
be later described as a tegra20-gpio node. In other words, if the GPIO
HW block has 2 sets of disjoint registers, then the DT should include
those both in reg even if features accessible through the simple-gpio
view don't need the second bank.

Re: dat/set/clr/... - yes, I imagine the whole point of later switching
to a more specific binding would be to enable more features, which would
likely in turn require the use of more registers. However, I would
expect the tegra20-gpio binding to embody the specific set of registers
that are present in HW. In other words, the simple-gpio binding would
require dat/set/clr/... properties and drivers would solely use those to
know how to access the various registers. However, use of the more
specific tegra20-gpio interface would entail the user having pre-defined
explicit knowledge of the register layout, and hence it would entirely
ignore dat/set/clr/... properties, and use its own built-in layout
knowledge.

> What about the case where some mfd IP block can act as a gpio
> controller, compatible with simple-gpio, and also provides some other
> functionality requiring a separate driver? I suspect people will
> describe this as two devices, mirroring the Linux driver model, rather
> than describing the hardware itself.

Are the GPIO registers truly an entirely separate HW block? If so,
simple-gpio v.s. something else shouldn't matter.

If the GPIO registers are co-mingled with other features, then I think
that it'd be legal for the DT to contain either:

a)

Just a simple-gpio block. The other features would not be available.

b)

A more precise compatible value, thus allowing SW to expose all the
features.

The one issue here is that perhaps we could never "re-purpose" a plain
simple-gpio node simply by binding a different driver to it; all the
other properties could be missing. As such, perhaps when using
simple-gpio we shouldn't actually include the more precise entry in the
compatible property.

The possible solutions would be:

a) Very carefully craft every GPIO-related binding so that all
properties beyond what simple-gpio provides were optional, and hence a
pure simple-gpio node would work fine with the more complex driver. This
could be difficult.

b) Make sure that the DT already contained the union of the properties
required by simple-gpio and the more complex binding from the start. In
this case, there might not be much point doing simple-gpio; just use the
more advanced binding from the start.

I guess this boils down to: When an old DT is used with a new kernel, is
the minimum expectation that all previous functionality will still
operate correctly, *or* that no DT changes are required to enable any
new functionality that the new kernel could provide on that hardware?

I'd tend to lean towards new kernels maintaining at least old
functionality, rather than requiring that no DT changes are required to
enable new functionality.

> As I see it, a "simple-gpio" compatible string says "I can be driven by
> the Linux simple-gpio driver", and the rest of the description is a
> reflection of the structure of the simple-gpio driver rather than the
> device.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Grant Likely]

On Thu, 08 Aug 2013 12:34:28 -0600, Stephen Warren <swarren@wwwdotorg.org> wrote:
> I'd tend to lean towards new kernels maintaining at least old
> functionality, rather than requiring that no DT changes are required to
> enable new functionality.

+1

g.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Mark Rutland]

On Thu, Aug 08, 2013 at 07:34:28PM +0100, Stephen Warren wrote:
> On 08/08/2013 03:11 AM, Mark Rutland wrote:
> > On Wed, Aug 07, 2013 at 05:12:12PM +0100, Stephen Warren wrote:
> >> On 08/07/2013 08:07 AM, Mark Rutland wrote:
> >>> On Tue, Aug 06, 2013 at 12:00:50PM +0100, Pawel Moll wrote:
> >>>> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
> >>>>>> Ah, I guess the question more: Do we want generic bindings that describe
> >>>>>> the low-level details of the HW in a completely generic fashion so that
> >>>>>> new HW can be supported with zero kernel changes, or do we want a simple
> >>>>>> driver with a lookup table that maps from compatible value to the HW
> >>>>>> configuration? One of the potential benefits of DT is to be able to
> >>>>>> support new HW without code changes, although perhaps that's more
> >>>>>> typically considered in the context of new boards rather than new IP
> >>>>>> blocks or SoCs.
> >>>>
> >>>> ... or FPGAs that can be synthesized with random collection of standard
> >>>> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
> >>>> simpler and simpler...
> >>>>
> >>>>> I think that going forward it would be better to have have a compatible
> >>>>> string per different device. As Olof pointed out, we're leaking the way
> >>>>> we currently handle things in Linux into the binding, rather than
> >>>>> precisely describing the hardware (with a unique compatible string). I'm
> >>>>> not sure if this is much better than embedding a bytecode describing how
> >>>>> to poke devices.
> >>
> >> This really isn't leaking information about how Linux handles the
> >> device. It's simply saying that there is a GPIO controller whose HW is
> >> able to be described by a simple/generic binding, and that binding
> >> provides full details of the register layout. Other OSs can handle this
> >> differently; see below ...
> > 
> > I worry that it doesn't provide a full description, but rather a
> > description of the subset of the hardware used by the driver.
> 
> I don't see that as a problem.
> 
> A particular DT file provides a description of an interface to HW. To my
> mind, if that particular DT doesn't describe everything about a
> particular HW module (e.g. some advanced feature can't be enabled),
> that's basically equivalent to not describing aspect of the board/system
> (so e.g. some I2C device isn't represented at all, and hence some
> temperature probe can't be monitored).

My concern is simply that if people can get basic functionality with the
generic driver they won't bother describing the details of the more
specific binding, and you'll never be able to use the board with a
better driver without having to modify the dt later.

I guess the question boils down to how much we care about that
situation.

> 
> I think both the simple(!) simple-gpio interface to HW, and the more
> complex tegra20-gpio interface are both equally valid interfaces; they
> simply allow a different set of features to be accessed.

They're certianly both valid. I fear they may become mutually exclusive
(people only define one or the other, and the bindings drift and become
incompatible).

> 
> >> ...
> >>>> Frankly speaking I don't know where to draw the line, but I feel that in
> >>>> this particular case - a "generic" GPIO binding - is worth the effort.
> >>>> SOCs are literally littered with control registers driving random bits.
> >>>> My favourite example - Versatile Express ;-) - have random registers
> >>>> representing things like LEDs or MMC status lines. Depending on the
> >>>> motherboard/FPGA version they can live in different places. And yes, I
> >>>> can have a Versatile Express "platform" driver registering different set
> >>>> of them depending on the particular variant of the FPGA bitfile. Or try
> >>>> to represent them in the tree...
> >>>
> >>> I worry that going down that route encourages bindings to describe a
> >>> single way to use a given device, rather than describing the device
> >>> itself and allowing the OS to decide how to use it. This limits what we
> >>> can do in future, and I worry about how we can handle quirks sanely if
> >>> we describe devices in this way.
> >>
> >> Well, each DT node that uses this binding must still have a compatible
> >> property that fully defines the exact instance of the HW. In other
> >> words, assuming this binding worked fine for Tegra, the DT must contain:
> >>
> >> compatible = "nvidia,tegra20-gpio", "simple-gpio";
> >>
> >> and not just:
> >>
> >> compatible = "simple-gpio";
> >>
> >> In that case, an OS could choose to match on "nvidia,tegra20-gpio" and
> >> ignore most of the other properties to instantiate a more "custom"
> >> driver, or to enable HW-specific quirks.
> > 
> > In that case, does the "nvidia,tegra20-gpio" require any extra reg
> > fields for registers not used by the "simple-gpio" binding? If peopel
> > are given a shortcut, I don't see why they'd bother to describe the
> > hardware they're not using.
> 
> Do you mean entries in the reg field itself, or the various
> dat/set/clr/... properties that describe the register layout?

I mean entries. If people describe the "dat", "set," "clr" registers
from the simple-gpio binding, they may also need some
"nvidia,tegra20-gpio"-specific reg entry describing the whole register
set (which will likely overlap). Describing things twice seems odd to
me.

> 
> I would expect the reg property for a DT node to be entirely complete in
> all cases, even if writing a simple-gpio node when the HW could instead
> be later described as a tegra20-gpio node. In other words, if the GPIO
> HW block has 2 sets of disjoint registers, then the DT should include
> those both in reg even if features accessible through the simple-gpio
> view don't need the second bank.

If that's the case, we need to ensure that the bindings for any more
specific binding compatible with simple-gpio is a strict superset of the
simple-gpio binding (and this needs to be clear from the start in the
binding document for simple-gpio). Presumably, any more specific binding
*must* have one or more reg-names entries for its more useful data.

> 
> Re: dat/set/clr/... - yes, I imagine the whole point of later switching
> to a more specific binding would be to enable more features, which would
> likely in turn require the use of more registers. However, I would
> expect the tegra20-gpio binding to embody the specific set of registers
> that are present in HW. In other words, the simple-gpio binding would
> require dat/set/clr/... properties and drivers would solely use those to
> know how to access the various registers. However, use of the more
> specific tegra20-gpio interface would entail the user having pre-defined
> explicit knowledge of the register layout, and hence it would entirely
> ignore dat/set/clr/... properties, and use its own built-in layout
> knowledge.

Ok, that sounds like what I was getting at above. I'd want a push-back
on simple-gpio usage without a more-specific binding also being
documented.

> 
> > What about the case where some mfd IP block can act as a gpio
> > controller, compatible with simple-gpio, and also provides some other
> > functionality requiring a separate driver? I suspect people will
> > describe this as two devices, mirroring the Linux driver model, rather
> > than describing the hardware itself.
> 
> Are the GPIO registers truly an entirely separate HW block? If so,
> simple-gpio v.s. something else shouldn't matter.
> 
> If the GPIO registers are co-mingled with other features, then I think
> that it'd be legal for the DT to contain either:
> 
> a)
> 
> Just a simple-gpio block. The other features would not be available.
> 
> b)
> 
> A more precise compatible value, thus allowing SW to expose all the
> features.
> 
> The one issue here is that perhaps we could never "re-purpose" a plain
> simple-gpio node simply by binding a different driver to it; all the
> other properties could be missing. As such, perhaps when using
> simple-gpio we shouldn't actually include the more precise entry in the
> compatible property.
> 
> The possible solutions would be:
> 
> a) Very carefully craft every GPIO-related binding so that all
> properties beyond what simple-gpio provides were optional, and hence a
> pure simple-gpio node would work fine with the more complex driver. This
> could be difficult.
> 
> b) Make sure that the DT already contained the union of the properties
> required by simple-gpio and the more complex binding from the start. In
> this case, there might not be much point doing simple-gpio; just use the
> more advanced binding from the start.

That would be my preference. That also caters for the case of a device
that happens to be usable with more than one generic binding
simultaneously.

> 
> I guess this boils down to: When an old DT is used with a new kernel, is
> the minimum expectation that all previous functionality will still
> operate correctly, *or* that no DT changes are required to enable any
> new functionality that the new kernel could provide on that hardware?
> 
> I'd tend to lean towards new kernels maintaining at least old
> functionality, rather than requiring that no DT changes are required to
> enable new functionality.

That certainly makes sense. I'd also like to aim for new dts maintaining
the old functionality on old kernels, or we're still stuck with a
dtb<->kernel version requirement.

Thanks,
Mark.

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Stephen Warren]

On 08/09/2013 03:09 AM, Mark Rutland wrote:
> On Thu, Aug 08, 2013 at 07:34:28PM +0100, Stephen Warren wrote:
>> On 08/08/2013 03:11 AM, Mark Rutland wrote:
>>> On Wed, Aug 07, 2013 at 05:12:12PM +0100, Stephen Warren wrote:
>>>> On 08/07/2013 08:07 AM, Mark Rutland wrote:
>>>>> On Tue, Aug 06, 2013 at 12:00:50PM +0100, Pawel Moll wrote:
>>>>>> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
>>>>>>>> Ah, I guess the question more: Do we want generic bindings that describe
>>>>>>>> the low-level details of the HW in a completely generic fashion so that
>>>>>>>> new HW can be supported with zero kernel changes, or do we want a simple
>>>>>>>> driver with a lookup table that maps from compatible value to the HW
>>>>>>>> configuration? One of the potential benefits of DT is to be able to
>>>>>>>> support new HW without code changes, although perhaps that's more
>>>>>>>> typically considered in the context of new boards rather than new IP
>>>>>>>> blocks or SoCs.
>>>>>>
>>>>>> ... or FPGAs that can be synthesized with random collection of standard
>>>>>> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
>>>>>> simpler and simpler...
>>>>>>
>>>>>>> I think that going forward it would be better to have have a compatible
>>>>>>> string per different device. As Olof pointed out, we're leaking the way
>>>>>>> we currently handle things in Linux into the binding, rather than
>>>>>>> precisely describing the hardware (with a unique compatible string). I'm
>>>>>>> not sure if this is much better than embedding a bytecode describing how
>>>>>>> to poke devices.
>>>>
>>>> This really isn't leaking information about how Linux handles the
>>>> device. It's simply saying that there is a GPIO controller whose HW is
>>>> able to be described by a simple/generic binding, and that binding
>>>> provides full details of the register layout. Other OSs can handle this
>>>> differently; see below ...
>>>
>>> I worry that it doesn't provide a full description, but rather a
>>> description of the subset of the hardware used by the driver.
>>
>> I don't see that as a problem.
>>
>> A particular DT file provides a description of an interface to HW. To my
>> mind, if that particular DT doesn't describe everything about a
>> particular HW module (e.g. some advanced feature can't be enabled),
>> that's basically equivalent to not describing aspect of the board/system
>> (so e.g. some I2C device isn't represented at all, and hence some
>> temperature probe can't be monitored).
> 
> My concern is simply that if people can get basic functionality with the
> generic driver they won't bother describing the details of the more
> specific binding, and you'll never be able to use the board with a
> better driver without having to modify the dt later.
> 
> I guess the question boils down to how much we care about that
> situation.

I personally think it's fine. If people only want to expose the basic
functionality and can do so with simple-gpio, I don't see any problem at
all with that. (Why should we care that a binding doesn't expose all
features of the device? I'm sure there are many undocumented features,
e.g. debug/backdoor, in HW that we don't even know exist and so don't
know aren't exposed by various bindings)

If someone later finds it useful to expose more functionality than is
sensible to expose through simple-gpio, then a HW-specific binding can
be added for that purpose, and the .dts file amended to use that.

The more I think about this, the more I think that simply having
disjoint simple and HW-specific bindings makes sense.

In that case, the new DT won't be compatible with an old kernel, but I
think that's reasonable, since it was explicitly changed to implement
new features.

I think we should resolve this aspect first before considering the
details. I'm curious what other DT maintainers and users think about
this topic?

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Alexander Shiyan]

> >>>>>> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
> >>>>>>>> Ah, I guess the question more: Do we want generic bindings that describe
> >>>>>>>> the low-level details of the HW in a completely generic fashion so that
> >>>>>>>> new HW can be supported with zero kernel changes, or do we want a simple
> >>>>>>>> driver with a lookup table that maps from compatible value to the HW
> >>>>>>>> configuration? One of the potential benefits of DT is to be able to
> >>>>>>>> support new HW without code changes, although perhaps that's more
> >>>>>>>> typically considered in the context of new boards rather than new IP
> >>>>>>>> blocks or SoCs.
> >>>>>>
> >>>>>> ... or FPGAs that can be synthesized with random collection of standard
> >>>>>> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
> >>>>>> simpler and simpler...
> >>>>>>
> >>>>>>> I think that going forward it would be better to have have a compatible
> >>>>>>> string per different device. As Olof pointed out, we're leaking the way
> >>>>>>> we currently handle things in Linux into the binding, rather than
> >>>>>>> precisely describing the hardware (with a unique compatible string). I'm
> >>>>>>> not sure if this is much better than embedding a bytecode describing how
> >>>>>>> to poke devices.
> >>>>
> >>>> This really isn't leaking information about how Linux handles the
> >>>> device. It's simply saying that there is a GPIO controller whose HW is
> >>>> able to be described by a simple/generic binding, and that binding
> >>>> provides full details of the register layout. Other OSs can handle this
> >>>> differently; see below ...
> >>>
> >>> I worry that it doesn't provide a full description, but rather a
> >>> description of the subset of the hardware used by the driver.
> >>
> >> I don't see that as a problem.
> >>
> >> A particular DT file provides a description of an interface to HW. To my
> >> mind, if that particular DT doesn't describe everything about a
> >> particular HW module (e.g. some advanced feature can't be enabled),
> >> that's basically equivalent to not describing aspect of the board/system
> >> (so e.g. some I2C device isn't represented at all, and hence some
> >> temperature probe can't be monitored).
> > 
> > My concern is simply that if people can get basic functionality with the
> > generic driver they won't bother describing the details of the more
> > specific binding, and you'll never be able to use the board with a
> > better driver without having to modify the dt later.
> > 
> > I guess the question boils down to how much we care about that
> > situation.
> 
> I personally think it's fine. If people only want to expose the basic
> functionality and can do so with simple-gpio, I don't see any problem at
> all with that. (Why should we care that a binding doesn't expose all
> features of the device? I'm sure there are many undocumented features,
> e.g. debug/backdoor, in HW that we don't even know exist and so don't
> know aren't exposed by various bindings)
> 
> If someone later finds it useful to expose more functionality than is
> sensible to expose through simple-gpio, then a HW-specific binding can
> be added for that purpose, and the .dts file amended to use that.
> 
> The more I think about this, the more I think that simply having
> disjoint simple and HW-specific bindings makes sense.
> 
> In that case, the new DT won't be compatible with an old kernel, but I
> think that's reasonable, since it was explicitly changed to implement
> new features.
> 
> I think we should resolve this aspect first before considering the
> details. I'm curious what other DT maintainers and users think about
> this topic?

I want to add from myself that the ability to use a simple-gpio interface
makes things very simple :) As an example, I cite one of the architectures
on which I work. ARM CLPS711X. I can freely use a simple-gpio driver instead
of the special driver, since the architecture does not have any other capacity
for the pins (interrupts, etc.).
But mostly, I intended to use this driver for such things like PLD, 74HC24x,
74HC57x, etc. on the bus.
---

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Linus Walleij]

Here are my €0.01 for this discussion...

On Tue, Aug 6, 2013 at 1:00 PM, Pawel Moll <pawel.moll@arm.com> wrote:
> On Wed, 2013-07-31 at 16:56 +0100, Mark Rutland wrote:
>> > Ah, I guess the question more: Do we want generic bindings that describe
>> > the low-level details of the HW in a completely generic fashion so that
>> > new HW can be supported with zero kernel changes, or do we want a simple
>> > driver with a lookup table that maps from compatible value to the HW
>> > configuration? One of the potential benefits of DT is to be able to
>> > support new HW without code changes, although perhaps that's more
>> > typically considered in the context of new boards rather than new IP
>> > blocks or SoCs.
>
> ... or FPGAs that can be synthesized with random collection of standard
> IP blocks. With Xilinx's Zynq and Altera's SOCFPGA this is getting
> simpler and simpler...

I think the stance should be similar as with pinctrl-single:
we can have such a driver if and only if the hardware:

- Does not really have a fixes, etched in stone, datasheet.

- Flixes and flexes and reconfigures at the wave of a HW
  engineering wand

- Instead of datasheets the HW engineers provide some
  magic markup that you need to process to figure out how
  to hit the bits correctly.

> This is one of the important decisions we may have to make going
> forward... Do we only accept bindings for "real" blocks of IP? (and how
> we define "real"?) If so, why does the "simple-bus"?

Yeah how do you define "real" ... I think we all hearda movie quote
righ there. With the advent of virtual prototyping
in fast models of SoCs and FPGAs, we're not really dealing with real
silicon.

Plato in ~400 BC regarded our ideas as the highest form of reality
as illustrated in the famous allegory of the cave:
http://en.wikipedia.org/wiki/Allegory_of_the_Cave
(This is called platonism and found in Gnosticism and popular
culture references all over the place.)

With a simple DT representation we represent the idea of
a certain simple GPIO controller, whereas in reality it will have
flaws, and those will be discovered when adding ever more
usecases and power management and other things that hit
the deep silicon structures of the projection of this idea into
silicon reality.

So I think this thing should be implemented, but the next step
is to be guarded - when people want to add "quirks" to this
driver it cannot be called "simple" anymore, and a real driver
with full knowledge of the hardware need to be created in its
place at this point, so we should require that any DTS(I)
using this "simple" driver *also* define a unique compatible
string for the platform, such that a newer kernel may instantiate
a more HW-specific driver for this one GPIO and disregard any
older properties for the "simple" controller.

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

Re: [RFC RESEND] GPIO: gpio-generic: Add DT support

[Linus Walleij]

On Tue, Jul 30, 2013 at 1:18 PM, Alexander Shiyan <shc_work@mail.ru> wrote:

> +++ b/Documentation/devicetree/bindings/gpio/gpio-generic.txt
> @@ -0,0 +1,25 @@
> +Generic memory-mapped GPIO controller
> +
> +Required properties:
> +- compatible: Should be "basic-mmio-gpio" or "basic-mmio-gpio-be".
> +- reg: Physical base GPIO controller registers location and length.
> +- reg-names: Should be the names of reg resources. Each register uses
> +  its own reg name, so there should be as many reg names as referenced
> +  registers:
> +  "dat"                : Input/output register (Required),
> +  "set"                : Register for set output bits (Optional),
> +  "clr"                : Register for clear output bits (Optional),
> +  "dirout"     : Register for setup direction as output (Optional),
> +  "dirin"      : Register for setup direction as input (Optional).
> +- gpio-controller: Marks the device node as a gpio controller.
> +- #gpio-cells: Should be two.

This appears to make a *lot* of implicit assumptions about these
registers. You absolutely *have* to define every assumption in clear
text for this.

Example of implicit assumptions:

- Registers in "basic-mmio-gpio" are these assumed to be
  little endian? Or target-CPU-endian?

- Does "-be" mean that the CPU or the bus is big endian?
  (I assume the bus...)

- Is is assumed that 0 in a dat bit is low line and 1is a high line?

- Is it assumed that setting a bit to 1 in set drives the line high and
  setting it to 1 in clr clears the line?

- Same question for dirout/dirin?

- What happens if you set the same bit in both dirin and dirout?

- Is it assumed that the first GPIO line is bit 0, second GPIO line
  is bit 1 (etc) up to bit ....N?

- How many bits are there in a register after all? 8? 16? 32? 64?
  Or the bus size of the architecture maybe?

This really need some very details definition before it can be
considered for merging.

Yours,
Linus Walleij