On Tue, Feb 14, 2023 at 12:20:36PM -0800, Dixit, Ashutosh wrote:
> On Tue, 14 Feb 2023 06:50:44 -0800, Rodrigo Vivi wrote:
> >
> > > +static int
> > > +hwm_power_max_write(struct hwm_drvdata *ddat, long val)
> > +{
> > > + struct i915_hwmon *hwmon = ddat->hwmon;
> > > + u32 nval;
> > > +
> > > + /* Computation in 64-bits to avoid overflow. Round to nearest. */
> > > + nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> > > +
> > > + hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > > + PKG_PWR_LIM_1,
> > > + REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> > > + return 0;
> >
> > Let's keep this function as void and the return 0 in the previous spot.
>
> Hmm, see your point. Though there is an identical situation for
> hwm_power_max_read read too (in hwm_power_read). Maybe I'll change it there
> too in the same patch to keep things symmetrical and retain your R-b?
okay then. let's move this one as is and fix both functions in a follow up.
Thanks,
Rodrigo.
>
> > With that change:
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>
> Thanks.
