Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-11 Thread Guenter Roeck 

On 07/11/2017 06:20 PM, Andrew Jeffery wrote:

On Tue, 2017-07-11 at 06:31 -0700, Guenter Roeck wrote:

On 07/10/2017 06:56 AM, Andrew Jeffery wrote:

Some PMBus chips, such as the MAX31785, use different coefficients for
FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
or RPM mode. Add a callback to allow the driver to provide the
applicable coefficients to avoid imposing on devices that don't have
this requirement.



Why not just introduce another class, such as PSC_PWM ?


I considered this up front however I wasn't sure where the PMBus sensor
classes were modelled from. The PMBus spec generally doesn't discuss


The classes are modeled from my brain, so we can do whatever we want with them.


PMW beyond the concept of fans, and given PSC_FAN already existed I had
a vague feeling that introducing PSC_PWM might not be the way to go. It
also means that PSC_FAN is implicitly RPM in some circumstances, or
both RPM and PWM in others, and wasn't previously contrasted against
PWM as PWM-specific configuration wasn't an option.

However if it's reasonable it should lead to a more straight forward
patch. I'll rework and resend if it falls out nicely.


Please do.

Thanks,
Guenter


Thanks,

Andrew




Signed-off-by: Andrew Jeffery 

---
   drivers/hwmon/pmbus/pmbus.h  |  18 +--
   drivers/hwmon/pmbus/pmbus_core.c | 112 
---
   2 files changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 927eabc1b273..338ecc8b25a4 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
   enum pmbus_data_format { linear = 0, direct, vid };
   enum vrm_version { vr11 = 0, vr12 };
   
