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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
