Hi Ramiro, Am Dienstag, den 27.12.2016, 12:37 +0000 schrieb Ramiro Oliveira: > The optional functions weren't really optional so this patch makes them > really optional.
Please add a bit of detail to the description. Since this changes the API, you should mention that the reset_control_get_optional variants now return NULL instead of an error if there is no matching reset phandle in the device tree and that the reset_control_* functions accept NULL rstc pointers. > Signed-off-by: Ramiro Oliveira <[email protected]> > --- > drivers/reset/core.c | 35 +++++++++++++++++++++++++++++------ > include/linux/reset.h | 45 ++++++++++++++++++++++++++------------------- > 2 files changed, 55 insertions(+), 25 deletions(-) > > diff --git a/drivers/reset/core.c b/drivers/reset/core.c > index 70023997d031..f933e9dfebc5 100644 > --- a/drivers/reset/core.c > +++ b/drivers/reset/core.c > @@ -135,9 +135,15 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register); > * @rstc: reset controller > * > * Calling this on a shared reset controller is an error. > + * > + * 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 +164,15 @@ 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 rstc is NULL it is an optional reset and the function will just > + * return 0. > */ > int reset_control_assert(struct reset_control *rstc) > { > + if (!rstc) > + return 0; > + > if (!rstc->rcdev->ops->assert) > return -ENOTSUPP; > > @@ -181,9 +193,15 @@ EXPORT_SYMBOL_GPL(reset_control_assert); > * @rstc: reset controller > * > * After calling this function, the reset is guaranteed to be deasserted. > + * > + * If rstc is NULL it is an optional reset and the function will just > + * return 0. > */ > int reset_control_deassert(struct reset_control *rstc) > { > + if (!rstc) > + return 0; > + > if (!rstc->rcdev->ops->deassert) > return -ENOTSUPP; > > @@ -199,11 +217,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); > > @@ -258,7 +279,8 @@ static void __reset_control_put(struct reset_control > *rstc) > } > > struct reset_control *__of_reset_control_get(struct device_node *node, > - const char *id, int index, bool shared) > + const char *id, int index, bool shared, > + bool optional) > { > struct reset_control *rstc; > struct reset_controller_dev *r, *rcdev; > @@ -273,13 +295,13 @@ struct reset_control *__of_reset_control_get(struct > device_node *node, > index = of_property_match_string(node, > "reset-names", id); > if (index < 0) > - return ERR_PTR(-ENOENT); > + return optional ? NULL : ERR_PTR(-ENOENT); of_property_match_string can return -EINVAL, -ENODATA, or -EILSEQ. I think -EILSEQ should still be returned. > } > > ret = of_parse_phandle_with_args(node, "resets", "#reset-cells", > index, &args); > if (ret) > - return ERR_PTR(ret); > + return optional ? NULL : ERR_PTR(ret); of_parse_phandle_with_args can return -ENOENT or -EINVAL. I think -EINVAL should still be returned. > > mutex_lock(&reset_list_mutex); > rcdev = NULL; > @@ -338,7 +360,8 @@ static void devm_reset_control_release(struct device > *dev, void *res) > } > > struct reset_control *__devm_reset_control_get(struct device *dev, > - const char *id, int index, bool shared) > + const char *id, int index, bool shared, > + bool optional) > { > struct reset_control **ptr, *rstc; > > @@ -348,7 +371,7 @@ struct reset_control *__devm_reset_control_get(struct > device *dev, > return ERR_PTR(-ENOMEM); > > rstc = __of_reset_control_get(dev ? dev->of_node : NULL, > - id, index, shared); > + id, index, shared, optional); > if (!IS_ERR(rstc)) { > *ptr = rstc; > devres_add(dev, ptr); > diff --git a/include/linux/reset.h b/include/linux/reset.h > index ec1d1fd28f5f..e4cc7146b5ed 100644 > --- a/include/linux/reset.h > +++ b/include/linux/reset.h > @@ -13,10 +13,12 @@ int reset_control_deassert(struct reset_control *rstc); > int reset_control_status(struct reset_control *rstc); > > struct reset_control *__of_reset_control_get(struct device_node *node, > - const char *id, int index, bool shared); > + const char *id, int index, bool shared, > + bool optional); > void reset_control_put(struct reset_control *rstc); > struct reset_control *__devm_reset_control_get(struct device *dev, > - const char *id, int index, bool shared); > + const char *id, int index, bool shared, > + bool optional); > > int __must_check device_reset(struct device *dev); > > @@ -69,14 +71,15 @@ static inline int device_reset_optional(struct device > *dev) > > static inline struct reset_control *__of_reset_control_get( > struct device_node *node, > - const char *id, int index, bool shared) > + const char *id, int index, bool shared, > + bool optional) > { > return ERR_PTR(-ENOTSUPP); > } > > static inline struct reset_control *__devm_reset_control_get( > - struct device *dev, > - const char *id, int index, bool shared) > + struct device *dev, const char *id, > + int index, bool shared, bool optional) > { > return ERR_PTR(-ENOTSUPP); > } > @@ -104,7 +107,8 @@ __must_check reset_control_get_exclusive(struct device > *dev, const char *id) > #ifndef CONFIG_RESET_CONTROLLER > WARN_ON(1); > #endif > - return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 0); > + return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, false, > + false); > } > > /** > @@ -132,19 +136,22 @@ __must_check reset_control_get_exclusive(struct device > *dev, const char *id) > static inline struct reset_control *reset_control_get_shared( > struct device *dev, const char *id) > { > - return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, true); > + return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, true, > + false); > } > > 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, false); > + return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, false, > + true); > } > > 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, true); > + return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, true, > + true); > } > > /** > @@ -160,7 +167,7 @@ static inline struct reset_control > *reset_control_get_optional_shared( > static inline struct reset_control *of_reset_control_get_exclusive( > struct device_node *node, const char *id) > { > - return __of_reset_control_get(node, id, 0, 0); > + return __of_reset_control_get(node, id, 0, false, false); > } > > /** > @@ -185,7 +192,7 @@ static inline struct reset_control > *of_reset_control_get_exclusive( > static inline struct reset_control *of_reset_control_get_shared( > struct device_node *node, const char *id) > { > - return __of_reset_control_get(node, id, 0, true); > + return __of_reset_control_get(node, id, 0, true, false); > } > > /** > @@ -202,7 +209,7 @@ static inline struct reset_control > *of_reset_control_get_shared( > static inline struct reset_control *of_reset_control_get_exclusive_by_index( > struct device_node *node, int index) > { > - return __of_reset_control_get(node, NULL, index, false); > + return __of_reset_control_get(node, NULL, index, false, false); > } > > /** > @@ -230,7 +237,7 @@ static inline struct reset_control > *of_reset_control_get_exclusive_by_index( > static inline struct reset_control *of_reset_control_get_shared_by_index( > struct device_node *node, int index) > { > - return __of_reset_control_get(node, NULL, index, true); > + return __of_reset_control_get(node, NULL, index, true, false); > } > > /** > @@ -252,7 +259,7 @@ __must_check devm_reset_control_get_exclusive(struct > device *dev, > #ifndef CONFIG_RESET_CONTROLLER > WARN_ON(1); > #endif > - return __devm_reset_control_get(dev, id, 0, false); > + return __devm_reset_control_get(dev, id, 0, false, false); > } > > /** > @@ -267,19 +274,19 @@ __must_check devm_reset_control_get_exclusive(struct > device *dev, > static inline struct reset_control *devm_reset_control_get_shared( > struct device *dev, const char *id) > { > - return __devm_reset_control_get(dev, id, 0, true); > + return __devm_reset_control_get(dev, id, 0, true, false); > } > > 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, false); > + return __devm_reset_control_get(dev, id, 0, false, true); > } > > 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, true); > + return __devm_reset_control_get(dev, id, 0, true, true); > } > > /** > @@ -297,7 +304,7 @@ static inline struct reset_control > *devm_reset_control_get_optional_shared( > static inline struct reset_control * > devm_reset_control_get_exclusive_by_index(struct device *dev, int index) > { > - return __devm_reset_control_get(dev, NULL, index, false); > + return __devm_reset_control_get(dev, NULL, index, false, false); > } > > /** > @@ -313,7 +320,7 @@ devm_reset_control_get_exclusive_by_index(struct device > *dev, int index) > static inline struct reset_control * > devm_reset_control_get_shared_by_index(struct device *dev, int index) > { > - return __devm_reset_control_get(dev, NULL, index, true); > + return __devm_reset_control_get(dev, NULL, index, true, false); > } > > /* regards Philipp
