Fri, Oct 06, 2023 at 01:41:01PM CEST, arkadiusz.kubalewski@xxxxxxxxx wrote:
>Align the aproach of pin frequency set behavior with the approach
>introduced with pin phase adjust set.
>Fail the request if any of devices did not registered the callback ops.
>If callback op on any pin's registered device fails, return error and
>rollback the value to previous one.
>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@xxxxxxxxx>
>---
> drivers/dpll/dpll_netlink.c | 50 +++++++++++++++++++++++++++++--------
> 1 file changed, 40 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>index 97319a9e4667..8e5fea74aec1 100644
>--- a/drivers/dpll/dpll_netlink.c
>+++ b/drivers/dpll/dpll_netlink.c
>@@ -615,30 +615,60 @@ static int
> dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
> struct netlink_ext_ack *extack)
> {
>- u64 freq = nla_get_u64(a);
>- struct dpll_pin_ref *ref;
>+ u64 freq = nla_get_u64(a), old_freq;
>+ struct dpll_pin_ref *ref, *failed;
>+ const struct dpll_pin_ops *ops;
>+ struct dpll_device *dpll;
> unsigned long i;
> int ret;
>
> if (!dpll_pin_is_freq_supported(pin, freq)) {
>- NL_SET_ERR_MSG_ATTR(extack, a, "frequency is not supported by the device");
>+ NL_SET_ERR_MSG_ATTR(extack, a,
>+ "frequency is not supported by the device");
No need for this wrapping. Seems unrelated to the rest of the patch
anyway.
> return -EINVAL;
> }
>-
No need for this too.
> xa_for_each(&pin->dpll_refs, i, ref) {
>- const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
>- struct dpll_device *dpll = ref->dpll;
>-
>- if (!ops->frequency_set)
>+ ops = dpll_pin_ops(ref);
>+ if (!ops->frequency_set || !ops->frequency_get)
> return -EOPNOTSUPP;
Add an extack msg while you are at it - could be a separate patch.
>+ }
>+ ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
>+ ops = dpll_pin_ops(ref);
>+ dpll = ref->dpll;
>+ ret = ops->frequency_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
>+ dpll_priv(dpll), &old_freq, extack);
>+ if (ret) {
>+ NL_SET_ERR_MSG(extack, "unable to get old frequency value");
>+ return ret;
>+ }
>+ if (freq == old_freq)
>+ return 0;
>+
>+ xa_for_each(&pin->dpll_refs, i, ref) {
>+ ops = dpll_pin_ops(ref);
>+ dpll = ref->dpll;
> ret = ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
> dpll, dpll_priv(dpll), freq, extack);
>- if (ret)
>- return ret;
>+ if (ret) {
>+ failed = ref;
Extack msg.
>+ goto rollback;
>+ }
> }
> __dpll_pin_change_ntf(pin);
>
> return 0;
>+
>+rollback:
>+ xa_for_each(&pin->dpll_refs, i, ref) {
>+ if (ref == failed)
>+ break;
>+ ops = dpll_pin_ops(ref);
>+ dpll = ref->dpll;
>+ if (ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), old_freq, extack))
>+ NL_SET_ERR_MSG(extack, "set frequency rollback failed");
>+ }
>+ return ret;
> }
>
> static int
>--
>2.38.1
>
