>From: Vadim Fedorenko <vadim.fedorenko@xxxxxxxxx>
>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 <arkadiusz.kubalewski@xxxxxxxxx>
>
>[...]
>
>> +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
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;
>> +}
>> +
