Config max31785 based on devtree attrs
v2 cover Config max31785 based on devtree attrs
Add optional property for max31785 in device tree to indicate that fan PWM should be set to 100% on fault. Driver references this property during initialization time to set fault response appropriately.
----
Changes in v2:
- Set fault response for fans on pages 0-3 was previously only setting page 0.
Christopher Bostic (3):
drivers/hwmon: Document optional dev tree property 'fault-max-fan' arch/arm/boot/dts: Add optional property for max31785 drivers/hwmon: max31785 Set fan fault response based on dev tree
Documentation/devicetree/bindings/hwmon/max31785.txt | 20 ++++++++++++++++++++ arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 + drivers/hwmon/max31785.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+)
Signature · 1 line
1.8.2.2
PATCH v2 1/3 drivers/hwmon: Document optional dev tree property 'fault-max-fan'
Document new optional device tree property 'fault-max-fan'. This property indicates that the max31785 should be configured to drive fan with 100% PWM duty cycle on fault condition.
--- Documentation/devicetree/bindings/hwmon/max31785.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/max31785.txt
Documentation/devicetree/bindings/hwmon/max31785.txt new file +20 -0
@@ -0,0 +1,20 @@ +Bindings for MAX31785 I2C fan controller + +Reference: +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf + +Required properties: +- compatible : "maxim,max31785" +- reg : I2C address, 0x52. + +Optional properties, default is to retain the chip's current setting: +- fault-max-fan : When present this property indicates to set fan to 100% PWM + duty cycle on FAULT condition. + +Example: + + max31785@52 { + compatible = "maxim,max31785"; + reg = <0x52>; + fault-max-fan; + };
Signature · 1 line
1.8.2.2
Joel Stanley
maintainerRe: [PATCH linux dev-4.10 v2 1/3] drivers/hwmon: Document optional dev tree property 'fault-max-fan'
Quoted 18 lines · Christopher Bostic
Document new optional device tree property 'fault-max-fan'. This property indicates that the max31785 should be configured to drive fan with 100% PWM duty cycle on fault condition.
Signed-off-by Christopher Bostic <[email protected]>--- Documentation/devicetree/bindings/hwmon/max31785.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/max31785.txtDocumentation/devicetree/bindings/hwmon/max31785.txt new file +2
-0@@ -0,0 +1,20 @@ +Bindings for MAX31785 I2C fan controller +
Documentation/devicetree/bindings/hwmon/max31785.txt line 12
Quoted 10 lines
Documentation/devicetree/bindings/hwmon/max31785.txt new file +10
-0+Reference: +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf + +Required properties: +- compatible : "maxim,max31785" +- reg : I2C address, 0x52. + +Optional properties, default is to retain the chip's current setting: +- fault-max-fan : When present this property indicates to set fan to 100% PWM + duty cycle on FAULT condition.
The hardware has 6 pages that can be configured to assert this on fault. We should describe that in the binding.
Should the binding describe the four pages as separate properties, or just the one?
One option is to say: we are describing the presence of the wire that connects the fault pin to something, so the presence/absence of the hardware is all that will live in the binding?
Cheers,
Joel
Quoted 11 lines
Documentation/devicetree/bindings/hwmon/max31785.txt new file +7
-1+Example: + + max31785@52 { + compatible = "maxim,max31785"; + reg = <0x52>; + fault-max-fan; + }; --1.8.2.2
PATCH v2 2/3 arch/arm/boot/dts: Add optional property for max31785
In reply to Christopher BosticAdd new optional property 'fault-max-fan'. Indicates that the max31785 should be configured to set 100% PWM fan duty cycle on fault condition.
--- arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 1 + 1 file changed, 1 insertion(+)
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts +1 -0
@@ -196,6 +196,7 @@ max31785@52 { compatible = "maxim,max31785"; reg = <0x52>; + fault-max-fan; }; dps310@76 {
Signature · 1 line
1.8.2.2
PATCH v2 3/3 drivers/hwmon: max31785 Set fan fault response based on dev tree
In reply to Christopher BosticCheck for the optional device tree property 'fault-max-fan'. If present, configure hardware for 100% PWM fan duty cycle on fault condition.
--- drivers/hwmon/max31785.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
drivers/hwmon/max31785.c +38 -0
@@ -38,6 +38,7 @@#define MAX31785_REG_MFR_ID 0x99#define MAX31785_REG_MFR_MODEL 0x9a#define MAX31785_REG_MFR_REVISION 0x9b +#define MAX31785_REG_MFR_FAULT_RESP 0xd9#define MAX31785_REG_MFR_FAN_CONFIG 0xf1#define MAX31785_REG_READ_FAN_PWM 0xf3
@@ -51,6 +52,9 @@/* Fan Status register bits */ #define MAX31785_FAN_STATUS_FAULT_MASK 0x80 +/* Fault response register bits */ +#define MAX31785_FAULT_RESP_PIN_MONITOR 0x01 +/* Fan Command constants */ #define MAX31785_FAN_COMMAND_PWM_RATIO 40
@@ -762,6 +766,36 @@ static int max31785_get_capabilities(struct max31785 *data)return 0 ; } +static int max31785_init_fault_resp (struct i2c_client *client) +{ +struct device_node *np = client->dev.of_node; +int page; +int rc; + +if (np &&of_get_property (np,"fault-max-fan" ,NULL )) { +/* TODO: number of fans/pages is platform dependent */ +for (page =0 ; page <4 ; page++) { + +/* set max fans on fault */ + rc =max31785_set_page (client, page); +if (rc <0 ) +return rc; + + rc =i2c_smbus_read_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP ); +if (rc <0 ) +return rc; + + rc |=MAX31785_FAULT_RESP_PIN_MONITOR ; + rc =i2c_smbus_write_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP , rc); + } +return rc; + } + +return 0 ; +} +static int max31785_probe (struct i2c_client *client,const struct i2c_device_id *id) {
@@ -783,6 +817,10 @@ static int max31785_probe(struct i2c_client *client, data->client = client;mutex_init (&data->lock); + rc =max31785_init_fault_resp (client); +if (rc) +return rc; + rc =max31785_init_fans (data);if (rc)return rc;
Signature · 1 line
1.8.2.2
Joel Stanley
maintainerRe: [PATCH linux dev-4.10 v2 3/3] drivers/hwmon: max31785 Set fan fault response based on dev tree
Quoted 22 lines · Christopher Bostic
Check for the optional device tree property 'fault-max-fan'. If present, configure hardware for 100% PWM fan duty cycle on fault condition.
Signed-off-by Christopher Bostic <[email protected]>--- drivers/hwmon/max31785.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)drivers/hwmon/max31785.c +1
-0@@ -38,6 +38,7 @@#define MAX31785_REG_MFR_ID 0x99#define MAX31785_REG_MFR_MODEL 0x9a#define MAX31785_REG_MFR_REVISION 0x9b +#define MAX31785_REG_MFR_FAULT_RESP 0xd9#define MAX31785_REG_MFR_FAN_CONFIG 0xf1#define MAX31785_REG_READ_FAN_PWM 0xf3
drivers/hwmon/max31785.c line 56
Quoted 5 lines
drivers/hwmon/max31785.c +2
-0@@ -51,6 +52,9 @@/* Fan Status register bits */ #define MAX31785_FAN_STATUS_FAULT_MASK 0x80 +/* Fault response register bits */ +#define MAX31785_FAULT_RESP_PIN_MONITOR 0x01
It's a bit, so use BIT(0).
The bit is called "FAULT_PIN_MONITOR". Please use that naming in your code.
Quoted 7 lines
drivers/hwmon/max31785.c +1
-0+/* Fan Command constants */ #define MAX31785_FAN_COMMAND_PWM_RATIO 40@@ -762,6 +766,36 @@ static int max31785_get_capabilities(struct max31785 *data)return 0 ; }
drivers/hwmon/max31785.c static int max31785_get_capabilities(struct max31785 *data) · line 777
Quoted 10 lines
drivers/hwmon/max31785.c +9
-0+static int max31785_init_fault_resp (struct i2c_client *client) +{ +struct device_node *np = client->dev.of_node; +int page; +int rc; + +if (np &&of_get_property (np,"fault-max-fan" ,NULL )) { +/* TODO: number of fans/pages is platform dependent */ +for (page =0 ; page <4 ; page++) {
The datasheet says "this bit is only valid for pages 0 to 5. Other pages return 0".
Is there a reason you only apply it to the first four pages, and not the fifth (index 4) and sixth (index 5)?
If you intended to apply this for all 6 supported pages, you could set page 255, and have the condition applied for all of the valid pages.
If you only set it for the first four pages on purpose then please add a comment to say that. However, I think we should fix the binding so the pages can be selected. I will review the bindings separately.
Cheers,
Joel
Quoted 41 lines
drivers/hwmon/max31785.c +21
-0+ +/* set max fans on fault */ + rc =max31785_set_page (client, page); +if (rc <0 ) +return rc; + + rc =i2c_smbus_read_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP ); +if (rc <0 ) +return rc; + + rc |=MAX31785_FAULT_RESP_PIN_MONITOR ; + rc =i2c_smbus_write_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP , rc); + } +return rc; + } + +return 0 ; +} +static int max31785_probe(struct
i2c_client*client,
const structi2c_device_id*id)
{drivers/hwmon/max31785.c +4
-1@@ -783,6 +817,10 @@ static int max31785_probe(struct i2c_client *client, data->client = client;mutex_init (&data->lock); + rc =max31785_init_fault_resp (client); +if (rc) +return rc; + rc =max31785_init_fans (data);if (rc)return rc; --1.8.2.2
Re: [PATCH linux dev-4.10 v2 3/3] drivers/hwmon: max31785 Set fan fault response based on dev tree
Quoted 27 lines · Joel Stanley
Quoted 25 lines · Christopher Bostic
Check for the optional device tree property 'fault-max-fan'. If present, configure hardware for 100% PWM fan duty cycle on fault condition.
Signed-off-by Christopher Bostic <[email protected]>--- drivers/hwmon/max31785.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)drivers/hwmon/max31785.c +1
-0@@ -38,6 +38,7 @@#define MAX31785_REG_MFR_ID 0x99#define MAX31785_REG_MFR_MODEL 0x9a#define MAX31785_REG_MFR_REVISION 0x9b +#define MAX31785_REG_MFR_FAULT_RESP 0xd9#define MAX31785_REG_MFR_FAN_CONFIG 0xf1#define MAX31785_REG_READ_FAN_PWM 0xf3@@ -51,6 +52,9 @@/* Fan Status register bits */ #define MAX31785_FAN_STATUS_FAULT_MASK 0x80
Quoted 5 lines
drivers/hwmon/max31785.c line 56
Quoted 2 lines
drivers/hwmon/max31785.c +2
-0+/* Fault response register bits */ +#define MAX31785_FAULT_RESP_PIN_MONITOR 0x01It's a bit, so use BIT(0).
The bit is called "
FAULT_PIN_MONITOR". Please use that naming in your code.
Hi Joel,
OK will make those updates.
Quoted 21 lines
Quoted 8 lines
drivers/hwmon/max31785.c +1
-0+/* Fan Command constants */ #define MAX31785_FAN_COMMAND_PWM_RATIO 40@@ -762,6 +766,36 @@ static int max31785_get_capabilities(struct max31785 *data)return 0 ; }drivers/hwmon/max31785.c static int max31785_get_capabilities(struct max31785 *data) · line 777
Quoted 10 lines
drivers/hwmon/max31785.c +9
-0+static int max31785_init_fault_resp (struct i2c_client *client) +{ +struct device_node *np = client->dev.of_node; +int page; +int rc; + +if (np &&of_get_property (np,"fault-max-fan" ,NULL )) { +/* TODO: number of fans/pages is platform dependent */ +for (page =0 ; page <4 ; page++) {The datasheet says "this bit is only valid for pages 0 to 5. Other pages return 0".
Quoted 2 lines
Is there a reason you only apply it to the first four pages, and not the fifth (index 4) and sixth (index 5)?
Was thinking in terms of Witherspoon which only uses fans 0 - 3. Clearly not a generic solution.
Quoted 3 lines
If you intended to apply this for all 6 supported pages, you could set page 255, and have the condition applied for all of the valid pages.
I'll use this approach.
Thanks,
Chris
Quoted 50 lines
If you only set it for the first four pages on purpose then please add a comment to say that. However, I think we should fix the binding so the pages can be selected. I will review the bindings separately.
Cheers,
Joel
Quoted 40 lines
drivers/hwmon/max31785.c +21
-0+ +/* set max fans on fault */ + rc =max31785_set_page (client, page); +if (rc <0 ) +return rc; + + rc =i2c_smbus_read_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP ); +if (rc <0 ) +return rc; + + rc |=MAX31785_FAULT_RESP_PIN_MONITOR ; + rc =i2c_smbus_write_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP , rc); + } +return rc; + } + +return 0 ; +} + static int max31785_probe(struct i2c_client *client,const struct
i2c_device_id*id)
{drivers/hwmon/max31785.c +4
-1@@ -783,6 +817,10 @@ static int max31785_probe(struct i2c_client *client, data->client = client;mutex_init (&data->lock); + rc =max31785_init_fault_resp (client); +if (rc) +return rc; + rc =max31785_init_fans (data);if (rc)return rc; --1.8.2.2
Re: [PATCH linux dev-4.10 v2 3/3] drivers/hwmon: max31785 Set fan fault response based on dev tree
Quoted 64 lines · Christopher Bostic
Quoted 27 lines · Joel Stanley
Quoted 25 lines · Christopher Bostic
Check for the optional device tree property 'fault-max-fan'. If present, configure hardware for 100% PWM fan duty cycle on fault condition.
Signed-off-by Christopher Bostic <[email protected]>--- drivers/hwmon/max31785.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)drivers/hwmon/max31785.c +1
-0@@ -38,6 +38,7 @@#define MAX31785_REG_MFR_ID 0x99#define MAX31785_REG_MFR_MODEL 0x9a#define MAX31785_REG_MFR_REVISION 0x9b +#define MAX31785_REG_MFR_FAULT_RESP 0xd9#define MAX31785_REG_MFR_FAN_CONFIG 0xf1#define MAX31785_REG_READ_FAN_PWM 0xf3@@ -51,6 +52,9 @@/* Fan Status register bits */ #define MAX31785_FAN_STATUS_FAULT_MASK 0x80Quoted 5 lines
drivers/hwmon/max31785.c line 56
Quoted 2 lines
drivers/hwmon/max31785.c +2
-0+/* Fault response register bits */ +#define MAX31785_FAULT_RESP_PIN_MONITOR 0x01It's a bit, so use BIT(0).
The bit is called "
FAULT_PIN_MONITOR". Please use that naming in your code.Hi Joel,
OK will make those updates.
Quoted 22 lines
Quoted 9 lines
drivers/hwmon/max31785.c +1
-0+/* Fan Command constants */ #define MAX31785_FAN_COMMAND_PWM_RATIO 40@@ -762,6 +766,36 @@ static int max31785_get_capabilities(struct max31785 *data)return 0 ; }drivers/hwmon/max31785.c static int max31785_get_capabilities(struct · line 778
Quoted 10 lines
drivers/hwmon/max31785.c +9
-0+static int max31785_init_fault_resp (struct i2c_client *client) +{ +struct device_node *np = client->dev.of_node; +int page; +int rc; + +if (np &&of_get_property (np,"fault-max-fan" ,NULL )) { +/* TODO: number of fans/pages is platform dependent */ +for (page =0 ; page <4 ; page++) {The datasheet says "this bit is only valid for pages 0 to 5. Other pages return 0".
Quoted 2 lines
Is there a reason you only apply it to the first four pages, and not the fifth (index 4) and sixth (index 5)?
Quoted 8 lines
Was thinking in terms of Witherspoon which only uses fans 0 - 3. Clearly not a generic solution.
Quoted 3 lines
If you intended to apply this for all 6 supported pages, you could set page 255, and have the condition applied for all of the valid pages.
I'll use this approach.
Hi Joel,
Actually, from the spec sheet I have on the max31785 it appears page 255 doesn't allow access to the fault register, 0xd9. My spec version is 19-5703; Rev 3; 8/12
Unless I missed something in a later revision I'll apply the fault mode for all 6 supported pages.
Thanks,
Quoted 53 lines
Chris
Quoted 51 lines
If you only set it for the first four pages on purpose then please add a comment to say that. However, I think we should fix the binding so the pages can be selected. I will review the bindings separately.
Cheers,
Joel
Quoted 41 lines
drivers/hwmon/max31785.c +21
-0+ +/* set max fans on fault */ + rc =max31785_set_page (client, page); +if (rc <0 ) +return rc; + + rc =i2c_smbus_read_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP ); +if (rc <0 ) +return rc; + + rc |=MAX31785_FAULT_RESP_PIN_MONITOR ; + rc =i2c_smbus_write_byte_data (client, +MAX31785_REG_MFR_FAULT_RESP , rc); + } +return rc; + } + +return 0 ; +} +static int max31785_probe(struct
i2c_client*client,
const structi2c_device_id*id)
{drivers/hwmon/max31785.c +4
-0@@ -783,6 +817,10 @@ static int max31785_probe(struct i2c_client *client, data->client = client;mutex_init (&data->lock); + rc =max31785_init_fault_resp (client); +if (rc) +return rc; + rc =max31785_init_fans (data);if (rc)return rc;Signature · 1 line
1.8.2.2