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

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Badal,

> > > +struct hwm_reg {
> > > +};
> > > +
> > > +struct hwm_drvdata {
> > > +	struct i915_hwmon *hwmon;
> > > +	struct intel_uncore *uncore;
> > > +	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;
> > > +};
> > > +
> > > +static const struct hwmon_channel_info *hwm_info[] = {
> > > +	NULL
> > > +};
> > > +
> > > +static umode_t
> > > +hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> > > +	       u32 attr, int channel)
> > > +{
> > > +	switch (type) {
> > > +	default:
> > > +		return 0;
> > > +	}
> > > +}
> > > +
> > > +static int
> > > +hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> > > +	 int channel, long *val)
> > > +{
> > > +	switch (type) {
> > > +	default:
> > > +		return -EOPNOTSUPP;
> > > +	}
> > > +}
> > > +
> > > +static int
> > > +hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> > > +	  int channel, long val)
> > > +{
> > > +	switch (type) {
> > > +	default:
> > > +		return -EOPNOTSUPP;
> > > +	}
> > > +}
> > > +
> > > +static const struct hwmon_ops hwm_ops = {
> > > +	.is_visible = hwm_is_visible,
> > > +	.read = hwm_read,
> > > +	.write = hwm_write,
> > > +};
> > > +
> > > +static const struct hwmon_chip_info hwm_chip_info = {
> > > +	.ops = &hwm_ops,
> > > +	.info = hwm_info,
> > > +};
> > 
> > what's the point for splitting so much? Can't you just send the
> > hwmon driver all at once? With this patch you are not actually
> > doing anything useful. In my opinion this should be squashed with
> > the next ones.

> During discussion in cover letter of rev0 series we decided to create
> separate infrastructure patch, as we wanted to keep kconfig, i915 hwmon
> structures and new file addition in separate patch. Further feature wise we
> kept adding new patches.

I don't really like this patch splitting, but it's my fault I
haven't reviewed it already in v1. Please, ignore then.

Andi