On Mon, Mar 16, 2015 at 4:21 PM, Baruch Siach <baruch@xxxxxxxxxx> 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@xxxxxxxxxx>
(...)
> +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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
