On 25.11.2015 11:27, Linus Walleij wrote:
trying to use the Kirkwood pinctrl driver with compatible =
"marvell,88f6192-pinctrl";
on a Pogoplug series 4 yields the following message when instantiating
the driver:
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 36
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 37
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 38
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 39
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 40
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 41
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 42
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 43
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 44
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 45
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 46
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 47
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 48
kirkwood-pinctrl f1010000.pin-controller: unknown pinctrl group 49
kirkwood-pinctrl f1010000.pin-controller: registered pinctrl driver
It looks harmless but seems like a bug and make me uncertain.
The following naive patch fixes it:
diff --git a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
index 0f07dc554a1d..6c7c2c8819b8 100644
--- a/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
+++ b/drivers/pinctrl/mvebu/pinctrl-kirkwood.c
@@ -411,7 +411,7 @@ static struct mvebu_pinctrl_soc_info mv88f6190_info = {
.controls = mv88f619x_mpp_controls,
.ncontrols = ARRAY_SIZE(mv88f619x_mpp_controls),
.modes = mv88f6xxx_mpp_modes,
- .nmodes = ARRAY_SIZE(mv88f6xxx_mpp_modes),
+ .nmodes = ARRAY_SIZE(mv88f6xxx_mpp_modes) - 14,
.gpioranges = mv88f619x_gpio_ranges,
.ngpioranges = ARRAY_SIZE(mv88f619x_gpio_ranges),
};
@@ -421,7 +421,7 @@ static struct mvebu_pinctrl_soc_info mv88f6192_info = {
.controls = mv88f619x_mpp_controls,
.ncontrols = ARRAY_SIZE(mv88f619x_mpp_controls),
.modes = mv88f6xxx_mpp_modes,
- .nmodes = ARRAY_SIZE(mv88f6xxx_mpp_modes),
+ .nmodes = ARRAY_SIZE(mv88f6xxx_mpp_modes) - 14,
.gpioranges = mv88f619x_gpio_ranges,
.ngpioranges = ARRAY_SIZE(mv88f619x_gpio_ranges),
};
What is the proper way to fix this?
Linus,
I had a quick look at the pinctrl driver.
mv88f6xxx_mpp_modes contains mpp modes 0-49 plus corresponding
functions for all Kirkwood SoCs, some SoCs only have a subset
of that.
Looking at
static struct mvebu_mpp_ctrl mv88f619x_mpp_controls[] = {
MPP_FUNC_CTRL(0, 35, NULL, kirkwood_mpp_ctrl),
};
Kirkwood 619x only provides mpp0-35.
Now in pinctrl-mvebu.c, we loop over the controls and
collect the number of available groups. For kirkwood
there are no groups with more than one single mpp pin
like Dove has.
/* count controls and create names for mvebu generic
register controls; also does sanity checks */
pctl->num_groups = 0;
pctl->desc.npins = 0;
for (n = 0; n < soc->ncontrols; n++) {
struct mvebu_mpp_ctrl *ctrl = &soc->controls[n];
...
/*
* We allow to pass controls with NULL name that we treat
* as a range of one-pin groups with generic mvebu register
* controls.
*/
if (!ctrl->name) {
pctl->num_groups += ctrl->npins;
...
} else {
pctl->num_groups += 1;
}
}
After the loop pctl->num_groups is 36, i.e. mpp0 to mpp35.
A little later, we do:
/* assign mpp modes to groups */
for (n = 0; n < soc->nmodes; n++) {
struct mvebu_mpp_mode *mode = &soc->modes[n];
struct mvebu_pinctrl_group *grp =
mvebu_pinctrl_find_group_by_pid(pctl, mode->pid);
unsigned num_settings;
if (!grp) {
dev_warn(&pdev->dev, "unknown pinctrl group %d\n",
mode->pid);
continue;
}
...
}
Which is looping over all modes (0-49) passed to the pinctrl-mvebu
core driver. As said earlier, we pass one control with range from
0-35 that gets translated to 36 groups (pctl->num_groups).
mvebu_find_group_by_pid() will try to find the corresponding group
for a given pin number by checking pctl->num_groups.
That obviously fails for modes 36-49 and will issue that annoying
warning.
IMHO, the correct fix will be to make the last loop above run from
0 to min(pctl->num_groups, soc->nmodes) instead of soc->nmodes.
We could also limit pctl->num_groups to soc->nmodes earlier and let
the loop run from 0 to pctl->num_groups.
I am very short on time, but if nobody else jumps in earlier, I can
stich a patch within a week or so.
Thanks for reporting the issue,
Sebastian
--
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
