Messages in this thread Patch in this message | | | Subject | Re: [PATCH] reset: Make optional functions really optional. | From | Philipp Zabel <> | Date | Fri, 23 Dec 2016 11:58:57 +0100 |
| |
Hi Ramiro,
Am Donnerstag, den 15.12.2016, 18:05 +0000 schrieb Ramiro Oliveira: > Up until now optional functions in the reset API were similar to the non > optional. > > This patch corrects that, while maintaining compatibility with existing > drivers. > > As suggested here: https://lkml.org/lkml/2016/12/14/502 > > Signed-off-by: Ramiro Oliveira <Ramiro.Oliveira@synopsys.com> > --- > drivers/reset/core.c | 21 +++++++++++++++++++-- > include/linux/reset.h | 46 ++++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 61 insertions(+), 6 deletions(-) > > diff --git a/drivers/reset/core.c b/drivers/reset/core.c > index 395dc9c..6150e7c 100644 > --- a/drivers/reset/core.c > +++ b/drivers/reset/core.c > @@ -135,9 +135,14 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register); > * @rstc: reset controller > * > * Calling this on a shared reset controller is an error. > + * > + * If it's an optional reset it will return 0.
I'd prefer this to explicitly mention that rstc==NULL means this is an optional reset:
"If rstc is NULL it is an optional reset and the function will just return 0."
> */ > int reset_control_reset(struct reset_control *rstc) > { > + if (!rstc) > + return 0; > + > if (WARN_ON(rstc->shared)) > return -EINVAL; > > @@ -158,9 +163,14 @@ EXPORT_SYMBOL_GPL(reset_control_reset); > * > * For shared reset controls a driver cannot expect the hw's registers and > * internal state to be reset, but must be prepared for this to happen. > + * > + * If it's an optional reset it will return 0.
Same as above.
> */ > int reset_control_assert(struct reset_control *rstc) > { > + if (!rstc) > + return 0; > + > if (!rstc->rcdev->ops->assert) > return -ENOTSUPP; > > @@ -180,10 +190,14 @@ EXPORT_SYMBOL_GPL(reset_control_assert); > * reset_control_deassert - deasserts the reset line > * @rstc: reset controller > * > - * After calling this function, the reset is guaranteed to be deasserted. > + * After calling this function, the reset is guaranteed to be deasserted, if > + * it's not optional.
Same as above.
> */ > int reset_control_deassert(struct reset_control *rstc) > { > + if (!rstc) > + return 0; > + > if (!rstc->rcdev->ops->deassert) > return -ENOTSUPP; > > @@ -199,11 +213,14 @@ EXPORT_SYMBOL_GPL(reset_control_deassert); > /** > * reset_control_status - returns a negative errno if not supported, a > * positive value if the reset line is asserted, or zero if the reset > - * line is not asserted. > + * line is not asserted or if the desc is NULL (optional reset). > * @rstc: reset controller > */ > int reset_control_status(struct reset_control *rstc) > { > + if (!rstc) > + return 0; > + > if (rstc->rcdev->ops->status) > return rstc->rcdev->ops->status(rstc->rcdev, rstc->id); > > diff --git a/include/linux/reset.h b/include/linux/reset.h > index 5daff15..1af1e62 100644 > --- a/include/linux/reset.h > +++ b/include/linux/reset.h > @@ -138,13 +138,33 @@ static inline struct reset_control *reset_control_get_shared( > static inline struct reset_control *reset_control_get_optional_exclusive( > struct device *dev, const char *id) > { > - return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 0); > + struct reset_control *desc; > + > + desc = __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 0);
Note that the __of_reset_control_get stub returns -ENOTSUPP if CONFIG_RESET_CONTROLLER is disabled.
> + if (IS_ERR(desc)) { > + if (PTR_ERR(desc) == -ENOENT) > + return NULL; > + } > + > + return desc; > + > } > > static inline struct reset_control *reset_control_get_optional_shared( > struct device *dev, const char *id) > { > - return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 1); > + > + struct reset_control *desc; > + > + desc = __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 1); > + > + if (IS_ERR(desc)) { > + if (PTR_ERR(desc) == -ENOENT) > + return NULL; > + }
With this duplication, I think it might be better to add an int optional parameter to __of_reset_control_get and let that return NULL if optional is set and either of_property_match_string or of_parse_phandle_with_args would cause an -ENOENT return.
The stub could then return optional ? NULL : ERR_PTR(-EONOENT);
> + return desc; > } > > /** > @@ -273,13 +293,31 @@ static inline struct reset_control *devm_reset_control_get_shared( > static inline struct reset_control *devm_reset_control_get_optional_exclusive( > struct device *dev, const char *id) > { > - return __devm_reset_control_get(dev, id, 0, 0); > + struct reset_control *desc; > + > + desc = __devm_reset_control_get(dev, id, 0, 0); > + > + if (IS_ERR(desc)) { > + if (PTR_ERR(desc) == -ENOENT) > + return NULL; > + }
Same as for __of_reset_control_get above.
> + return desc; > } > > static inline struct reset_control *devm_reset_control_get_optional_shared( > struct device *dev, const char *id) > { > - return __devm_reset_control_get(dev, id, 0, 1); > + struct reset_control *desc; > + > + desc = __devm_reset_control_get(dev, id, 0, 1); > + > + if (IS_ERR(desc)) { > + if (PTR_ERR(desc) == -ENOENT) > + return NULL; > + } > + > + return desc; > } > > /**
regards Philipp
| |