Re: [PATCH 1/2] hwmon: Add PEC attribute support to hardware monitoring core

["Nuno Sá" <noname.nuno@gmail.com>]

On Wed, 2024-05-29 at 11:01 -0700, Guenter Roeck wrote:
> Several hardware monitoring chips optionally support Packet Error Checking
> (PEC). For some chips, PEC support can be enabled simply by setting
> I2C_CLIENT_PEC in the i2c client data structure. Others require chip
> specific code to enable or disable PEC support.
> 
> Introduce hwmon_chip_pec and HWMON_C_PEC to simplify adding configurable
> PEC support for hardware monitoring drivers. A driver can set HWMON_C_PEC
> in its chip information data to indicate PEC support. If a chip requires
> chip specific code to enable or disable PEC support, the driver only needs
> to implement support for the hwmon_chip_pec attribute to its write
> function.
> 
> The hardware monitoring core does not depend on the I2C subsystem after
> this change. However, the I2C subsystem needs to be reachable. This
> requires a new HWMON dependency to ensure that HWMON can only be built
> as module if I2C is built as module. This should not make a practical
> difference.
> 
> Cc: Radu Sabau <radu.sabau@analog.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/Kconfig |   1 +
>  drivers/hwmon/hwmon.c | 136 +++++++++++++++++++++++++++++++++++++-----
>  include/linux/hwmon.h |   2 +
>  3 files changed, 123 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index e14ae18a973b..7f384a2494c9 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -6,6 +6,7 @@
>  menuconfig HWMON
>  	tristate "Hardware Monitoring support"
>  	depends on HAS_IOMEM
> +	depends on I2C || I2C=n
>  	default y
>  	help
>  	  Hardware monitoring devices let you monitor the hardware health
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 3b259c425ab7..1fdea8b1ec91 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -14,6 +14,7 @@
>  #include <linux/err.h>
>  #include <linux/gfp.h>
>  #include <linux/hwmon.h>
> +#include <linux/i2c.h>
>  #include <linux/idr.h>
>  #include <linux/kstrtox.h>
>  #include <linux/list.h>
> @@ -309,6 +310,103 @@ static int hwmon_attr_base(enum hwmon_sensor_types type)
>  	return 1;
>  }
>  
> +/*
> + * PEC support
> + *
> + * The 'pec' attribute is attached to I2C client devices. It is only provided
> + * if the i2c controller supports PEC.
> + *
> + * The mutex ensures that PEC configuration between i2c device and the hardware
> + * is consistent. Use a single mutex because attribute writes are supposed to be
> + * rare, and maintaining a separate mutex for each hardware monitoring device
> + * would add substantial complexity to the driver for little if any gain.
> + *
> + * The hardware monitoring device is identified as child of the i2c client
> + * device. This assumes that only a single hardware monitoring device is
> + * attached to an i2c client device.
> + */
> +
> +static DEFINE_MUTEX(hwmon_pec_mutex);
> +
> +static int hwmon_match_device(struct device *dev, void *data)
> +{
> +	return dev->class == &hwmon_class;
> +}
> +
> +static ssize_t pec_show(struct device *dev, struct device_attribute *dummy,
> +			char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	return sprintf(buf, "%d\n", !!(client->flags & I2C_CLIENT_PEC));

sysfs_emit()?


with the above,

Acked-by: Nuno Sa <nuno.sa@analog.com>