On Tue, Sep 26, 2017 at 01:08:21PM +0000, Quentin Schulz wrote: > Hi Maxime, > > On 26/09/2017 15:00, Maxime Ripard wrote: > > On Tue, Sep 26, 2017 at 12:17:12PM +0000, Quentin Schulz wrote: > >> +static const struct axp20x_desc_pin axp209_pins[] = { > >> + AXP20X_PIN(AXP20X_PINCTRL_PIN(0, "GPIO0"), > >> + AXP20X_FUNCTION(0x0, "gpio_out"), > >> + AXP20X_FUNCTION(0x2, "gpio_in"), > >> + AXP20X_FUNCTION(0x3, "ldo"), > >> + AXP20X_FUNCTION(0x4, "adc")), > >> + AXP20X_PIN(AXP20X_PINCTRL_PIN(1, "GPIO1"), > >> + AXP20X_FUNCTION(0x0, "gpio_out"), > >> + AXP20X_FUNCTION(0x2, "gpio_in"), > >> + AXP20X_FUNCTION(0x3, "ldo"), > >> + AXP20X_FUNCTION(0x4, "adc")), > >> + AXP20X_PIN(AXP20X_PINCTRL_PIN(2, "GPIO2"), > >> + AXP20X_FUNCTION(0x0, "gpio_out"), > >> + AXP20X_FUNCTION(0x2, "gpio_in")), > >> +}; > > > > If all the functions are the same, and at the same offset, can't we > > just hardcode it, instead of having (and duplicate) all the logic > > below? > > > > AXP20X_PIN(AXP20X_PINCTRL_PIN(0, "GPIO0"), > AXP20X_GPIO_OUT, > AXP20X_GPIO_IN, > AXP20X_LDO, > AXP20X_ADC)) > > That's what you mean? What I mean is: static int axp20x_get_func(char *func) { if (!strcmp(func, "gpio_out")) return 0; if (!strcmp(func, "gpio_in")) return 2; if (!strcmp(func, "ldo")) return 3; if (!strcmp(func, "adc")) return 4; return -EINVAL; } > >> + pctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctrl_desc), GFP_KERNEL); > >> + if (!pctrl_desc) > >> + return -ENOMEM; > >> + > >> + pctrl_desc->name = dev_name(&pdev->dev); > >> + pctrl_desc->owner = THIS_MODULE; > >> + pctrl_desc->pins = pins; > >> + pctrl_desc->npins = gpio->desc->npins; > >> + pctrl_desc->pctlops = &axp20x_pctrl_ops; > >> + pctrl_desc->pmxops = &axp20x_pmx_ops; > > > > The strict flag needs to be set too in order to avoid concurrent uses > > of GPIO and other functions. > > > > Strict is a property of pinmux_ops struct (pmxops) and it is set. Ah, right, my bad. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Attachment:
signature.asc
Description: PGP signature