On 1/16/2015 2:14 AM, Linus Walleij wrote: > 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 > Okay. In summary, I think both of us think the following approach makes sense in my situation: - leave pinmux in pinctrl-bcm-cygnus.c - leave pinctrl + gpio in pinctrl-bcm-cygnus-gpio.c under drivers/pinctrl/* But by thinking about this more, I thought this would create duplicated pinctrl descriptors in our system, one from the pinmux driver, and the other from this pinctrl+gpio driver. That is probably undesirable? By reviewing various drivers in the pinctrl directory, I found what pinctrl-u300.c and pinctrl-coh901.c does seems to serve as a good model for me to follow: - pinctrl-u300.c is the pinmux driver - pinctrl-coh901.c is the gpio+pinctrl driver The GPIO pinctrl logic is in the coh901 block, so pinctrl-coh901.c exposed two public functions u300_gpio_config_get, u300_gpio_config_set that pinctrl-u300.c can use. The u300 populates all pinmux/pinctrl related functions into the subsystem. This way there's only one pinctrl descriptor, populated through pinctrl-u300.c. Does that model make more sense to you? Thanks, Ray -- 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
