Re: [PATCH] pinctrl: qcom: Enable device links to consumers

[Stephen Boyd <swboyd@xxxxxxxxxxxx>]

Quoting Linus Walleij (2019-05-23 01:06:54)
> A recent core change makes it possible to create device links
> between a pin controller and its consumers. This is necessary
> to ascertain the right suspend/resume order for the devices:
> if a device is using a certain pin control state and want
> to switch that before/after going to suspend, then the pin
> controller may not be suspended already, and conversely
> on the resume path.
> 
> Make sure any qcom pin control consumers are suspended before
> the qcom pin control is suspended.
> 
> Since Qualcomm is one of the few pin controllers implementing
> suspend/resume I suppose you will see this problem sooner
> or later so let's see if we can just fix it right now before
> you run into it.
> 
> Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> Cc: Brian Masney <masneyb@xxxxxxxxxxxxx>
> Cc: Lina Iyer <ilina@xxxxxxxxxxxxxx>
> Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> You can test this patch by pulling in this branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=consumer-links
> ---
>  drivers/pinctrl/qcom/pinctrl-msm.c | 1 +
>  1 file changed, 1 insertion(+)

I don't know how much it will matter for qcom right now. This pinctrl
driver just forces over some suspend pins for the hogs during suspend,
so it's not like drivers that are suspended after this moves hogs over
will break, unless somehow the hogs change behavior of the pins those
other drivers are using which doesn't seem possible.

Also, what is the usecase for device links in pinctrl? Doesn't the
driver core reorder the suspend list when probing devices so that
devices that probe defer get moved later in list and thus suspended
first? I can understand that runtime suspend may be important because
order of suspend isn't fixed, but system suspend should be unaffected,
right?

> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index ee8119879c4c..d4a6edbccdb9 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -1139,6 +1139,7 @@ int msm_pinctrl_probe(struct platform_device *pdev,
>         pctrl->desc.name = dev_name(&pdev->dev);
>         pctrl->desc.pins = pctrl->soc->pins;
>         pctrl->desc.npins = pctrl->soc->npins;
> +       pctrl->desc.link_consumers = true;

Why is it an opt-in flag instead of a mandated feature for all pinctrl
providers?