Re: [Intel-gfx] [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

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@intel.com>

> 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.