Re: [PATCH v6 2/3] gpio: Cygnus: add GPIO driver

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Tue, Jan 13, 2015 at 6:05 PM, Ray Jui <rjui@xxxxxxxxxxxx> wrote:
> On 1/13/2015 12:53 AM, Linus Walleij wrote:
>> On Tue, Dec 16, 2014 at 3:18 AM, Ray Jui <rjui@xxxxxxxxxxxx> wrote:
>>
>>> +/* drive strength control for ASIU GPIO */
>>> +#define CYGNUS_GPIO_ASIU_DRV0_CTRL_OFFSET 0x58
>>> +
>>> +/* drive strength control for CCM GPIO */
>>> +#define CYGNUS_GPIO_CCM_DRV0_CTRL_OFFSET  0x00
>>
>> This stuff (drive strength) is pin control, pin config.
>> It does not belong in a pure GPIO driver. If you're
>> making a combined pin control + GPIO driver, it
>> shall be put in drivers/pinctrl/*
>>
> Okay, I have some questions here. Are you suggesting me to register this
> driver to both the pinctrl subsystem and gpiolib and move it to under
> drivers/pinctrl/*?

Either you can have a combined driver in drivers/pinctrl/*
which has one probe() function calling pinctrl_register(),
gpiochip_add(), gpiochip_add_pin_range(), having the gpio
parts call into the pin control backend with
pinctrl_request_gpio(), pinctrl_free_gpio(),
pinctrl_gpio_direction_input(), pinctrl_gpio_direction_output().

Or you can split it in one driver in drivers/pinctrl/*
dealing with just the pin control stuff, and another driver
in drivers/gpio/* dealing with the GPIO stuff, each with one
probe() function.

If they are using the same register range, the first approach
is probably most intuitive. If the pin control and GPIO parts
are separated in different register ranges, probably the
second approach is the best.

> Or Are you suggesting me to combine this driver with the other Cygnus
> pinctrl driver (which only supports pinmux)?

Depends on which hardware block the pin control-like
registers belongs in. See per above.

> Note in Cygnus, all pinmux logic is done in the pinmux block. And there
> are 3 GPIO controllers, that handle GPIO, drive strength of the GPIO
> pins, internal pull up/down of the GPIO pins, which are handled in this
> driver. So this driver is generic to all 3 GPIO controllers, as you can
> see from the device tree bindings, there are 3 nodes.
>
> Therefore, I think it makes sense to have one pinmux driver that handles
> the pinmux block, and one generic pinctrl + gpio driver that handles
> functions supported by all 3 GPIO controllers. Does this make sense to you?

Yep.

Some hardware designs put the software-controlled biasing
resistors in the GPIO block electronically connected to the actual
pins, so that e.g. the biasing will be available if some MMC or
whatever is using the same pins in another muxing. In such
situations it's quite evident that they need to be a combined
GPIO and pin controller.

I have some regrets that bolting a second pin controller to the
GPIO chip make things a bit complex but it's a price we have
to pay for getting some kind of generic interface.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html