+struct pmbus_coeffs {

+   int m; /* mantissa for direct data format */
+   int b; /* offset */
+   int R; /* exponent */

+};
+
   struct pmbus_driver_info {

int pages;  /* Total number of pages */

enum pmbus_data_format format[PSC_NUM_CLASSES];

@@ -353,9 +359,7 @@ struct pmbus_driver_info {

 * Support one set of coefficients for each sensor type
 * Used for chips providing data in direct mode.
 */

-   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
-   int b[PSC_NUM_CLASSES]; /* offset */
-   int R[PSC_NUM_CLASSES]; /* exponent */

+   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
   

u32 func[PMBUS_PAGES];  /* Functionality, per page */

/*

@@ -382,6 +386,14 @@ struct pmbus_driver_info {

int (*identify)(struct i2c_client *client,
struct pmbus_driver_info *info);
   

+   /*
+* If a fan's coefficents change over time (e.g. between RPM and PWM
+* mode), then the driver can provide a function for retrieving the
+* currently applicable coefficients.
+*/
+   const struct pmbus_coeffs *(*get_fan_coeffs)(
+   const struct pmbus_driver_info *info, int index,
+   enum pmbus_fan_mode mode, int command);
/* Allow the driver to interpret the fan command value */
int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
int (*set_pwm_mode)(int id, long mode, u8 *fan_config,

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3b0a55bbbd2c..4ff6a1fd5cce 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -58,10 +58,11 @@
   struct pmbus_sensor {

struct pmbus_sensor *next;

char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */

-   struct device_attribute attribute;
+   struct sensor_device_attribute attribute;

u8 page;/* page number */
u16 reg;/* register */
enum pmbus_sensor_classes class;/* sensor class */

+   const struct pmbus_coeffs *coeffs;

bool update;/* runtime sensor update needed */
int data;   /* Sensor data.

   Negative if there was a read error */

@@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {

u8 id;
u8 config;
u16 command;
+   const struct pmbus_coeffs *coeffs;

   };
   #define to_pmbus_fan_ctrl_attr(_attr) \

container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)

@@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,

long val = (s16) sensor->data;
long m, b, R;
   

-   m = data->info->m[sensor->class];
-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b =

Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-11 Thread Guenter Roeck 

On 07/11/2017 06:20 PM, Andrew Jeffery wrote:

On Tue, 2017-07-11 at 06:31 -0700, Guenter Roeck wrote:

On 07/10/2017 06:56 AM, Andrew Jeffery wrote:

Some PMBus chips, such as the MAX31785, use different coefficients for
FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
or RPM mode. Add a callback to allow the driver to provide the
applicable coefficients to avoid imposing on devices that don't have
this requirement.



Why not just introduce another class, such as PSC_PWM ?


I considered this up front however I wasn't sure where the PMBus sensor
classes were modelled from. The PMBus spec generally doesn't discuss


The classes are modeled from my brain, so we can do whatever we want with them.


PMW beyond the concept of fans, and given PSC_FAN already existed I had
a vague feeling that introducing PSC_PWM might not be the way to go. It
also means that PSC_FAN is implicitly RPM in some circumstances, or
both RPM and PWM in others, and wasn't previously contrasted against
PWM as PWM-specific configuration wasn't an option.

However if it's reasonable it should lead to a more straight forward
patch. I'll rework and resend if it falls out nicely.


Please do.

Thanks,
Guenter


Thanks,

Andrew




Signed-off-by: Andrew Jeffery 

---
   drivers/hwmon/pmbus/pmbus.h  |  18 +--
   drivers/hwmon/pmbus/pmbus_core.c | 112 
---
   2 files changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 927eabc1b273..338ecc8b25a4 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
   enum pmbus_data_format { linear = 0, direct, vid };
   enum vrm_version { vr11 = 0, vr12 };
   
+struct pmbus_coeffs {

+   int m; /* mantissa for direct data format */
+   int b; /* offset */
+   int R; /* exponent */

+};
+
   struct pmbus_driver_info {

int pages;  /* Total number of pages */

enum pmbus_data_format format[PSC_NUM_CLASSES];

@@ -353,9 +359,7 @@ struct pmbus_driver_info {

 * Support one set of coefficients for each sensor type
 * Used for chips providing data in direct mode.
 */

-   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
-   int b[PSC_NUM_CLASSES]; /* offset */
-   int R[PSC_NUM_CLASSES]; /* exponent */

+   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
   

u32 func[PMBUS_PAGES];  /* Functionality, per page */

/*

@@ -382,6 +386,14 @@ struct pmbus_driver_info {

int (*identify)(struct i2c_client *client,
struct pmbus_driver_info *info);
   

+   /*
+* If a fan's coefficents change over time (e.g. between RPM and PWM
+* mode), then the driver can provide a function for retrieving the
+* currently applicable coefficients.
+*/
+   const struct pmbus_coeffs *(*get_fan_coeffs)(
+   const struct pmbus_driver_info *info, int index,
+   enum pmbus_fan_mode mode, int command);
/* Allow the driver to interpret the fan command value */
int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
int (*set_pwm_mode)(int id, long mode, u8 *fan_config,

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3b0a55bbbd2c..4ff6a1fd5cce 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -58,10 +58,11 @@
   struct pmbus_sensor {

struct pmbus_sensor *next;

char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */

-   struct device_attribute attribute;
+   struct sensor_device_attribute attribute;

u8 page;/* page number */
u16 reg;/* register */
enum pmbus_sensor_classes class;/* sensor class */

+   const struct pmbus_coeffs *coeffs;

bool update;/* runtime sensor update needed */
int data;   /* Sensor data.

   Negative if there was a read error */

@@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {

u8 id;
u8 config;
u16 command;
+   const struct pmbus_coeffs *coeffs;

   };
   #define to_pmbus_fan_ctrl_attr(_attr) \

container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)

@@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,

long val = (s16) sensor->data;
long m, b, R;
   

-   m = data->info->m[sensor->class];
-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+

Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-11 Thread Andrew Jeffery 
On Tue, 2017-07-11 at 06:31 -0700, Guenter Roeck wrote:
> On 07/10/2017 06:56 AM, Andrew Jeffery wrote:
> > Some PMBus chips, such as the MAX31785, use different coefficients for
> > FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
> > or RPM mode. Add a callback to allow the driver to provide the
> > applicable coefficients to avoid imposing on devices that don't have
> > this requirement.
> > 
> 
> Why not just introduce another class, such as PSC_PWM ?

I considered this up front however I wasn't sure where the PMBus sensor
classes were modelled from. The PMBus spec generally doesn't discuss
PMW beyond the concept of fans, and given PSC_FAN already existed I had
a vague feeling that introducing PSC_PWM might not be the way to go. It
also means that PSC_FAN is implicitly RPM in some circumstances, or
both RPM and PWM in others, and wasn't previously contrasted against
PWM as PWM-specific configuration wasn't an option.

However if it's reasonable it should lead to a more straight forward
patch. I'll rework and resend if it falls out nicely.

Thanks,

Andrew

> 
> > > > Signed-off-by: Andrew Jeffery 
> > ---
> >   drivers/hwmon/pmbus/pmbus.h  |  18 +--
> >   drivers/hwmon/pmbus/pmbus_core.c | 112 
> > ---
> >   2 files changed, 108 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> > index 927eabc1b273..338ecc8b25a4 100644
> > --- a/drivers/hwmon/pmbus/pmbus.h
> > +++ b/drivers/hwmon/pmbus/pmbus.h
> > @@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
> >   enum pmbus_data_format { linear = 0, direct, vid };
> >   enum vrm_version { vr11 = 0, vr12 };
> >   
> > +struct pmbus_coeffs {
> > > > +   int m; /* mantissa for direct data format */
> > > > +   int b; /* offset */
> > > > +   int R; /* exponent */
> > +};
> > +
> >   struct pmbus_driver_info {
> > > > > >     int pages;  /* Total number of pages */
> > > >     enum pmbus_data_format format[PSC_NUM_CLASSES];
> > @@ -353,9 +359,7 @@ struct pmbus_driver_info {
> > > >      * Support one set of coefficients for each sensor type
> > > >      * Used for chips providing data in direct mode.
> > > >      */
> > > > > > -   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
> > > > > > -   int b[PSC_NUM_CLASSES]; /* offset */
> > > > > > -   int R[PSC_NUM_CLASSES]; /* exponent */
> > > > +   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
> >   
> > > > > >     u32 func[PMBUS_PAGES];  /* Functionality, per page */
> > > >     /*
> > @@ -382,6 +386,14 @@ struct pmbus_driver_info {
> > > >     int (*identify)(struct i2c_client *client,
> > > >     struct pmbus_driver_info *info);
> >   
> > > > +   /*
> > > > +    * If a fan's coefficents change over time (e.g. between RPM 
> > > > and PWM
> > > > +    * mode), then the driver can provide a function for retrieving 
> > > > the
> > > > +    * currently applicable coefficients.
> > > > +    */
> > > > +   const struct pmbus_coeffs *(*get_fan_coeffs)(
> > > > +   const struct pmbus_driver_info *info, int index,
> > > > +   enum pmbus_fan_mode mode, int command);
> > > >     /* Allow the driver to interpret the fan command value */
> > > >     int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
> > > >     int (*set_pwm_mode)(int id, long mode, u8 *fan_config,
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c 
> > b/drivers/hwmon/pmbus/pmbus_core.c
> > index 3b0a55bbbd2c..4ff6a1fd5cce 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -58,10 +58,11 @@
> >   struct pmbus_sensor {
> > > >     struct pmbus_sensor *next;
> > > > > >     char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */
> > > > -   struct device_attribute attribute;
> > > > +   struct sensor_device_attribute attribute;
> > > > > >     u8 page;/* page number */
> > > > > >     u16 reg;/* register */
> > > > > >     enum pmbus_sensor_classes class;/* sensor class */
> > > > +   const struct pmbus_coeffs *coeffs;
> > > > > >     bool update;/* runtime sensor update needed */
> > > > > >     int data;   /* Sensor data.
> > > >        Negative if there was a read error */
> > @@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {
> > > >     u8 id;
> > > >     u8 config;
> > > >     u16 command;
> > > > +   const struct pmbus_coeffs *coeffs;
> >   };
> >   #define to_pmbus_fan_ctrl_attr(_attr) \
> > > >     container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)
> > @@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data 
> > *data,
> > > >     long val = (s16) sensor->data;
> > > >     long m, b, R;
> >   
> > > > -   m = data->info->m[sensor->class];
>

Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-11 Thread Andrew Jeffery 
On Tue, 2017-07-11 at 06:31 -0700, Guenter Roeck wrote:
> On 07/10/2017 06:56 AM, Andrew Jeffery wrote:
> > Some PMBus chips, such as the MAX31785, use different coefficients for
> > FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
> > or RPM mode. Add a callback to allow the driver to provide the
> > applicable coefficients to avoid imposing on devices that don't have
> > this requirement.
> > 
> 
> Why not just introduce another class, such as PSC_PWM ?

I considered this up front however I wasn't sure where the PMBus sensor
classes were modelled from. The PMBus spec generally doesn't discuss
PMW beyond the concept of fans, and given PSC_FAN already existed I had
a vague feeling that introducing PSC_PWM might not be the way to go. It
also means that PSC_FAN is implicitly RPM in some circumstances, or
both RPM and PWM in others, and wasn't previously contrasted against
PWM as PWM-specific configuration wasn't an option.

However if it's reasonable it should lead to a more straight forward
patch. I'll rework and resend if it falls out nicely.

Thanks,

Andrew

> 
> > > > Signed-off-by: Andrew Jeffery 
> > ---
> >   drivers/hwmon/pmbus/pmbus.h  |  18 +--
> >   drivers/hwmon/pmbus/pmbus_core.c | 112 
> > ---
> >   2 files changed, 108 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> > index 927eabc1b273..338ecc8b25a4 100644
> > --- a/drivers/hwmon/pmbus/pmbus.h
> > +++ b/drivers/hwmon/pmbus/pmbus.h
> > @@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
> >   enum pmbus_data_format { linear = 0, direct, vid };
> >   enum vrm_version { vr11 = 0, vr12 };
> >   
> > +struct pmbus_coeffs {
> > > > +   int m; /* mantissa for direct data format */
> > > > +   int b; /* offset */
> > > > +   int R; /* exponent */
> > +};
> > +
> >   struct pmbus_driver_info {
> > > > > >     int pages;  /* Total number of pages */
> > > >     enum pmbus_data_format format[PSC_NUM_CLASSES];
> > @@ -353,9 +359,7 @@ struct pmbus_driver_info {
> > > >      * Support one set of coefficients for each sensor type
> > > >      * Used for chips providing data in direct mode.
> > > >      */
> > > > > > -   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
> > > > > > -   int b[PSC_NUM_CLASSES]; /* offset */
> > > > > > -   int R[PSC_NUM_CLASSES]; /* exponent */
> > > > +   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
> >   
> > > > > >     u32 func[PMBUS_PAGES];  /* Functionality, per page */
> > > >     /*
> > @@ -382,6 +386,14 @@ struct pmbus_driver_info {
> > > >     int (*identify)(struct i2c_client *client,
> > > >     struct pmbus_driver_info *info);
> >   
> > > > +   /*
> > > > +    * If a fan's coefficents change over time (e.g. between RPM 
> > > > and PWM
> > > > +    * mode), then the driver can provide a function for retrieving 
> > > > the
> > > > +    * currently applicable coefficients.
> > > > +    */
> > > > +   const struct pmbus_coeffs *(*get_fan_coeffs)(
> > > > +   const struct pmbus_driver_info *info, int index,
> > > > +   enum pmbus_fan_mode mode, int command);
> > > >     /* Allow the driver to interpret the fan command value */
> > > >     int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
> > > >     int (*set_pwm_mode)(int id, long mode, u8 *fan_config,
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c 
> > b/drivers/hwmon/pmbus/pmbus_core.c
> > index 3b0a55bbbd2c..4ff6a1fd5cce 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -58,10 +58,11 @@
> >   struct pmbus_sensor {
> > > >     struct pmbus_sensor *next;
> > > > > >     char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */
> > > > -   struct device_attribute attribute;
> > > > +   struct sensor_device_attribute attribute;
> > > > > >     u8 page;/* page number */
> > > > > >     u16 reg;/* register */
> > > > > >     enum pmbus_sensor_classes class;/* sensor class */
> > > > +   const struct pmbus_coeffs *coeffs;
> > > > > >     bool update;/* runtime sensor update needed */
> > > > > >     int data;   /* Sensor data.
> > > >        Negative if there was a read error */
> > @@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {
> > > >     u8 id;
> > > >     u8 config;
> > > >     u16 command;
> > > > +   const struct pmbus_coeffs *coeffs;
> >   };
> >   #define to_pmbus_fan_ctrl_attr(_attr) \
> > > >     container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)
> > @@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data 
> > *data,
> > > >     long val = (s16) sensor->data;
> > > >     long m, b, R;
> >   
> > > > -   m = data->info->m[sensor->class];
> > > > -   b

Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-11 Thread Guenter Roeck 

On 07/10/2017 06:56 AM, Andrew Jeffery wrote:

Some PMBus chips, such as the MAX31785, use different coefficients for
FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
or RPM mode. Add a callback to allow the driver to provide the
applicable coefficients to avoid imposing on devices that don't have
this requirement.



Why not just introduce another class, such as PSC_PWM ?


Signed-off-by: Andrew Jeffery 
---
  drivers/hwmon/pmbus/pmbus.h  |  18 +--
  drivers/hwmon/pmbus/pmbus_core.c | 112 ---
  2 files changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 927eabc1b273..338ecc8b25a4 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
  enum pmbus_data_format { linear = 0, direct, vid };
  enum vrm_version { vr11 = 0, vr12 };
  
+struct pmbus_coeffs {

+   int m; /* mantissa for direct data format */
+   int b; /* offset */
+   int R; /* exponent */
+};
+
  struct pmbus_driver_info {
int pages;  /* Total number of pages */
enum pmbus_data_format format[PSC_NUM_CLASSES];
@@ -353,9 +359,7 @@ struct pmbus_driver_info {
 * Support one set of coefficients for each sensor type
 * Used for chips providing data in direct mode.
 */
-   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
-   int b[PSC_NUM_CLASSES]; /* offset */
-   int R[PSC_NUM_CLASSES]; /* exponent */
+   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
  
  	u32 func[PMBUS_PAGES];	/* Functionality, per page */

/*
@@ -382,6 +386,14 @@ struct pmbus_driver_info {
int (*identify)(struct i2c_client *client,
struct pmbus_driver_info *info);
  
+	/*

+* If a fan's coefficents change over time (e.g. between RPM and PWM
+* mode), then the driver can provide a function for retrieving the
+* currently applicable coefficients.
+*/
+   const struct pmbus_coeffs *(*get_fan_coeffs)(
+   const struct pmbus_driver_info *info, int index,
+   enum pmbus_fan_mode mode, int command);
/* Allow the driver to interpret the fan command value */
int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
int (*set_pwm_mode)(int id, long mode, u8 *fan_config,
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3b0a55bbbd2c..4ff6a1fd5cce 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -58,10 +58,11 @@
  struct pmbus_sensor {
struct pmbus_sensor *next;
char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */
-   struct device_attribute attribute;
+   struct sensor_device_attribute attribute;
u8 page;/* page number */
u16 reg;/* register */
enum pmbus_sensor_classes class;/* sensor class */
+   const struct pmbus_coeffs *coeffs;
bool update;/* runtime sensor update needed */
int data;   /* Sensor data.
   Negative if there was a read error */
@@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {
u8 id;
u8 config;
u16 command;
+   const struct pmbus_coeffs *coeffs;
  };
  #define to_pmbus_fan_ctrl_attr(_attr) \
container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)
@@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
long val = (s16) sensor->data;
long m, b, R;
  
-	m = data->info->m[sensor->class];

-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
  
  	if (m == 0)

return 0;
@@ -663,9 +671,15 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
  {
long m, b, R;
  
-	m = data->info->m[sensor->class];

-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
  
  	/* Power is in uW. Adjust R and b. */

if (sensor->class == PSC_POWER) {
@@ -796,7 +810,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,
 struct

Re: [RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-11 Thread Guenter Roeck 

On 07/10/2017 06:56 AM, Andrew Jeffery wrote:

Some PMBus chips, such as the MAX31785, use different coefficients for
FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
or RPM mode. Add a callback to allow the driver to provide the
applicable coefficients to avoid imposing on devices that don't have
this requirement.



Why not just introduce another class, such as PSC_PWM ?


Signed-off-by: Andrew Jeffery 
---
  drivers/hwmon/pmbus/pmbus.h  |  18 +--
  drivers/hwmon/pmbus/pmbus_core.c | 112 ---
  2 files changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 927eabc1b273..338ecc8b25a4 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
  enum pmbus_data_format { linear = 0, direct, vid };
  enum vrm_version { vr11 = 0, vr12 };
  
+struct pmbus_coeffs {

+   int m; /* mantissa for direct data format */
+   int b; /* offset */
+   int R; /* exponent */
+};
+
  struct pmbus_driver_info {
int pages;  /* Total number of pages */
enum pmbus_data_format format[PSC_NUM_CLASSES];
@@ -353,9 +359,7 @@ struct pmbus_driver_info {
 * Support one set of coefficients for each sensor type
 * Used for chips providing data in direct mode.
 */
-   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
-   int b[PSC_NUM_CLASSES]; /* offset */
-   int R[PSC_NUM_CLASSES]; /* exponent */
+   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
  
  	u32 func[PMBUS_PAGES];	/* Functionality, per page */

/*
@@ -382,6 +386,14 @@ struct pmbus_driver_info {
int (*identify)(struct i2c_client *client,
struct pmbus_driver_info *info);
  
+	/*

+* If a fan's coefficents change over time (e.g. between RPM and PWM
+* mode), then the driver can provide a function for retrieving the
+* currently applicable coefficients.
+*/
+   const struct pmbus_coeffs *(*get_fan_coeffs)(
+   const struct pmbus_driver_info *info, int index,
+   enum pmbus_fan_mode mode, int command);
/* Allow the driver to interpret the fan command value */
int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
int (*set_pwm_mode)(int id, long mode, u8 *fan_config,
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3b0a55bbbd2c..4ff6a1fd5cce 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -58,10 +58,11 @@
  struct pmbus_sensor {
struct pmbus_sensor *next;
char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */
-   struct device_attribute attribute;
+   struct sensor_device_attribute attribute;
u8 page;/* page number */
u16 reg;/* register */
enum pmbus_sensor_classes class;/* sensor class */
+   const struct pmbus_coeffs *coeffs;
bool update;/* runtime sensor update needed */
int data;   /* Sensor data.
   Negative if there was a read error */
@@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {
u8 id;
u8 config;
u16 command;
+   const struct pmbus_coeffs *coeffs;
  };
  #define to_pmbus_fan_ctrl_attr(_attr) \
container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)
@@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
long val = (s16) sensor->data;
long m, b, R;
  
-	m = data->info->m[sensor->class];

-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
  
  	if (m == 0)

return 0;
@@ -663,9 +671,15 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
  {
long m, b, R;
  
-	m = data->info->m[sensor->class];

-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
  
  	/* Power is in uW. Adjust R and b. */

if (sensor->class == PSC_POWER) {
@@ -796,7 +810,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,
 struct device_attribute

[RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-10 Thread Andrew Jeffery 
Some PMBus chips, such as the MAX31785, use different coefficients for
FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
or RPM mode. Add a callback to allow the driver to provide the
applicable coefficients to avoid imposing on devices that don't have
this requirement.

Signed-off-by: Andrew Jeffery 
---
 drivers/hwmon/pmbus/pmbus.h  |  18 +--
 drivers/hwmon/pmbus/pmbus_core.c | 112 ---
 2 files changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 927eabc1b273..338ecc8b25a4 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
 enum pmbus_data_format { linear = 0, direct, vid };
 enum vrm_version { vr11 = 0, vr12 };
 
+struct pmbus_coeffs {
+   int m; /* mantissa for direct data format */
+   int b; /* offset */
+   int R; /* exponent */
+};
+
 struct pmbus_driver_info {
int pages;  /* Total number of pages */
enum pmbus_data_format format[PSC_NUM_CLASSES];
@@ -353,9 +359,7 @@ struct pmbus_driver_info {
 * Support one set of coefficients for each sensor type
 * Used for chips providing data in direct mode.
 */
-   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
-   int b[PSC_NUM_CLASSES]; /* offset */
-   int R[PSC_NUM_CLASSES]; /* exponent */
+   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
 
u32 func[PMBUS_PAGES];  /* Functionality, per page */
/*
@@ -382,6 +386,14 @@ struct pmbus_driver_info {
int (*identify)(struct i2c_client *client,
struct pmbus_driver_info *info);
 
+   /*
+* If a fan's coefficents change over time (e.g. between RPM and PWM
+* mode), then the driver can provide a function for retrieving the
+* currently applicable coefficients.
+*/
+   const struct pmbus_coeffs *(*get_fan_coeffs)(
+   const struct pmbus_driver_info *info, int index,
+   enum pmbus_fan_mode mode, int command);
/* Allow the driver to interpret the fan command value */
int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
int (*set_pwm_mode)(int id, long mode, u8 *fan_config,
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3b0a55bbbd2c..4ff6a1fd5cce 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -58,10 +58,11 @@
 struct pmbus_sensor {
struct pmbus_sensor *next;
char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */
-   struct device_attribute attribute;
+   struct sensor_device_attribute attribute;
u8 page;/* page number */
u16 reg;/* register */
enum pmbus_sensor_classes class;/* sensor class */
+   const struct pmbus_coeffs *coeffs;
bool update;/* runtime sensor update needed */
int data;   /* Sensor data.
   Negative if there was a read error */
@@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {
u8 id;
u8 config;
u16 command;
+   const struct pmbus_coeffs *coeffs;
 };
 #define to_pmbus_fan_ctrl_attr(_attr) \
container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)
@@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
long val = (s16) sensor->data;
long m, b, R;
 
-   m = data->info->m[sensor->class];
-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
 
if (m == 0)
return 0;
@@ -663,9 +671,15 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
 {
long m, b, R;
 
-   m = data->info->m[sensor->class];
-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
 
/* Power is in uW. Adjust R and b. */
if (sensor->class == PSC_POWER) {
@@ -796,7 +810,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,
 struct device_attribute *devattr, char *buf)
 {
struct pmbus_data *data = pmbus_update_device(dev);
-

[RFC PATCH 3/4] pmbus: Allow dynamic fan coefficient values

2017-07-10 Thread Andrew Jeffery 
Some PMBus chips, such as the MAX31785, use different coefficients for
FAN_COMMAND_[1-4] depending on whether the fan is in PWM (percent duty)
or RPM mode. Add a callback to allow the driver to provide the
applicable coefficients to avoid imposing on devices that don't have
this requirement.

Signed-off-by: Andrew Jeffery 
---
 drivers/hwmon/pmbus/pmbus.h  |  18 +--
 drivers/hwmon/pmbus/pmbus_core.c | 112 ---
 2 files changed, 108 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 927eabc1b273..338ecc8b25a4 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -345,6 +345,12 @@ enum pmbus_sensor_classes {
 enum pmbus_data_format { linear = 0, direct, vid };
 enum vrm_version { vr11 = 0, vr12 };
 
+struct pmbus_coeffs {
+   int m; /* mantissa for direct data format */
+   int b; /* offset */
+   int R; /* exponent */
+};
+
 struct pmbus_driver_info {
int pages;  /* Total number of pages */
enum pmbus_data_format format[PSC_NUM_CLASSES];
@@ -353,9 +359,7 @@ struct pmbus_driver_info {
 * Support one set of coefficients for each sensor type
 * Used for chips providing data in direct mode.
 */
-   int m[PSC_NUM_CLASSES]; /* mantissa for direct data format */
-   int b[PSC_NUM_CLASSES]; /* offset */
-   int R[PSC_NUM_CLASSES]; /* exponent */
+   struct pmbus_coeffs coeffs[PSC_NUM_CLASSES];
 
u32 func[PMBUS_PAGES];  /* Functionality, per page */
/*
@@ -382,6 +386,14 @@ struct pmbus_driver_info {
int (*identify)(struct i2c_client *client,
struct pmbus_driver_info *info);
 
+   /*
+* If a fan's coefficents change over time (e.g. between RPM and PWM
+* mode), then the driver can provide a function for retrieving the
+* currently applicable coefficients.
+*/
+   const struct pmbus_coeffs *(*get_fan_coeffs)(
+   const struct pmbus_driver_info *info, int index,
+   enum pmbus_fan_mode mode, int command);
/* Allow the driver to interpret the fan command value */
int (*get_pwm_mode)(int id, u8 fan_config, u16 fan_command);
int (*set_pwm_mode)(int id, long mode, u8 *fan_config,
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3b0a55bbbd2c..4ff6a1fd5cce 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -58,10 +58,11 @@
 struct pmbus_sensor {
struct pmbus_sensor *next;
char name[PMBUS_NAME_SIZE]; /* sysfs sensor name */
-   struct device_attribute attribute;
+   struct sensor_device_attribute attribute;
u8 page;/* page number */
u16 reg;/* register */
enum pmbus_sensor_classes class;/* sensor class */
+   const struct pmbus_coeffs *coeffs;
bool update;/* runtime sensor update needed */
int data;   /* Sensor data.
   Negative if there was a read error */
@@ -89,6 +90,7 @@ struct pmbus_fan_ctrl {
u8 id;
u8 config;
u16 command;
+   const struct pmbus_coeffs *coeffs;
 };
 #define to_pmbus_fan_ctrl_attr(_attr) \
container_of(_attr, struct pmbus_fan_ctrl_attr, attribute)
@@ -511,9 +513,15 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
long val = (s16) sensor->data;
long m, b, R;
 
-   m = data->info->m[sensor->class];
-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
 
if (m == 0)
return 0;
@@ -663,9 +671,15 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
 {
long m, b, R;
 
-   m = data->info->m[sensor->class];
-   b = data->info->b[sensor->class];
-   R = data->info->R[sensor->class];
+   if (sensor->coeffs) {
+   m = sensor->coeffs->m;
+   b = sensor->coeffs->b;
+   R = sensor->coeffs->R;
+   } else {
+   m = data->info->coeffs[sensor->class].m;
+   b = data->info->coeffs[sensor->class].b;
+   R = data->info->coeffs[sensor->class].R;
+   }
 
/* Power is in uW. Adjust R and b. */
if (sensor->class == PSC_POWER) {
@@ -796,7 +810,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,
 struct device_attribute *devattr, char *buf)
 {
struct pmbus_data *data = pmbus_update_device(dev);
-   struct