Hi Alan,
On Wed, 2017-03-15 at 10:35 -0400, Alan Stern wrote:
> On Wed, 15 Mar 2017, Philipp Zabel wrote:
>
> > As of commit bb475230b8e5 ("reset: make optional functions really
> > optional"), the reset framework API calls use NULL pointers to describe
> > optional, non-present reset controls.
>
> What does it use to describe genuine errors?
Negative error pointers, as before. The only difference is that instead
of returning -ENOENT if no resets are specified in the device tree, the
optional reset_control_get variants now return NULL.
> > This allows to return errors from devm_reset_control_get_optional_shared
> > unconditionally.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > ---
> > drivers/usb/host/ehci-st.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/host/ehci-st.c b/drivers/usb/host/ehci-st.c
> > index be4a2788fc582..12e803d2c98df 100644
> > --- a/drivers/usb/host/ehci-st.c
> > +++ b/drivers/usb/host/ehci-st.c
> > @@ -210,18 +210,14 @@ static int st_ehci_platform_probe(struct platform_device *dev)
> > devm_reset_control_get_optional_shared(&dev->dev, "power");
> > if (IS_ERR(priv->pwr)) {
> > err = PTR_ERR(priv->pwr);
> > - if (err == -EPROBE_DEFER)
> > - goto err_put_clks;
> > - priv->pwr = NULL;
> > + goto err_put_clks;
> > }
> >
> > priv->rst =
> > devm_reset_control_get_optional_shared(&dev->dev, "softreset");
> > if (IS_ERR(priv->rst)) {
> > err = PTR_ERR(priv->rst);
> > - if (err == -EPROBE_DEFER)
> > - goto err_put_clks;
> > - priv->rst = NULL;
> > + goto err_put_clks;
> > }
> >
> > if (pdata->power_on) {
>
> These changes do not agree with the patch description. If any sort of
> error besides EPROBE_DEFER occurs then:
>
> the old code sets priv->pwr or priv->rst to NULL and continues;
>
> the new code returns with an error.
>
> The only way the patch could be equivalent to the existing code would
> be if devm_reset_control_get_optional_shared() returns no errors other
> than EPROBE_DEFER. But the patch description doesn't say this.
>
> Alan Stern
You are right, devm_reset_control_get_optional_shared can return:
-ENOMEM, returned by devres_alloc in __devm_reset_control_get,
-EILSEQ, returned by of_property_match_string in __of_reset_control_get
if the "reset-names" DT property is broken,
-EINVAL, returned by of_parse_phandle_with_args in
__of_reset_control_get, if there is a reset phandle specified in the
device tree but it is pointing to an invalid reset controller node
(missing "#reset-cells" property or number of phandle arguments does not
match).
-EINVAL, returned by the reset controller driver's choice of of_xlate,
also if the reset is specified but somehow invalid.
So yes, if there was a genuine error in the device tree, we would now
return the error instead of silently ignoring it as before. I assume
that these errors were never intended to be ignored, it just happened to
be a hassle to separate them from the -ENOENT condition with the old
API.
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
