Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute

[Ivan Vecera]

On 10/29/25 11:20 AM, Vadim Fedorenko wrote:
> On 24/10/2025 15:49, Ivan Vecera wrote:
> > Phase-adjust values are currently limited by a min-max range. Some
> > hardware requires, for certain pin types, that values be multiples of
> > a specific granularity, as in the zl3073x driver.
> >
> > Add a `phase-adjust-gran` pin attribute and an appropriate field in
> > dpll_pin_properties. If set by the driver, use its value to validate
> > user-provided phase-adjust values.
> >
> > Reviewed-by: Michal Schmidt <mschmidt@xxxxxxxxxx>
> > Reviewed-by: Petr Oros <poros@xxxxxxxxxx>
> > Tested-by: Prathosh Satish <Prathosh.Satish@xxxxxxxxxxxxx>
> > Signed-off-by: Ivan Vecera <ivecera@xxxxxxxxxx>
> > ---
> >   Documentation/driver-api/dpll.rst     | 36 +++++++++++++++------------
> >   Documentation/netlink/specs/dpll.yaml |  7 ++++++
> >   drivers/dpll/dpll_netlink.c           | 12 ++++++++-
> >   include/linux/dpll.h                  |  1 +
> >   include/uapi/linux/dpll.h             |  1 +
> >   5 files changed, 40 insertions(+), 17 deletions(-)
> >
> > @@ -1261,7 +1265,13 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, 
> > struct nlattr *phase_adj_attr,
> >       if (phase_adj > pin->prop.phase_range.max ||
> >           phase_adj < pin->prop.phase_range.min) {
> >           NL_SET_ERR_MSG_ATTR(extack, phase_adj_attr,
> > -                    "phase adjust value not supported");
> > +                    "phase adjust value of out range");
> > +        return -EINVAL;
> > +    }
> > +    if (pin->prop.phase_gran && phase_adj % pin->prop.phase_gran) {
> > +        NL_SET_ERR_MSG_ATTR_FMT(extack, phase_adj_attr,
> > +                    "phase adjust value not multiple of %u",
> > +                    pin->prop.phase_gran);
>
> That is pretty strict on the uAPI input. Maybe it's better to allow any
> value, but report back the one that is really applied based on driver's
> understanding of hardware? I mean the driver is doing some math before
> applying the math, it can potentially round any value to the closest
> acceptable and report it back?

I’d prefer to use the same approach as for phase-adjust-{min,max}.
Because we could treat them the same way - the user sets a value
above/below the max/min, and the driver clamps it.

Would it be better? I don't think so.

Let’s say the granularity is 1000, and the user sets a value of 499...
then the driver rounds it and sets it to 0. The user then reads the
current value via pin-get and finds that it hasn’t changed - quite
confusing, I’d say. If the user knows the granularity in advance,
they can adjust accordingly.

IMHO, strict behavior is better than smart behavior.

Ivan