On Thu, Jan 17, 2019 at 11:09:43AM +0300, Alexander Shiyan wrote:
> >Четверг, 17 января 2019, 10:49 +03:00 от Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>:
> ...
> >> >> >> >On Sat, Dec 22, 2018 at 06:54:34PM +0300, Alexander Shiyan wrote:
> >> >> >> >> As mentioned in the patch bdb0066df96e ("mfd: syscon: Decouple syscon
> >> >> >> >> interface from platform devices"), we need to switch to using the
> >> >> >> >> syscon_regmap_lookup_by_phandle() function. This patch makes this change.
> >> >> >> >>
> >> >> >> >> Signed-off-by: Alexander Shiyan < shc_work@xxxxxxx >
> >> >> >> >> ---
> >> >> >> >> drivers/input/keyboard/clps711x-keypad.c | 3 +--
> >> >> >> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >> >> >> >>
> >> >> >> >> diff --git a/drivers/input/keyboard/clps711x-keypad.c b/drivers/input/keyboard/clps711x-keypad.c
> >> >> >> >> index e319f74..f7f49da 100644
> >> >> >> >> --- a/drivers/input/keyboard/clps711x-keypad.c
> >> >> >> >> +++ b/drivers/input/keyboard/clps711x-keypad.c
> >> >> >> >> @@ -100,8 +100,7 @@ static int clps711x_keypad_probe(struct platform_device *pdev)
> >> >> >> >> if (!priv)
> >> >> >> >> return -ENOMEM;
> >> >> >> >>
> >> >> >> >> -priv->syscon =
> >> >> >> >> -syscon_regmap_lookup_by_compatible("cirrus,ep7209-syscon1");
> >> >> >> >> +priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
> >> ...
> >> >> >I would love to just apply the patch as is, but I am afraid we need to
> >> >> >keep compatibility with older DTSes.
> >> >>
> >> >> Ok, will it be better if we keep the syscon_regmap_lookup_by_compatible()
> >> >> call as a fallback method for getting syscon phandle?
> >> >
> >> >Yes, please. Just make sure you do not try to fall back if
> >> >syscon_regmap_lookup_by_phandle() returns -EPROBE_DEFER, but for all
> >> >other errors we should try old syscon_regmap_lookup_by_compatible().
> >>
> >> This check is no longer needed after the patch mentioned in the description.
> >
> >Why exactly?
>
> As far as I understand, syscon_regmap_lookup_by_phandle() calls syscon_node_to_regmap(),
> where it tries to find already registered syscon devices, then, if absent, of_syscon_register()
> is called, so we always get a syscon device or get an -ENODEV error.
of_syscon_register() may fail, and we do not know what error codes it
might return. Even if at this time it does not ever return
-EPROBE_DEFER, this can change in the future, and in this case we should
not execute fallback.
>
> The code below works fine for me.
>
> priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
> if (IS_ERR(priv->syscon)) {
Please change tp:
if (PTR_ERR(priv->syscon) != -EPROBE_DEFER) {
/* falling back to old bindings */
priv->syscon =
syscon_regmap_lookup_by_compatible(
"cirrus,ep7209-syscon1");
}
> /* falling back to old bindings */
> priv->syscon =
> syscon_regmap_lookup_by_compatible("cirrus,ep7209-syscon1");
> if (IS_ERR(priv->syscon))
> return PTR_ERR(priv->syscon);
> else
> dev_warn(dev, "Please migrate driver bindings"
> " to use syscon phandle.\n");
> }
>
> ---
Thanks.
--
Dmitry
