Message ID | 1444578499-17980-3-git-send-email-jogo@openwrt.org |
---|---|
State | New |
Headers | show |
++ Broadcom engineers working on Cygnus GPIO driver We will discuss this change on the other email thread, i.e., [PATCH RFC/RFT 0/2]... On 10/11/2015 8:48 AM, Jonas Gorski wrote: > Convert the driver to make use of passing ranges to gpiochip_add, and > drop the custom implemtation of gpio_ranges. > Since gpiochip_add_with_ranges also supports conditional setting of > request/free based on the existence of ranges, and pinmux_is_supported > held the same information, we can drop the custom request/free calbacks > at the same time and just use the generic implementations if needed. > > Signed-off-by: Jonas Gorski <jogo@openwrt.org> > --- > drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c | 126 +++++++----------------------- > 1 file changed, 27 insertions(+), 99 deletions(-) > > diff --git a/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c b/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c > index 1ca7830..d24218f 100644 > --- a/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c > +++ b/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c > @@ -75,8 +75,6 @@ > * @lock: lock to protect access to I/O registers > * @gc: GPIO chip > * @num_banks: number of GPIO banks, each bank supports up to 32 GPIOs > - * @pinmux_is_supported: flag to indicate this GPIO controller contains pins > - * that can be individually muxed to GPIO > * @pctl: pointer to pinctrl_dev > * @pctldesc: pinctrl descriptor > */ > @@ -91,8 +89,6 @@ struct cygnus_gpio { > struct gpio_chip gc; > unsigned num_banks; > > - bool pinmux_is_supported; > - > struct pinctrl_dev *pctl; > struct pinctrl_desc pctldesc; > }; > @@ -289,28 +285,6 @@ static struct irq_chip cygnus_gpio_irq_chip = { > /* > * Request the Cygnus IOMUX pinmux controller to mux individual pins to GPIO > */ > -static int cygnus_gpio_request(struct gpio_chip *gc, unsigned offset) > -{ > - struct cygnus_gpio *chip = to_cygnus_gpio(gc); > - unsigned gpio = gc->base + offset; > - > - /* not all Cygnus GPIO pins can be muxed individually */ > - if (!chip->pinmux_is_supported) > - return 0; > - > - return pinctrl_request_gpio(gpio); > -} > - > -static void cygnus_gpio_free(struct gpio_chip *gc, unsigned offset) > -{ > - struct cygnus_gpio *chip = to_cygnus_gpio(gc); > - unsigned gpio = gc->base + offset; > - > - if (!chip->pinmux_is_supported) > - return; > - > - pinctrl_free_gpio(gpio); > -} > > static int cygnus_gpio_direction_input(struct gpio_chip *gc, unsigned gpio) > { > @@ -596,22 +570,12 @@ static const struct pinconf_ops cygnus_pconf_ops = { > .pin_config_set = cygnus_pin_config_set, > }; > > -/* > - * Map a GPIO in the local gpio_chip pin space to a pin in the Cygnus IOMUX > - * pinctrl pin space > - */ > -struct cygnus_gpio_pin_range { > - unsigned offset; > - unsigned pin_base; > - unsigned num_pins; > -}; > - > -#define CYGNUS_PINRANGE(o, p, n) { .offset = o, .pin_base = p, .num_pins = n } > +#define CYGNUS_PINRANGE(o, p, n) { .base = o, .pin_base = p, .npins = n } > > /* > * Pin mapping table for mapping local GPIO pins to Cygnus IOMUX pinctrl pins > */ > -static const struct cygnus_gpio_pin_range cygnus_gpio_pintable[] = { > +static const struct pinctrl_gpio_range cygnus_gpio_pintable[] = { > CYGNUS_PINRANGE(0, 42, 1), > CYGNUS_PINRANGE(1, 44, 3), > CYGNUS_PINRANGE(4, 48, 1), > @@ -666,58 +630,6 @@ static const struct cygnus_gpio_pin_range cygnus_gpio_pintable[] = { > }; > > /* > - * The Cygnus IOMUX controller mainly supports group based mux configuration, > - * but certain pins can be muxed to GPIO individually. Only the ASIU GPIO > - * controller can support this, so it's an optional configuration > - * > - * Return -ENODEV means no support and that's fine > - */ > -static int cygnus_gpio_pinmux_add_range(struct cygnus_gpio *chip) > -{ > - struct device_node *node = chip->dev->of_node; > - struct device_node *pinmux_node; > - struct platform_device *pinmux_pdev; > - struct gpio_chip *gc = &chip->gc; > - int i, ret = 0; > - > - /* parse DT to find the phandle to the pinmux controller */ > - pinmux_node = of_parse_phandle(node, "pinmux", 0); > - if (!pinmux_node) > - return -ENODEV; > - > - pinmux_pdev = of_find_device_by_node(pinmux_node); > - /* no longer need the pinmux node */ > - of_node_put(pinmux_node); > - if (!pinmux_pdev) { > - dev_err(chip->dev, "failed to get pinmux device\n"); > - return -EINVAL; > - } > - > - /* now need to create the mapping between local GPIO and PINMUX pins */ > - for (i = 0; i < ARRAY_SIZE(cygnus_gpio_pintable); i++) { > - ret = gpiochip_add_pin_range(gc, dev_name(&pinmux_pdev->dev), > - cygnus_gpio_pintable[i].offset, > - cygnus_gpio_pintable[i].pin_base, > - cygnus_gpio_pintable[i].num_pins); > - if (ret) { > - dev_err(chip->dev, "unable to add GPIO pin range\n"); > - goto err_put_device; > - } > - } > - > - chip->pinmux_is_supported = true; > - > - /* no need for pinmux_pdev device reference anymore */ > - put_device(&pinmux_pdev->dev); > - return 0; > - > -err_put_device: > - put_device(&pinmux_pdev->dev); > - gpiochip_remove_pin_ranges(gc); > - return ret; > -} > - > -/* > * Cygnus GPIO controller supports some PINCONF related configurations such as > * pull up, pull down, and drive strength, when the pin is configured to GPIO > * > @@ -805,6 +717,10 @@ static int cygnus_gpio_probe(struct platform_device *pdev) > int irq, ret; > const struct of_device_id *match; > const struct cygnus_gpio_data *gpio_data; > + struct device_node *pinmux_node; > + const char *pinctrl_name = NULL; > + const struct pinctrl_gpio_range *ranges = NULL; > + unsigned int nranges = 0; > > match = of_match_device(cygnus_gpio_of_match, dev); > if (!match) > @@ -844,25 +760,37 @@ static int cygnus_gpio_probe(struct platform_device *pdev) > gc->label = dev_name(dev); > gc->dev = dev; > gc->of_node = dev->of_node; > - gc->request = cygnus_gpio_request; > - gc->free = cygnus_gpio_free; > gc->direction_input = cygnus_gpio_direction_input; > gc->direction_output = cygnus_gpio_direction_output; > gc->set = cygnus_gpio_set; > gc->get = cygnus_gpio_get; > > - ret = gpiochip_add(gc); > + /* parse DT to find the phandle to the pinmux controller */ > + pinmux_node = of_parse_phandle(dev->of_node, "pinmux", 0); > + if (pinmux_node) { > + struct platform_device *pinmux_pdev; > + > + pinmux_pdev = of_find_device_by_node(pinmux_node); > + /* no longer need the pinmux node */ > + of_node_put(pinmux_node); > + if (!pinmux_pdev) { > + dev_err(chip->dev, "failed to get pinmux device\n"); > + return -EINVAL; > + } > + > + pinctrl_name = dev_name(&pinmux_pdev->dev); > + ranges = cygnus_gpio_pintable; > + nranges = ARRAY_SIZE(cygnus_gpio_pintable); > + > + put_device(&pinmux_pdev->dev); > + } > + > + ret = gpiochip_add_with_ranges(gc, pinctrl_name, ranges, nranges); > if (ret < 0) { > dev_err(dev, "unable to add GPIO chip\n"); > return ret; > } > > - ret = cygnus_gpio_pinmux_add_range(chip); > - if (ret && ret != -ENODEV) { > - dev_err(dev, "unable to add GPIO pin range\n"); > - goto err_rm_gpiochip; > - } > - > ret = cygnus_gpio_register_pinconf(chip); > if (ret) { > dev_err(dev, "unable to register pinconf\n"); > -- 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
diff --git a/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c b/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c index 1ca7830..d24218f 100644 --- a/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c +++ b/drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c @@ -75,8 +75,6 @@ * @lock: lock to protect access to I/O registers * @gc: GPIO chip * @num_banks: number of GPIO banks, each bank supports up to 32 GPIOs - * @pinmux_is_supported: flag to indicate this GPIO controller contains pins - * that can be individually muxed to GPIO * @pctl: pointer to pinctrl_dev * @pctldesc: pinctrl descriptor */ @@ -91,8 +89,6 @@ struct cygnus_gpio { struct gpio_chip gc; unsigned num_banks; - bool pinmux_is_supported; - struct pinctrl_dev *pctl; struct pinctrl_desc pctldesc; }; @@ -289,28 +285,6 @@ static struct irq_chip cygnus_gpio_irq_chip = { /* * Request the Cygnus IOMUX pinmux controller to mux individual pins to GPIO */ -static int cygnus_gpio_request(struct gpio_chip *gc, unsigned offset) -{ - struct cygnus_gpio *chip = to_cygnus_gpio(gc); - unsigned gpio = gc->base + offset; - - /* not all Cygnus GPIO pins can be muxed individually */ - if (!chip->pinmux_is_supported) - return 0; - - return pinctrl_request_gpio(gpio); -} - -static void cygnus_gpio_free(struct gpio_chip *gc, unsigned offset) -{ - struct cygnus_gpio *chip = to_cygnus_gpio(gc); - unsigned gpio = gc->base + offset; - - if (!chip->pinmux_is_supported) - return; - - pinctrl_free_gpio(gpio); -} static int cygnus_gpio_direction_input(struct gpio_chip *gc, unsigned gpio) { @@ -596,22 +570,12 @@ static const struct pinconf_ops cygnus_pconf_ops = { .pin_config_set = cygnus_pin_config_set, }; -/* - * Map a GPIO in the local gpio_chip pin space to a pin in the Cygnus IOMUX - * pinctrl pin space - */ -struct cygnus_gpio_pin_range { - unsigned offset; - unsigned pin_base; - unsigned num_pins; -}; - -#define CYGNUS_PINRANGE(o, p, n) { .offset = o, .pin_base = p, .num_pins = n } +#define CYGNUS_PINRANGE(o, p, n) { .base = o, .pin_base = p, .npins = n } /* * Pin mapping table for mapping local GPIO pins to Cygnus IOMUX pinctrl pins */ -static const struct cygnus_gpio_pin_range cygnus_gpio_pintable[] = { +static const struct pinctrl_gpio_range cygnus_gpio_pintable[] = { CYGNUS_PINRANGE(0, 42, 1), CYGNUS_PINRANGE(1, 44, 3), CYGNUS_PINRANGE(4, 48, 1), @@ -666,58 +630,6 @@ static const struct cygnus_gpio_pin_range cygnus_gpio_pintable[] = { }; /* - * The Cygnus IOMUX controller mainly supports group based mux configuration, - * but certain pins can be muxed to GPIO individually. Only the ASIU GPIO - * controller can support this, so it's an optional configuration - * - * Return -ENODEV means no support and that's fine - */ -static int cygnus_gpio_pinmux_add_range(struct cygnus_gpio *chip) -{ - struct device_node *node = chip->dev->of_node; - struct device_node *pinmux_node; - struct platform_device *pinmux_pdev; - struct gpio_chip *gc = &chip->gc; - int i, ret = 0; - - /* parse DT to find the phandle to the pinmux controller */ - pinmux_node = of_parse_phandle(node, "pinmux", 0); - if (!pinmux_node) - return -ENODEV; - - pinmux_pdev = of_find_device_by_node(pinmux_node); - /* no longer need the pinmux node */ - of_node_put(pinmux_node); - if (!pinmux_pdev) { - dev_err(chip->dev, "failed to get pinmux device\n"); - return -EINVAL; - } - - /* now need to create the mapping between local GPIO and PINMUX pins */ - for (i = 0; i < ARRAY_SIZE(cygnus_gpio_pintable); i++) { - ret = gpiochip_add_pin_range(gc, dev_name(&pinmux_pdev->dev), - cygnus_gpio_pintable[i].offset, - cygnus_gpio_pintable[i].pin_base, - cygnus_gpio_pintable[i].num_pins); - if (ret) { - dev_err(chip->dev, "unable to add GPIO pin range\n"); - goto err_put_device; - } - } - - chip->pinmux_is_supported = true; - - /* no need for pinmux_pdev device reference anymore */ - put_device(&pinmux_pdev->dev); - return 0; - -err_put_device: - put_device(&pinmux_pdev->dev); - gpiochip_remove_pin_ranges(gc); - return ret; -} - -/* * Cygnus GPIO controller supports some PINCONF related configurations such as * pull up, pull down, and drive strength, when the pin is configured to GPIO * @@ -805,6 +717,10 @@ static int cygnus_gpio_probe(struct platform_device *pdev) int irq, ret; const struct of_device_id *match; const struct cygnus_gpio_data *gpio_data; + struct device_node *pinmux_node; + const char *pinctrl_name = NULL; + const struct pinctrl_gpio_range *ranges = NULL; + unsigned int nranges = 0; match = of_match_device(cygnus_gpio_of_match, dev); if (!match) @@ -844,25 +760,37 @@ static int cygnus_gpio_probe(struct platform_device *pdev) gc->label = dev_name(dev); gc->dev = dev; gc->of_node = dev->of_node; - gc->request = cygnus_gpio_request; - gc->free = cygnus_gpio_free; gc->direction_input = cygnus_gpio_direction_input; gc->direction_output = cygnus_gpio_direction_output; gc->set = cygnus_gpio_set; gc->get = cygnus_gpio_get; - ret = gpiochip_add(gc); + /* parse DT to find the phandle to the pinmux controller */ + pinmux_node = of_parse_phandle(dev->of_node, "pinmux", 0); + if (pinmux_node) { + struct platform_device *pinmux_pdev; + + pinmux_pdev = of_find_device_by_node(pinmux_node); + /* no longer need the pinmux node */ + of_node_put(pinmux_node); + if (!pinmux_pdev) { + dev_err(chip->dev, "failed to get pinmux device\n"); + return -EINVAL; + } + + pinctrl_name = dev_name(&pinmux_pdev->dev); + ranges = cygnus_gpio_pintable; + nranges = ARRAY_SIZE(cygnus_gpio_pintable); + + put_device(&pinmux_pdev->dev); + } + + ret = gpiochip_add_with_ranges(gc, pinctrl_name, ranges, nranges); if (ret < 0) { dev_err(dev, "unable to add GPIO chip\n"); return ret; } - ret = cygnus_gpio_pinmux_add_range(chip); - if (ret && ret != -ENODEV) { - dev_err(dev, "unable to add GPIO pin range\n"); - goto err_rm_gpiochip; - } - ret = cygnus_gpio_register_pinconf(chip); if (ret) { dev_err(dev, "unable to register pinconf\n");
Convert the driver to make use of passing ranges to gpiochip_add, and drop the custom implemtation of gpio_ranges. Since gpiochip_add_with_ranges also supports conditional setting of request/free based on the existence of ranges, and pinmux_is_supported held the same information, we can drop the custom request/free calbacks at the same time and just use the generic implementations if needed. Signed-off-by: Jonas Gorski <jogo@openwrt.org> --- drivers/pinctrl/bcm/pinctrl-cygnus-gpio.c | 126 +++++++----------------------- 1 file changed, 27 insertions(+), 99 deletions(-)