[PATCH 1/2] gpio: mockup: Allow probing from device tree

[PATCH 1/2] gpio: mockup: Allow probing from device tree

[Vincent Whitchurch]

Allow the mockup driver to be probed via the device tree, allowing it to
be used to configure and test higher level drivers like the leds-gpio
driver and corresponding userspace before actual hardware is available.

The mockup debugfs interface can be used to inject events, and actions
on the GPIO can be traced with ftrace.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/gpio/gpio-mockup.c | 87 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 63 insertions(+), 24 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index d66b7a768ecd..abe9059f116b 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/of.h>
 #include <linux/irq_sim.h>
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
@@ -255,27 +256,42 @@ static int gpio_mockup_name_lines(struct device *dev,
 
 static int gpio_mockup_probe(struct platform_device *pdev)
 {
-	struct gpio_mockup_platform_data *pdata;
+	struct device_node *node = pdev->dev.of_node;
+	struct gpio_mockup_platform_data *pdata = NULL;
 	struct gpio_mockup_chip *chip;
 	struct gpio_chip *gc;
 	int rv, base, ngpio;
 	struct device *dev;
-	char *name;
+	const char *name;
 
 	dev = &pdev->dev;
-	pdata = dev_get_platdata(dev);
-	base = pdata->base;
-	ngpio = pdata->ngpio;
+
+	if (node) {
+		u32 nr_gpios;
+		int ret;
+
+		ret = of_property_read_u32(node, "nr-gpios", &nr_gpios);
+		if (ret)
+			return ret;
+
+		base = -1;
+		ngpio = nr_gpios;
+		name = pdev->name;
+	} else {
+		pdata = dev_get_platdata(dev);
+		base = pdata->base;
+		ngpio = pdata->ngpio;
+
+		name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
+				      pdev->name, pdata->index);
+		if (!name)
+			return -ENOMEM;
+	}
 
 	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
-	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
-			      pdev->name, pdata->index);
-	if (!name)
-		return -ENOMEM;
-
 	gc = &chip->gc;
 	gc->base = base;
 	gc->ngpio = ngpio;
@@ -295,7 +311,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	if (!chip->lines)
 		return -ENOMEM;
 
-	if (pdata->named_lines) {
+	if (pdata && pdata->named_lines) {
 		rv = gpio_mockup_name_lines(dev, chip);
 		if (rv)
 			return rv;
@@ -315,9 +331,18 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_mockup_of_match[] = {
+	{ .compatible = "gpio-mockup", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gpio_mockup_of_match);
+#endif
+
 static struct platform_driver gpio_mockup_driver = {
 	.driver = {
 		.name = GPIO_MOCKUP_NAME,
+		.of_match_table = of_match_ptr(gpio_mockup_of_match),
 	},
 	.probe = gpio_mockup_probe,
 };
@@ -337,12 +362,15 @@ static void gpio_mockup_unregister_pdevs(void)
 	}
 }
 
-static int __init gpio_mockup_init(void)
+static int __init gpio_mockup_init_pdevs(void)
 {
-	int i, num_chips, err = 0, index = 'A';
+	int i, num_chips, index = 'A';
 	struct gpio_mockup_platform_data pdata;
 	struct platform_device *pdev;
 
+	if (IS_ENABLED(CONFIG_OF) && !gpio_mockup_num_ranges)
+		return 0;
+
 	if ((gpio_mockup_num_ranges < 2) ||
 	    (gpio_mockup_num_ranges % 2) ||
 	    (gpio_mockup_num_ranges > GPIO_MOCKUP_MAX_RANGES))
@@ -360,16 +388,6 @@ static int __init gpio_mockup_init(void)
 			return -EINVAL;
 	}
 
-	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
-	if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
-		gpio_mockup_err("error creating debugfs directory\n");
-
-	err = platform_driver_register(&gpio_mockup_driver);
-	if (err) {
-		gpio_mockup_err("error registering platform driver\n");
-		return err;
-	}
-
 	for (i = 0; i < num_chips; i++) {
 		pdata.index = index++;
 		pdata.base = gpio_mockup_range_base(i);
@@ -384,7 +402,6 @@ static int __init gpio_mockup_init(void)
 							 sizeof(pdata));
 		if (IS_ERR(pdev)) {
 			gpio_mockup_err("error registering device");
-			platform_driver_unregister(&gpio_mockup_driver);
 			gpio_mockup_unregister_pdevs();
 			return PTR_ERR(pdev);
 		}
@@ -395,6 +412,28 @@ static int __init gpio_mockup_init(void)
 	return 0;
 }
 
