On Thu, 25 Aug 2022 06:21:12 -0700, Badal Nilawar wrote: > A couple of minor observations below but otherwise this patch is: Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > new file mode 100644 > index 000000000000..103dd543a214 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_hwmon.c /snip/ > +struct hwm_reg { > +}; > + > +struct hwm_drvdata { > + struct i915_hwmon *hwmon; > + struct intel_uncore *uncore; Instead of 'struct intel_uncore' we could have a 'struct intel_gt' here since intel_gt is a higher level but I think uncore is fine and anyway has a backpointer to intel_gt should we need it. So no changes needed. > + struct device *hwmon_dev; > + char name[12]; > +}; > + > +struct i915_hwmon { > + struct hwm_drvdata ddat; > + struct mutex hwmon_lock; /* counter overflow logic and rmw */ > + struct hwm_reg rg; > +}; Somebody looking at just this patch might wonder why we have two data structs hwm_drvdata and i915_hwmon, rather than just one. The answer becomes clear in a later patch and that of course is that i915 exposes multiple hwmon devices. Anyway, just an observation, no changes required.