On Mon, Mar 16, 2015 at 4:21 PM, Baruch Siach [off-list ref] wrote:

This adds pinctrl and gpio driver to the CX92755 SoC "General Purpose Pin
Mapping" hardware block. The CX92755 is one SoC from the Conexant Digicolor
series. Pin mapping hardware supports configuring pins as either GPIO, or up to
3 other "client select" functions. This driver adds support for pin muxing
using the generic device tree binding, and a basic gpiolib driver for the GPIO
functionality.

This driver does not currently support GPIO interrupts, and pad configuration.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>

(...)

+struct dc_pinmap {
+       void __iomem            *regs;
+       struct device           *dev;
+       struct pinctrl_dev      *pctl;
+
+       struct pinctrl_pin_desc *pins;

Instead of storing just an array of pinctrl_pin_desc
use the .pins in struct pinctrl_desc and store a pointer
to your struct pinctrl_desc in this state container.

+       const char              *pin_names[PINS_COUNT];

This duplicates names that should already be in the
.pins array in struct pinctrl_desc, don't do that.

+
+       struct gpio_chip        chip;
+       spinlock_t              lock;
+};

+static const char *dc_get_group_name(struct pinctrl_dev *pctldev,
+                                    unsigned selector)
+{
+       struct dc_pinmap *pmap = pinctrl_dev_get_drvdata(pctldev);
+
+       return pmap->pin_names[selector];
+}

Add some comment explaining that you have exactly one group per pin.

You are also re-implementing the .pins field in struct pinctrl_desc
where each pin is described by a struct pinctrl_pin_desc with
special macros available to define them and all.

If you want to have one group per pin named like this
why not just

return pmap->desc->pins[selector].name;

?

Yours,
Linus Walleij