On Mon, Jan 25, 2021 at 02:49:35PM -0800, Alex Qiu wrote:
> Nit: Would it be better to specify what P and N mean upfront in the doc?
>
I thought about it, but figured that the inline definition
is more useful. But of course that is a matter of PoV.
Let's see if people complain ...
Thanks a lot for the review!
Guenter
> - Alex Qiu
>
> On Mon, Jan 25, 2021 at 10:53 AM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >
> > The MAX16601 can report the number of populated phases. Use this
> > information to only create sysfs attributes for populated phases.
> >
> > Cc: Alex Qiu <xqiu@xxxxxxxxxx>
> > Cc: Ugur Usug <Ugur.Usug@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> Reviewed-by: Alex Qiu <xqiu@xxxxxxxxxx>
> > ---
> > Documentation/hwmon/max16601.rst | 98 ++++++++++----------------------
> > drivers/hwmon/pmbus/max16601.c | 17 +++++-
> > 2 files changed, 45 insertions(+), 70 deletions(-)
> >
> > diff --git a/Documentation/hwmon/max16601.rst b/Documentation/hwmon/max16601.rst
> > index 346e74674c51..93d25dfa028e 100644
> > --- a/Documentation/hwmon/max16601.rst
> > +++ b/Documentation/hwmon/max16601.rst
> > @@ -60,75 +60,35 @@ curr1_input VCORE input current, derived from duty cycle and output
> > curr1_max Maximum input current.
> > curr1_max_alarm Current high alarm.
> >
> > -curr2_label "iin1.0"
> > -curr2_input VCORE phase 0 input current.
> > -
> > -curr3_label "iin1.1"
> > -curr3_input VCORE phase 1 input current.
> > -
> > -curr4_label "iin1.2"
> > -curr4_input VCORE phase 2 input current.
> > -
> > -curr5_label "iin1.3"
> > -curr5_input VCORE phase 3 input current.
> > -
> > -curr6_label "iin1.4"
> > -curr6_input VCORE phase 4 input current.
> > -
> > -curr7_label "iin1.5"
> > -curr7_input VCORE phase 5 input current.
> > -
> > -curr8_label "iin1.6"
> > -curr8_input VCORE phase 6 input current.
> > -
> > -curr9_label "iin1.7"
> > -curr9_input VCORE phase 7 input current.
> > -
> > -curr10_label "iin2"
> > -curr10_input VCORE input current, derived from sensor element.
> > -
> > -curr11_label "iin3"
> > -curr11_input VSA input current.
> > -
> > -curr12_label "iout1"
> > -curr12_input VCORE output current.
> > -curr12_crit Critical output current.
> > -curr12_crit_alarm Output current critical alarm.
> > -curr12_max Maximum output current.
> > -curr12_max_alarm Output current high alarm.
> > -
> > -curr13_label "iout1.0"
> > -curr13_input VCORE phase 0 output current.
> > -
> > -curr14_label "iout1.1"
> > -curr14_input VCORE phase 1 output current.
> > -
> > -curr15_label "iout1.2"
> > -curr15_input VCORE phase 2 output current.
> > -
> > -curr16_label "iout1.3"
> > -curr16_input VCORE phase 3 output current.
> > -
> > -curr17_label "iout1.4"
> > -curr17_input VCORE phase 4 output current.
> > -
> > -curr18_label "iout1.5"
> > -curr18_input VCORE phase 5 output current.
> > -
> > -curr19_label "iout1.6"
> > -curr19_input VCORE phase 6 output current.
> > -
> > -curr20_label "iout1.7"
> > -curr20_input VCORE phase 7 output current.
> > -
> > -curr21_label "iout3"
> > -curr21_input VSA output current.
> > -curr21_highest Historical maximum VSA output current.
> > -curr21_reset_history Write any value to reset curr21_highest.
> > -curr21_crit Critical output current.
> > -curr21_crit_alarm Output current critical alarm.
> > -curr21_max Maximum output current.
> > -curr21_max_alarm Output current high alarm.
> > +curr[P+2]_label "iin1.P"
> > +curr[P+2]_input VCORE phase P input current.
> > +
> > +curr[N+2]_label "iin2"
> > +curr[N+2]_input VCORE input current, derived from sensor element.
> > + 'N' is the number of enabled/populated phases.
> > +
> > +curr[N+3]_label "iin3"
> > +curr[N+3]_input VSA input current.
> > +
> > +curr[N+4]_label "iout1"
> > +curr[N+4]_input VCORE output current.
> > +curr[N+4]_crit Critical output current.
> > +curr[N+4]_crit_alarm Output current critical alarm.
> > +curr[N+4]_max Maximum output current.
> > +curr[N+4]_max_alarm Output current high alarm.
> > +
> > +curr[N+P+5]_label "iout1.P"
> > +curr[N+P+5]_input VCORE phase P output current.
> > +
> > +curr[2*N+5]_label "iout3"
> > +curr[2*N+5]_input VSA output current.
> > +curr[2*N+5]_highest Historical maximum VSA output current.
> > +curr[2*N+5]_reset_history
> > + Write any value to reset curr21_highest.
> > +curr[2*N+5]_crit Critical output current.
> > +curr[2*N+5]_crit_alarm Output current critical alarm.
> > +curr[2*N+5]_max Maximum output current.
> > +curr[2*N+5]_max_alarm Output current high alarm.
> >
> > power1_label "pin1"
> > power1_input Input power, derived from duty cycle and output current.
> > diff --git a/drivers/hwmon/pmbus/max16601.c b/drivers/hwmon/pmbus/max16601.c
> > index a960b86e72d2..efe6da3bc8d0 100644
> > --- a/drivers/hwmon/pmbus/max16601.c
> > +++ b/drivers/hwmon/pmbus/max16601.c
> > @@ -31,6 +31,7 @@
> >
> > #include "pmbus.h"
> >
> > +#define REG_DEFAULT_NUM_POP 0xc4
> > #define REG_SETPT_DVID 0xd1
> > #define DAC_10MV_MODE BIT(4)
> > #define REG_IOUT_AVG_PK 0xee
> > @@ -40,6 +41,8 @@
> > #define CORE_RAIL_INDICATOR BIT(7)
> > #define REG_PHASE_REPORTING 0xf4
> >
> > +#define MAX16601_NUM_PHASES 8
> > +
> > struct max16601_data {
> > struct pmbus_driver_info info;
> > struct i2c_client *vsa;
> > @@ -195,6 +198,18 @@ static int max16601_identify(struct i2c_client *client,
> > else
> > info->vrm_version[0] = vr12;
> >
> > + reg = i2c_smbus_read_byte_data(client, REG_DEFAULT_NUM_POP);
> > + if (reg < 0)
> > + return reg;
> > +
> > + /*
> > + * If REG_DEFAULT_NUM_POP returns 0, we don't know how many phases
> > + * are populated. Stick with the default in that case.
> > + */
> > + reg &= 0x0f;
> > + if (reg && reg <= MAX16601_NUM_PHASES)
> > + info->phases[0] = reg;
> > +
> > return 0;
> > }
> >
> > @@ -216,7 +231,7 @@ static struct pmbus_driver_info max16601_info = {
> > .func[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT |
> > PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> > PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_PAGE_VIRTUAL,
> > - .phases[0] = 8,
> > + .phases[0] = MAX16601_NUM_PHASES,
> > .pfunc[0] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
> > .pfunc[1] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT,
> > .pfunc[2] = PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP,
> > --
> > 2.17.1
> >
