On Tue, Oct 19, 2021 at 4:00 PM Mauri Sandberg [off-list ref] wrote:

Adds support for building cascades of GPIO lines. That is, it allows
setups when there is one upstream line and multiple cascaded lines, out
of which one can be chosen at a time. The status of the upstream line
can be conveyed to the selected cascaded line or, vice versa, the status
of the cascaded line can be conveyed to the upstream line.

A multiplexer is being used to select, which cascaded GPIO line is being
used at any given time.

At the moment only input direction is supported. In future it should be
possible to add support for output direction, too.

Thanks for an update! My comments below.

...

+config GPIO_CASCADE
+       tristate "General GPIO cascade"
+       select MULTIPLEXER
+       help
+         Say yes here to enable support for generic GPIO cascade.
+
+         This allows building one-to-many cascades of GPIO lines using
+         different types of multiplexers readily available. At the
+         moment only input lines are supported.

Care to mention what will be the module name in the case of being
built as a module?
(Hint: there are plenty of existing examples in the kernel)

...

+#include <linux/module.h>

+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>

I would move this group...

+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mux/consumer.h>
+

...to be somewhere here to explicitly show that this is the GPIO
subsystem related driver.

...

+       mc = devm_mux_control_get(dev, NULL);
+       if (IS_ERR(mc))

+               return dev_err_probe(dev,
+                                    PTR_ERR(mc),
+                                    "unable to get mux-control\n");

Why not one line?

...

+       upstream = devm_gpiod_get(dev, "upstream",  GPIOD_IN);
+       if (IS_ERR(upstream)) {
+               dev_err(dev, "unable to claim upstream GPIO line\n");
+               return -ENODEV;

Why shadowing error code? What happens if it's deferred?
Hint: use dev_err_probe() here as well.

+       }

...

+       err = devm_gpiochip_add_data(dev, &cas->gpio_chip, NULL);
+       if (err) {
+               dev_err(dev, "unable to add gpio chip\n");
+               return err;
+       }
+
+       platform_set_drvdata(pdev, cas);
+       return 0;

I would rather do

       platform_set_drvdata(pdev, cas);

       return devm_gpiochip_add_data(dev, &cas->gpio_chip, NULL);

-- 
With Best Regards,
Andy Shevchenko