Hello,
On Tue, Jun 17, 2025 at 05:34:56PM +0200, Jorge Marques wrote:
> On Tue, Jun 17, 2025 at 04:59:48PM +0200, Uwe Kleine-König wrote:
> > On Tue, Jun 10, 2025 at 09:34:37AM +0200, Jorge Marques wrote:
> > > +static int ad4052_get_samp_freq(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int *val,
> > > + int *val2)
> > > +{
> > > + struct ad4052_state *st = iio_priv(indio_dev);
> > > +
> > > + *val = DIV_ROUND_UP_ULL(NSEC_PER_SEC, st->pwm_st.period);
> > > + return IIO_VAL_INT;
> >
> > st->pwm_st.period is the period that was requested before. If you want
> > the real period that is currently emitted, check pwm_get_state_hw().
>
> I believe only ad4695.c uses this method and the reason for that is if
> the pwm is disabled we still want to obtain the requested value.
>
> Reverting slightly to v2, the semantic to allow fetching from hw when
> enabled, and using the managed state when disabled, would be:
>
> struct pwm_state pwm_st;
> int ret
>
> ret = pwm_get_state_hw(st->cnv_pwm, &pwm_st);
> if (ret)
> goto out_release;
>
> if (!pwm_st.enabled)
> pwm_st = st->pwm_st;
>
> *val = DIV_ROUND_UP_ULL(NSEC_PER_SEC, pwm_st.period);
>
> Is this ok?
Looks fine to me. I didn't object the original suggested code, just
wanted to highlight the semantics.
I would expect that the compiler optimizes out the unnecessary
assignments done in pwm_st = st->pwm_st, as only pwm_st.period is used
later on.
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
