[PATCH v3 2/3] pinctrl: at91: add pinctrl driver

[Raphaël Poggi]

Sounds good, but files like at91samXXX_devices.c need gpio functions.
I think the problem is that the mach-at91/gpio.c file include a gpio
driver which not belong to be here, the right place is in the
driver/pinctrl (like my patch). But all files like
at91samXXX_devices.c are using the functions from this gpio driver.

So I think we have to:

1. Rewrite mach-at91/gpio.c and populate it with basic gpio functions
(cf. http://lxr.free-electrons.com/source/arch/arm/mach-at91/gpio.c
from line 130 to 425).
2. Replace in at91samXXX_devices.c files, reference of functions from
the gpio driver by these basic new functions.

By this way, when AT91 code are registering the gpio subsystem, we use
the pinctrl/gpio driver and also we avoid duplicate code.

Let me know if I am on the right way or not.

Raphaël

2014-09-03 8:56 GMT+02:00 Sascha Hauer <s.hauer at pengutronix.de>:
> On Tue, Sep 02, 2014 at 01:07:59PM +0200, Raphaël Poggi wrote:
>> +static int at91_gpio_probe(struct device_d *dev)
>> +{
>> +     struct at91_gpio_chip *at91_gpio;
>> +     struct clk *clk;
>> +     int ret;
>> +     int alias_idx = of_alias_get_id(dev->device_node, "gpio");
>> +
>> +     BUG_ON(dev->id > MAX_GPIO_BANKS);
>> +
>> +     at91_gpio = &gpio_chip[alias_idx];
>> +
>> +     clk = clk_get(dev, NULL);
>> +     if (IS_ERR(clk)) {
>> +             ret = PTR_ERR(clk);
>> +             dev_err(dev, "clock not found: %d\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     ret = clk_enable(clk);
>> +     if (ret < 0) {
>> +             dev_err(dev, "clock failed to enable: %d\n", ret);
>> +             clk_put(clk);
>> +             return ret;
>> +     }
>> +
>> +     gpio_banks = max(gpio_banks, alias_idx + 1);
>> +     at91_gpio->regbase = dev_request_mem_region(dev, 0);
>
> Please check the return value.
>
> This driver duplicates the existing Atmel GPIO driver. Shouldn't the
> existing driver be dropped? If yes, you should also make sure that this
> driver is compiled unconditionally on Atmel SoCs and add a
>
>         if (!IS_ENABLED(CONFIG_PINCTRL))
>                 return 0;
>
> to at91_pinctrl_init() to reduce the overhead when pinctrl is disabled.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> _______________________________________________
> barebox mailing list
> barebox at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox