Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions Documentation/hwmon/max34440.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ Supported chips:

Datasheet: -

* ADI ADPM12200

Prefixes: 'adpm12200'

Addresses scanned: -

Datasheet: -

* Maxim MAX34440

Prefixes: 'max34440'
Expand Down Expand Up @@ -79,10 +87,11 @@ This driver supports multiple devices: hardware monitoring for Maxim MAX34440
PMBus 6-Channel Power-Supply Manager, MAX34441 PMBus 5-Channel Power-Supply
Manager and Intelligent Fan Controller, and MAX34446 PMBus Power-Supply Data
Logger; PMBus Voltage Monitor and Sequencers for MAX34451, MAX34460, and
MAX34461; PMBus DC/DC Power Module ADPM12160. The MAX34451 supports monitoring
voltage or current of 12 channels based on GIN pins. The MAX34460 supports 12
voltage channels, and the MAX34461 supports 16 voltage channels. The ADPM1260
also monitors both input and output of voltage and current.
MAX34461; PMBus DC/DC Power Module ADPM12160, and ADPM12200. The MAX34451
supports monitoring voltage or current of 12 channels based on GIN pins. The
MAX34460 supports 12 voltage channels, and the MAX34461 supports 16 voltage
channels. The ADPM12160, and ADPM12200 also monitors both input and output
of voltage and current.

The driver is a client driver to the core PMBus driver. Please see
Documentation/hwmon/pmbus.rst for details on PMBus client drivers.
Expand Down Expand Up @@ -140,7 +149,8 @@ in[1-6]_reset_history Write any value to reset history.
.. note::

- MAX34446 only supports in[1-4].
- ADPM12160 only supports in[1-2]. Label is "vin1" and "vout1" respectively.
- ADPM12160, and ADPM12200 only supports in[1-2]. Label is "vin1"
and "vout1" respectively.

Curr
~~~~
Expand All @@ -162,7 +172,8 @@ curr[1-6]_reset_history Write any value to reset history.

- in6 and curr6 attributes only exist for MAX34440.
- MAX34446 only supports curr[1-4].
- For ADPM12160, curr[1] is "iin1" and curr[2-6] are "iout[1-5].
- For ADPM12160, and ADPM12200, curr[1] is "iin1" and curr[2-6]
are "iout[1-5].

Power
~~~~~
Expand Down Expand Up @@ -198,7 +209,7 @@ temp[1-8]_reset_history Write any value to reset history.
.. note::
- temp7 and temp8 attributes only exist for MAX34440.
- MAX34446 only supports temp[1-3].
- ADPM12160 only supports temp[1].
- ADPM12160, and ADPM12200 only supports temp[1].


.. note::
Expand Down
2 changes: 1 addition & 1 deletion drivers/hwmon/pmbus/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ config SENSORS_MAX34440
help
If you say yes here you get hardware monitoring support for Maxim
MAX34440, MAX34441, MAX34446, MAX34451, MAX34460, and MAX34461.
Other compatible includes ADPM12160.
Other compatible are ADPM12160, and ADPM12200.

This driver can also be built as a module. If so, the module will
be called max34440.
Expand Down
56 changes: 47 additions & 9 deletions drivers/hwmon/pmbus/max34440.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

enum chips {
adpm12160,
adpm12200,
max34440,
max34441,
max34446,
Expand Down Expand Up @@ -89,7 +90,7 @@ static int max34440_read_word_data(struct i2c_client *client, int page,
break;
case PMBUS_VIRT_READ_IOUT_AVG:
if (data->id != max34446 && data->id != max34451 &&
data->id != adpm12160)
data->id != adpm12160 && data->id != adpm12200)
return -ENXIO;
ret = pmbus_read_word_data(client, page, phase,
MAX34446_MFR_IOUT_AVG);
Expand Down Expand Up @@ -174,7 +175,7 @@ static int max34440_write_word_data(struct i2c_client *client, int page,
ret = pmbus_write_word_data(client, page,
MAX34440_MFR_IOUT_PEAK, 0);
if (!ret && (data->id == max34446 || data->id == max34451 ||
data->id == adpm12160))
data->id == adpm12160 || data->id == adpm12200))
ret = pmbus_write_word_data(client, page,
MAX34446_MFR_IOUT_AVG, 0);

