Hello Kavya, On 29/10/2021 14:57:03+0530, Kavyasree Kotagiri wrote: > + LAN966X_PIN(76), > + LAN966X_PIN(77), > +}; > + > + One blank line should be removed > static int ocelot_get_functions_count(struct pinctrl_dev *pctldev) > { > return ARRAY_SIZE(ocelot_function_names); > @@ -709,6 +1056,9 @@ static int ocelot_pin_function_idx(struct ocelot_pinctrl *info, > for (i = 0; i < OCELOT_FUNC_PER_PIN; i++) { > if (function == p->functions[i]) > return i; > + > + if (function == p->a_functions[i]) > + return i + OCELOT_FUNC_PER_PIN; > } > > return -1; > @@ -744,6 +1094,36 @@ static int ocelot_pinmux_set_mux(struct pinctrl_dev *pctldev, > return 0; > } > > +static int lan966x_pinmux_set_mux(struct pinctrl_dev *pctldev, > + unsigned int selector, unsigned int group) > +{ > + struct ocelot_pinctrl *info = pinctrl_dev_get_drvdata(pctldev); > + struct ocelot_pin_caps *pin = info->desc->pins[group].drv_data; > + unsigned int p = pin->pin % 32; > + int f; > + > + f = ocelot_pin_function_idx(info, group, selector); > + if (f < 0) > + return -EINVAL; > + > + /* > + * f is encoded on three bits. > + * bit 0 of f goes in BIT(pin) of ALT[0], bit 1 of f goes in BIT(pin) of > + * ALT[1], bit 2 of f goes in BIT(pin) of ALT[2] > + * This is racy because both registers can't be updated at the same time That's three registers, not two so I guess this sentence should be reworked. > + * but it doesn't matter much for now. > + * Note: ALT0/ALT1/ALT2 are organized specially for 78 gpio targets > + */ > + regmap_update_bits(info->map, REG_ALT(0, info, pin->pin), > + BIT(p), f << p); > + regmap_update_bits(info->map, REG_ALT(1, info, pin->pin), > + BIT(p), (f >> 1) << p); > + regmap_update_bits(info->map, REG_ALT(2, info, pin->pin), > + BIT(p), (f >> 2) << p); > + I would have thought the hardware would be fixed because you now have 78 pins and this probably will get worse over time. This is really a poor choice of interface as now you will get two transient states instead of one. > + return 0; > +} > + > #define REG(r, info, p) ((r) * (info)->stride + (4 * ((p) / 32))) > > static int ocelot_gpio_set_direction(struct pinctrl_dev *pctldev, > @@ -774,6 +1154,23 @@ static int ocelot_gpio_request_enable(struct pinctrl_dev *pctldev, > return 0; > } > > +static int lan966x_gpio_request_enable(struct pinctrl_dev *pctldev, > + struct pinctrl_gpio_range *range, > + unsigned int offset) > +{ > + struct ocelot_pinctrl *info = pinctrl_dev_get_drvdata(pctldev); > + unsigned int p = offset % 32; > + > + regmap_update_bits(info->map, REG_ALT(0, info, offset), > + BIT(p), 0); > + regmap_update_bits(info->map, REG_ALT(1, info, offset), > + BIT(p), 0); > + regmap_update_bits(info->map, REG_ALT(2, info, offset), > + BIT(p), 0); > + > + return 0; > +} > + > static const struct pinmux_ops ocelot_pmx_ops = { > .get_functions_count = ocelot_get_functions_count, > .get_function_name = ocelot_get_function_name, > @@ -783,6 +1180,15 @@ static const struct pinmux_ops ocelot_pmx_ops = { > .gpio_request_enable = ocelot_gpio_request_enable, > }; > > +static const struct pinmux_ops lan966x_pmx_ops = { > + .get_functions_count = ocelot_get_functions_count, > + .get_function_name = ocelot_get_function_name, > + .get_function_groups = ocelot_get_function_groups, > + .set_mux = lan966x_pinmux_set_mux, > + .gpio_set_direction = ocelot_gpio_set_direction, > + .gpio_request_enable = lan966x_gpio_request_enable, > +}; > + > static int ocelot_pctl_get_groups_count(struct pinctrl_dev *pctldev) > { > struct ocelot_pinctrl *info = pinctrl_dev_get_drvdata(pctldev); > @@ -1074,6 +1480,14 @@ static struct pinctrl_desc sparx5_desc = { > .npins = ARRAY_SIZE(sparx5_pins), > .pctlops = &ocelot_pctl_ops, > .pmxops = &ocelot_pmx_ops, > +}; > + > +static struct pinctrl_desc lan966x_desc = { > + .name = "lan966x-pinctrl", > + .pins = lan966x_pins, > + .npins = ARRAY_SIZE(lan966x_pins), > + .pctlops = &ocelot_pctl_ops, > + .pmxops = &lan966x_pmx_ops, > .confops = &ocelot_confops, > .owner = THIS_MODULE, > }; > @@ -1114,6 +1528,7 @@ static int ocelot_create_group_func_map(struct device *dev, > return 0; > } > > + Useless blank line > static int ocelot_pinctrl_register(struct platform_device *pdev, > struct ocelot_pinctrl *info) > { > @@ -1337,6 +1752,7 @@ static const struct of_device_id ocelot_pinctrl_of_match[] = { > { .compatible = "mscc,ocelot-pinctrl", .data = &ocelot_desc }, > { .compatible = "mscc,jaguar2-pinctrl", .data = &jaguar2_desc }, > { .compatible = "microchip,sparx5-pinctrl", .data = &sparx5_desc }, > + { .compatible = "microchip,lan966x-pinctrl", .data = &lan966x_desc }, > {}, > }; > > -- > 2.17.1 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com