Mon, Oct 02, 2023 at 04:32:30PM CEST, [email protected] wrote: >>From: Vadim Fedorenko <[email protected]> >>Sent: Wednesday, September 27, 2023 8:09 PM >> >>On 27/09/2023 10:24, Arkadiusz Kubalewski wrote: >>> Add callback op (get) for pin-dpll phase-offset measurment. >>> Add callback ops (get/set) for pin signal phase adjustment. >>> Add min and max phase adjustment values to pin proprties. >>> Invoke get callbacks when filling up the pin details to provide user >>> with phase related attribute values. >>> Invoke phase-adjust set callback when phase-adjust value is provided for >>> pin-set request. >>> >>> Signed-off-by: Arkadiusz Kubalewski <[email protected]> >> >>[...] >> >>> +static int >>> +dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr >>> *phase_adj_attr, >>> + struct netlink_ext_ack *extack) >>> +{ >>> + struct dpll_pin_ref *ref; >>> + unsigned long i; >>> + s32 phase_adj; >>> + int ret; >>> + >>> + phase_adj = nla_get_s32(phase_adj_attr); >>> + if (phase_adj > pin->prop->phase_range.max || >>> + phase_adj < pin->prop->phase_range.min) { >>> + NL_SET_ERR_MSG(extack, "phase adjust value not supported"); >>> + return -EINVAL; >>> + } >>> + 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->phase_adjust_set) >>> + return -EOPNOTSUPP; >> >>I'm thinking about this part. We can potentially have dpll devices with >>different expectations on phase adjustments, right? And if one of them >>won't be able to adjust phase (or will fail in the next line), then >>netlink will return EOPNOTSUPP while _some_ of the devices will be >>adjusted. Doesn't look great. Can we think about different way to apply >>the change? >> > >Well makes sense to me. > >Does following makes sense as a fix? >We would call op for all devices which has been provided with the op. >If device has no op -> add extack error, continue
Is it real to expect some of the device support this and others don't? Is it true for ice? If not, I would got for all-or-nothing here. >If device fails to set -> add extack error, continue >Function always returns 0. > >Thank you! >Arkadiusz > >> >>> + ret = ops->phase_adjust_set(pin, >>> + dpll_pin_on_dpll_priv(dpll, pin), >>> + dpll, dpll_priv(dpll), phase_adj, >>> + extack); >>> + if (ret) >>> + return ret; >>> + } >>> + __dpll_pin_change_ntf(pin); >>> + >>> + return 0; >>> +} >>> + _______________________________________________ Intel-wired-lan mailing list [email protected] https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