Expand Down Expand Up @@ -326,18 +327,18 @@ static struct pmbus_driver_info max34440_info[] = {
.format[PSC_CURRENT_IN] = direct,
.format[PSC_CURRENT_OUT] = direct,
.format[PSC_TEMPERATURE] = direct,
.m[PSC_VOLTAGE_IN] = 1,
.m[PSC_VOLTAGE_IN] = 125,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this is questionable. What about devices running old firmware? Do we have any way (from HW registers) to get this info from the device and tweak this at runtime depending on the running FW?

Also, take care about the git subject. Not according to the subsystem style

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated git subject for commit 1

I am aware of the practice of supporting released products but I was told this hasn't actually released and will release with the latest FW I based this fix on.

short history:
they planned to release the driver+part at the same time. I was allowed to publicly push this even if that specific time it wasn't public yet. I even gave the upstream maintainers a copy of the datasheet to convince them to accept the patch of an unreleased part (with permission from adpm12160 owners)

I suppose this got delayed until eventually updating the fw from fw12 to fw14

I believe this kind of permanent fix is ok.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me, but you need to send these patches upstream and explain the change with what you just told me and I would also recommend to add a link (in your cover letter) to the patches where you first added the drivers so they can remember it was not yet a public part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will apply suggestions to cover letter
btw is this good/ok for upstream?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"This driver has permission to be released ahead of the part."

Not sure if the above adds much. But sure, go ahead and send it upstream

.b[PSC_VOLTAGE_IN] = 0,
.R[PSC_VOLTAGE_IN] = 0,
.m[PSC_VOLTAGE_OUT] = 1,
.m[PSC_VOLTAGE_OUT] = 125,
.b[PSC_VOLTAGE_OUT] = 0,
.R[PSC_VOLTAGE_OUT] = 0,
.m[PSC_CURRENT_IN] = 1,
.m[PSC_CURRENT_IN] = 250,
.b[PSC_CURRENT_IN] = 0,
.R[PSC_CURRENT_IN] = 2,
.m[PSC_CURRENT_OUT] = 1,
.R[PSC_CURRENT_IN] = -1,
.m[PSC_CURRENT_OUT] = 250,
.b[PSC_CURRENT_OUT] = 0,
.R[PSC_CURRENT_OUT] = 2,
.R[PSC_CURRENT_OUT] = -1,
.m[PSC_TEMPERATURE] = 1,
.b[PSC_TEMPERATURE] = 0,
.R[PSC_TEMPERATURE] = 2,
Expand All @@ -354,6 +355,42 @@ static struct pmbus_driver_info max34440_info[] = {
.read_word_data = max34440_read_word_data,
.write_word_data = max34440_write_word_data,
},
[adpm12200] = {
.pages = 19,
.format[PSC_VOLTAGE_IN] = direct,
.format[PSC_VOLTAGE_OUT] = direct,
.format[PSC_CURRENT_IN] = direct,
.format[PSC_CURRENT_OUT] = direct,
.format[PSC_TEMPERATURE] = direct,
.m[PSC_VOLTAGE_IN] = 125,
.b[PSC_VOLTAGE_IN] = 0,
.R[PSC_VOLTAGE_IN] = 0,
.m[PSC_VOLTAGE_OUT] = 125,
.b[PSC_VOLTAGE_OUT] = 0,
.R[PSC_VOLTAGE_OUT] = 0,
.m[PSC_CURRENT_IN] = 250,
.b[PSC_CURRENT_IN] = 0,
.R[PSC_CURRENT_IN] = -1,
.m[PSC_CURRENT_OUT] = 250,
.b[PSC_CURRENT_OUT] = 0,
.R[PSC_CURRENT_OUT] = -1,
.m[PSC_TEMPERATURE] = 1,
.b[PSC_TEMPERATURE] = 0,
.R[PSC_TEMPERATURE] = 2,
/* absent func below [18] are not for monitoring */
.func[2] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT,
.func[4] = PMBUS_HAVE_STATUS_IOUT,
.func[5] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT,
.func[6] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT,
.func[7] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT,
.func[8] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT,
.func[9] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT,
.func[10] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT,
.func[14] = PMBUS_HAVE_IOUT,
.func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
.read_word_data = max34440_read_word_data,
.write_word_data = max34440_write_word_data,
},
[max34440] = {
.pages = 14,
.format[PSC_VOLTAGE_IN] = direct,
Expand Down Expand Up @@ -584,7 +621,7 @@ static int max34440_probe(struct i2c_client *client)
rv = max34451_set_supported_funcs(client, data);
if (rv)
return rv;
} else if (data->id == adpm12160) {
} else if (data->id == adpm12160 || data->id == adpm12200) {
data->iout_oc_fault_limit = PMBUS_IOUT_OC_FAULT_LIMIT;
data->iout_oc_warn_limit = PMBUS_IOUT_OC_WARN_LIMIT;
}
Expand All @@ -594,6 +631,7 @@ static int max34440_probe(struct i2c_client *client)

static const struct i2c_device_id max34440_id[] = {
{"adpm12160", adpm12160},
{"adpm12200", adpm12200},
{"max34440", max34440},
{"max34441", max34441},
{"max34446", max34446},
Expand Down