+static int __init gpio_mockup_init(void)
+{
+	int err;
+
+	err = gpio_mockup_init_pdevs();
+	if (err)
+		return err;
+
+	err = platform_driver_register(&gpio_mockup_driver);
+	if (err) {
+		gpio_mockup_err("error registering platform driver\n");
+		gpio_mockup_unregister_pdevs();
+		return err;
+	}
+
+	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
+	if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
+		gpio_mockup_err("error creating debugfs directory\n");
+
+	return 0;
+}
+
 static void __exit gpio_mockup_exit(void)
 {
 	debugfs_remove_recursive(gpio_mockup_dbg_dir);
-- 
2.11.0

[PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Vincent Whitchurch]

This is a dummy GPIO controller which can be used in place of the real
hardware GPIO controller (for example, an external GPIO expander) before
actual hardware is available.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 .../devicetree/bindings/gpio/gpio-mockup.txt       | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mockup.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mockup.txt b/Documentation/devicetree/bindings/gpio/gpio-mockup.txt
new file mode 100644
index 000000000000..585f04e356fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mockup.txt
@@ -0,0 +1,22 @@
+Mockup GPIO controller bindings
+
+This creates a dummy GPIO controller which can be used for testing higher level
+drivers or userspace before actual hardware is available.
+
+Required properties:
+- compatible: Should contain "gpio-mockup"
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be two.  The first cell is the pin number and
+  the second cell is used to specify the gpio polarity:
+      0 = active high
+      1 = active low
+- nr-gpios: The number of dummy GPIOs
+
+Example:
+
+	gpio-mockup0 {
+		compatible = "gpio-mockup";
+		nr-gpios = <32>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
-- 
2.11.0

Re: [PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Linus Walleij]

Hi Vincent, thanks for the patch!

Including Bartosz who is kind of default maintainer for this component.

On Wed, Sep 5, 2018 at 3:26 PM Vincent Whitchurch
<vincent.whitchurch@axis.com> wrote:
>
> This is a dummy GPIO controller which can be used in place of the real
> hardware GPIO controller (for example, an external GPIO expander) before
> actual hardware is available.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>

This looks generally helpful. I don't know what the DT maintainers
think, but I am positive, I think rapid prototyping is a valid usecase.
We used to have the driver for testing only but this usecase is
just as valid.

> +Required properties:
> +- compatible: Should contain "gpio-mockup"
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be two.  The first cell is the pin number and
> +  the second cell is used to specify the gpio polarity:
> +      0 = active high
> +      1 = active low
> +- nr-gpios: The number of dummy GPIOs

For this there is a standardized binding "ngpios" in gpio.txt
so just use that and reference gpio.txt please.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: mockup: Allow probing from device tree

[Linus Walleij]

Hi Vincent,

looping in Bartosz here too.

On Wed, Sep 5, 2018 at 3:26 PM Vincent Whitchurch
<vincent.whitchurch@axis.com> wrote:
>
> Allow the mockup driver to be probed via the device tree, allowing it to
> be used to configure and test higher level drivers like the leds-gpio
> driver and corresponding userspace before actual hardware is available.
>
> The mockup debugfs interface can be used to inject events, and actions
> on the GPIO can be traced with ftrace.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>

As stated on the bindings, this looks generally helpful.

The code looks fine, would be good if Bartosz could test the patch
with his test setup so we know we don't regress anything.

Nitpicky comments below.

> -       struct gpio_mockup_platform_data *pdata;
> +       struct device_node *node = pdev->dev.of_node;

I usually just call it "np" (node pointer).

> +       if (node) {
> +               u32 nr_gpios;
> +               int ret;
> +
> +               ret = of_property_read_u32(node, "nr-gpios", &nr_gpios);

This we change to ngpios per the standard bindings.

> +               if (ret)
> +                       return ret;
> +
> +               base = -1;
> +               ngpio = nr_gpios;
> +               name = pdev->name;

Maybe of_node_full_name(node) is more helpful? (I'm not sure.)

Yours,
Linus Walleij

Re: [PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Bartosz Golaszewski]

2018-09-06 12:16 GMT+02:00 Linus Walleij <linus.walleij@linaro.org>:
> Hi Vincent, thanks for the patch!
>
> Including Bartosz who is kind of default maintainer for this component.
>
> On Wed, Sep 5, 2018 at 3:26 PM Vincent Whitchurch
> <vincent.whitchurch@axis.com> wrote:
>>
>> This is a dummy GPIO controller which can be used in place of the real
>> hardware GPIO controller (for example, an external GPIO expander) before
>> actual hardware is available.
>>
>> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
>
> This looks generally helpful. I don't know what the DT maintainers
> think, but I am positive, I think rapid prototyping is a valid usecase.
> We used to have the driver for testing only but this usecase is
> just as valid.
>
>> +Required properties:
>> +- compatible: Should contain "gpio-mockup"
>> +- gpio-controller : Marks the device node as a gpio controller.
>> +- #gpio-cells : Should be two.  The first cell is the pin number and
>> +  the second cell is used to specify the gpio polarity:
>> +      0 = active high
>> +      1 = active low
>> +- nr-gpios: The number of dummy GPIOs
>
> For this there is a standardized binding "ngpios" in gpio.txt
> so just use that and reference gpio.txt please.
>
> Yours,
> Linus Walleij

Do we even need DT bindings for this? Device tree bindings mean that
we commit to a stable interface so that once a device with given DT
blob is released, it will work on every future kernel. Also: DT should
only include information about actual HW, not operating system
concepts. Meanwhile this is for testing purposes only and it won't end
up in any actual DT source file upstream.

I like the feature, just don't think this needs a binding document. I
will review the other patch later today.

Also: if you want to get a DT bindings patch merged, you need an Ack
from Rob Herring (Cc'ed now). You'll probably need to resend anyway as
patchwork won't pick up my Cc here.

Best regards,
Bartosz Golaszewski

Re: [PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Linus Walleij]

On Thu, Sep 6, 2018 at 1:09 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Do we even need DT bindings for this? Device tree bindings mean that
> we commit to a stable interface so that once a device with given DT
> blob is released, it will work on every future kernel. Also: DT should
> only include information about actual HW, not operating system
> concepts. Meanwhile this is for testing purposes only and it won't end
> up in any actual DT source file upstream.

Hm that relates to another discussion I have with the DT maintainers
about a virtualized display panel.

I do not know how the DT maintainers feel about supporting things
in the kernel that uses DT infrastructure and specially tailored
device trees but include elements with no formal device tree
bindings. Would be interesting to hear their thoughts on this.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: mockup: Allow probing from device tree

[Bartosz Golaszewski]

2018-09-05 15:26 GMT+02:00 Vincent Whitchurch <vincent.whitchurch@axis.com>:
> Allow the mockup driver to be probed via the device tree, allowing it to
> be used to configure and test higher level drivers like the leds-gpio
> driver and corresponding userspace before actual hardware is available.
>
> The mockup debugfs interface can be used to inject events, and actions
> on the GPIO can be traced with ftrace.
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
>  drivers/gpio/gpio-mockup.c | 87 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 63 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index d66b7a768ecd..abe9059f116b 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -15,6 +15,7 @@
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> +#include <linux/of.h>

Nit: how about we keep some sense of grouping for includes? Let's keep
irq-related files together and put of.h somewhere else.

>  #include <linux/irq_sim.h>
>  #include <linux/debugfs.h>
>  #include <linux/uaccess.h>
> @@ -255,27 +256,42 @@ static int gpio_mockup_name_lines(struct device *dev,
>
>  static int gpio_mockup_probe(struct platform_device *pdev)
>  {
> -       struct gpio_mockup_platform_data *pdata;
> +       struct device_node *node = pdev->dev.of_node;
> +       struct gpio_mockup_platform_data *pdata = NULL;
>         struct gpio_mockup_chip *chip;
>         struct gpio_chip *gc;
>         int rv, base, ngpio;
>         struct device *dev;
> -       char *name;
> +       const char *name;
>

Nit: please use the reverse xmas tree ordering for local variables.

>         dev = &pdev->dev;
> -       pdata = dev_get_platdata(dev);
> -       base = pdata->base;
> -       ngpio = pdata->ngpio;
> +
> +       if (node) {
> +               u32 nr_gpios;
> +               int ret;
> +
> +               ret = of_property_read_u32(node, "nr-gpios", &nr_gpios);

In addition to what Linus already said: I think this is a good moment
to get rid of the platform data structure and instead use device
properties, what do you think? This way the probe function would look
the same for both cases. I think it would be much cleaner.

> +               if (ret)
> +                       return ret;
> +
> +               base = -1;
> +               ngpio = nr_gpios;
> +               name = pdev->name;
> +       } else {
> +               pdata = dev_get_platdata(dev);
> +               base = pdata->base;
> +               ngpio = pdata->ngpio;
> +
> +               name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
> +                                     pdev->name, pdata->index);
> +               if (!name)
> +                       return -ENOMEM;
> +       }
>
>         chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
>         if (!chip)
>                 return -ENOMEM;
>
> -       name = devm_kasprintf(dev, GFP_KERNEL, "%s-%c",
> -                             pdev->name, pdata->index);
> -       if (!name)
> -               return -ENOMEM;
> -
>         gc = &chip->gc;
>         gc->base = base;
>         gc->ngpio = ngpio;
> @@ -295,7 +311,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
>         if (!chip->lines)
>                 return -ENOMEM;
>
> -       if (pdata->named_lines) {
> +       if (pdata && pdata->named_lines) {
>                 rv = gpio_mockup_name_lines(dev, chip);
>                 if (rv)
>                         return rv;
> @@ -315,9 +331,18 @@ static int gpio_mockup_probe(struct platform_device *pdev)
>         return 0;
>  }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id gpio_mockup_of_match[] = {
> +       { .compatible = "gpio-mockup", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, gpio_mockup_of_match);
> +#endif
> +
>  static struct platform_driver gpio_mockup_driver = {
>         .driver = {
>                 .name = GPIO_MOCKUP_NAME,
> +               .of_match_table = of_match_ptr(gpio_mockup_of_match),
>         },
>         .probe = gpio_mockup_probe,
>  };
> @@ -337,12 +362,15 @@ static void gpio_mockup_unregister_pdevs(void)
>         }
>  }
>
> -static int __init gpio_mockup_init(void)
> +static int __init gpio_mockup_init_pdevs(void)
>  {
> -       int i, num_chips, err = 0, index = 'A';
> +       int i, num_chips, index = 'A';
>         struct gpio_mockup_platform_data pdata;
>         struct platform_device *pdev;
>
> +       if (IS_ENABLED(CONFIG_OF) && !gpio_mockup_num_ranges)
> +               return 0;
> +
>         if ((gpio_mockup_num_ranges < 2) ||
>             (gpio_mockup_num_ranges % 2) ||
>             (gpio_mockup_num_ranges > GPIO_MOCKUP_MAX_RANGES))
> @@ -360,16 +388,6 @@ static int __init gpio_mockup_init(void)
>                         return -EINVAL;
>         }
>
> -       gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
> -       if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
> -               gpio_mockup_err("error creating debugfs directory\n");
> -
> -       err = platform_driver_register(&gpio_mockup_driver);
> -       if (err) {
> -               gpio_mockup_err("error registering platform driver\n");
> -               return err;
> -       }
> -
>         for (i = 0; i < num_chips; i++) {
>                 pdata.index = index++;
>                 pdata.base = gpio_mockup_range_base(i);
> @@ -384,7 +402,6 @@ static int __init gpio_mockup_init(void)
>                                                          sizeof(pdata));
>                 if (IS_ERR(pdev)) {
>                         gpio_mockup_err("error registering device");
> -                       platform_driver_unregister(&gpio_mockup_driver);
>                         gpio_mockup_unregister_pdevs();
>                         return PTR_ERR(pdev);
>                 }
> @@ -395,6 +412,28 @@ static int __init gpio_mockup_init(void)
>         return 0;
>  }
>
> +static int __init gpio_mockup_init(void)
> +{
> +       int err;
> +
> +       err = gpio_mockup_init_pdevs();
> +       if (err)
> +               return err;
> +
> +       err = platform_driver_register(&gpio_mockup_driver);
> +       if (err) {
> +               gpio_mockup_err("error registering platform driver\n");
> +               gpio_mockup_unregister_pdevs();
> +               return err;
> +       }
> +
> +       gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup-event", NULL);
> +       if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
> +               gpio_mockup_err("error creating debugfs directory\n");
> +
> +       return 0;
> +}
> +
>  static void __exit gpio_mockup_exit(void)
>  {
>         debugfs_remove_recursive(gpio_mockup_dbg_dir);
> --
> 2.11.0
>

Best regards,
Bartosz Golaszewski

Re: [PATCH 1/2] gpio: mockup: Allow probing from device tree

[Linus Walleij]

On Thu, Sep 6, 2018 at 6:29 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-09-05 15:26 GMT+02:00 Vincent Whitchurch <vincent.whitchurch@axis.com>:

> > +               ret = of_property_read_u32(node, "nr-gpios", &nr_gpios);
>
> In addition to what Linus already said: I think this is a good moment
> to get rid of the platform data structure and instead use device
> properties, what do you think?

If you're thinking of fwnode that will not work with static platform devices
(such as add with board files or other random platform_device_register() etc)
The fwnode concept only works with DT and ACPI.

Yours,
Linus Walleij

Re: [PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Rob Herring]

On Thu, Sep 06, 2018 at 02:47:23PM +0200, Linus Walleij wrote:
> On Thu, Sep 6, 2018 at 1:09 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 
> > Do we even need DT bindings for this? Device tree bindings mean that
> > we commit to a stable interface so that once a device with given DT
> > blob is released, it will work on every future kernel. Also: DT should
> > only include information about actual HW, not operating system
> > concepts. Meanwhile this is for testing purposes only and it won't end
> > up in any actual DT source file upstream.
> 
> Hm that relates to another discussion I have with the DT maintainers
> about a virtualized display panel.
> 
> I do not know how the DT maintainers feel about supporting things
> in the kernel that uses DT infrastructure and specially tailored
> device trees but include elements with no formal device tree
> bindings. Would be interesting to hear their thoughts on this.

I have one usecase in mind where I think it is valid, but for this case 
in particular, I think it would be better to just provide a sysfs 
interface. If there's no GPIO binding connections to this mockup 
controller, there's no real need for DT. OTOH, if this device allowed 
building a test framework for all the other GPIO based drivers and 
bindings such as LED, PWM, bitbanged buses, etc.

The case I have in mind is where you want to attach a fake DT node to a 
non-DT device for the purposes of defining other downstream devices. For 
example, an FTDI USB to serial chip with downstream SPI, UART, I2C, 
GPIO, etc. devices attached. This could be on an x86 which doesn't have 
a base DT.

Rob

Re: [PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Vincent Whitchurch]

On Sun, Sep 16, 2018 at 06:24:52PM -0500, Rob Herring wrote:
> On Thu, Sep 06, 2018 at 02:47:23PM +0200, Linus Walleij wrote:
> > On Thu, Sep 6, 2018 at 1:09 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > 
> > > Do we even need DT bindings for this? Device tree bindings mean that
> > > we commit to a stable interface so that once a device with given DT
> > > blob is released, it will work on every future kernel. Also: DT should
> > > only include information about actual HW, not operating system
> > > concepts. Meanwhile this is for testing purposes only and it won't end
> > > up in any actual DT source file upstream.
> > 
> > Hm that relates to another discussion I have with the DT maintainers
> > about a virtualized display panel.
> > 
> > I do not know how the DT maintainers feel about supporting things
> > in the kernel that uses DT infrastructure and specially tailored
> > device trees but include elements with no formal device tree
> > bindings. Would be interesting to hear their thoughts on this.
> 
> I have one usecase in mind where I think it is valid, but for this case 
> in particular, I think it would be better to just provide a sysfs 
> interface. If there's no GPIO binding connections to this mockup 
> controller, there's no real need for DT. OTOH, if this device allowed 
> building a test framework for all the other GPIO based drivers and 
> bindings such as LED, PWM, bitbanged buses, etc.

My original usecase for this was for leds-gpio.  I used the DT bindings
added by this patch and hooked up a leds-gpio via DT in order to develop
userspace.  Once the hardware with the GPIO expander arrived, the mockup
device in the DT was replaced with the expander.  I don't see how a
sysfs interface would allow the same thing.

Re: [PATCH 1/2] gpio: mockup: Allow probing from device tree

[Bartosz Golaszewski]

2018-09-10 9:05 GMT+02:00 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Sep 6, 2018 at 6:29 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 2018-09-05 15:26 GMT+02:00 Vincent Whitchurch <vincent.whitchurch@axis.com>:
>
>> > +               ret = of_property_read_u32(node, "nr-gpios", &nr_gpios);
>>
>> In addition to what Linus already said: I think this is a good moment
>> to get rid of the platform data structure and instead use device
>> properties, what do you think?
>
> If you're thinking of fwnode that will not work with static platform devices
> (such as add with board files or other random platform_device_register() etc)
> The fwnode concept only works with DT and ACPI.
>

I was thinking about using generic device properties: add property
entries with platform_device_add_properties() and read them with
device_property_read_*() functions. Is there any reason this wouldn't
work?

Bart

Re: [PATCH 1/2] gpio: mockup: Allow probing from device tree

[Linus Walleij]

On Thu, Sep 20, 2018 at 9:28 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-09-10 9:05 GMT+02:00 Linus Walleij <linus.walleij@linaro.org>:
> > On Thu, Sep 6, 2018 at 6:29 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >> 2018-09-05 15:26 GMT+02:00 Vincent Whitchurch <vincent.whitchurch@axis.com>:
> >
> >> > +               ret = of_property_read_u32(node, "nr-gpios", &nr_gpios);
> >>
> >> In addition to what Linus already said: I think this is a good moment
> >> to get rid of the platform data structure and instead use device
> >> properties, what do you think?
> >
> > If you're thinking of fwnode that will not work with static platform devices
> > (such as add with board files or other random platform_device_register() etc)
> > The fwnode concept only works with DT and ACPI.
>
> I was thinking about using generic device properties: add property
> entries with platform_device_add_properties() and read them with
> device_property_read_*() functions. Is there any reason this wouldn't
> work?

Oh I never used that. So I can't tell. It looks neat from a few
grep commands.

This occupies a space similar to fwnode in my perception so
I start to feel there are so many ways to do things now I get all
confused.

Yours,
Linus Walleij

Re: [PATCH 2/2] dt-bindings: gpio: Add binding for gpio-mockup

[Rob Herring]

On Tue, Sep 18, 2018 at 01:25:17PM +0200, Vincent Whitchurch wrote:
> On Sun, Sep 16, 2018 at 06:24:52PM -0500, Rob Herring wrote:
> > On Thu, Sep 06, 2018 at 02:47:23PM +0200, Linus Walleij wrote:
> > > On Thu, Sep 6, 2018 at 1:09 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > 
> > > > Do we even need DT bindings for this? Device tree bindings mean that
> > > > we commit to a stable interface so that once a device with given DT
> > > > blob is released, it will work on every future kernel. Also: DT should
> > > > only include information about actual HW, not operating system
> > > > concepts. Meanwhile this is for testing purposes only and it won't end
> > > > up in any actual DT source file upstream.
> > > 
> > > Hm that relates to another discussion I have with the DT maintainers
> > > about a virtualized display panel.
> > > 
> > > I do not know how the DT maintainers feel about supporting things
> > > in the kernel that uses DT infrastructure and specially tailored
> > > device trees but include elements with no formal device tree
> > > bindings. Would be interesting to hear their thoughts on this.
> > 
> > I have one usecase in mind where I think it is valid, but for this case 
> > in particular, I think it would be better to just provide a sysfs 
> > interface. If there's no GPIO binding connections to this mockup 
> > controller, there's no real need for DT. OTOH, if this device allowed 
> > building a test framework for all the other GPIO based drivers and 
> > bindings such as LED, PWM, bitbanged buses, etc.
> 
> My original usecase for this was for leds-gpio.  I used the DT bindings
> added by this patch and hooked up a leds-gpio via DT in order to develop
> userspace.  Once the hardware with the GPIO expander arrived, the mockup
> device in the DT was replaced with the expander.  I don't see how a
> sysfs interface would allow the same thing.

Okay, I'm fine with the binding. I'm also okay with not documenting this 
at all as this should never be in any stable or upstream dts file.

Rob