Also updates the documentation accordingly.
Signed-off-by: Wilken Gottwalt <[email protected]>
---
Changes in v2:
- removed cleanup and typo fixes, sticking to feature adding only
---
Documentation/hwmon/corsair-psu.rst | 2 +
drivers/hwmon/corsair-psu.c | 62 ++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
index c389bd21f4f2..fc798c3df1d0 100644
--- a/Documentation/hwmon/corsair-psu.rst
+++ b/Documentation/hwmon/corsair-psu.rst
@@ -69,6 +69,8 @@ power1_input Total power usage
power2_input Power usage of the 12v psu rail
power3_input Power usage of the 5v psu rail
power4_input Power usage of the 3.3v psu rail
+pwm1 PWM value, read only
+pwm1_enable PWM mode, read only
temp1_input Temperature of the psu vrm component
temp1_crit Temperature max cirtical value of the psu vrm component
temp2_input Temperature of the psu case
diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
index dc24c566d08b..2389f605ca16 100644
--- a/drivers/hwmon/corsair-psu.c
+++ b/drivers/hwmon/corsair-psu.c
@@ -58,7 +58,8 @@
#define OCP_MULTI_RAIL 0x02
#define PSU_CMD_SELECT_RAIL 0x00 /* expects length 2 */
-#define PSU_CMD_RAIL_VOLTS_HCRIT 0x40 /* the rest of the commands expect length 3 */
+#define PSU_CMD_FAN_PWM 0x3B /* the rest of the commands expect length 3 */
+#define PSU_CMD_RAIL_VOLTS_HCRIT 0x40
#define PSU_CMD_RAIL_VOLTS_LCRIT 0x44
#define PSU_CMD_RAIL_AMPS_HCRIT 0x46
#define PSU_CMD_TEMP_HCRIT 0x4F
@@ -76,6 +77,7 @@
#define PSU_CMD_UPTIME 0xD2
#define PSU_CMD_OCPMODE 0xD8
#define PSU_CMD_TOTAL_WATTS 0xEE
+#define PSU_CMD_FAN_PWM_ENABLE 0xF0
#define PSU_CMD_INIT 0xFE
#define L_IN_VOLTS "v_in"
@@ -145,6 +147,14 @@ static int corsairpsu_linear11_to_int(const u16 val, const int scale)
return (exp >= 0) ? (result << exp) : (result >> -exp);
}
+/* the micro-controller uses percentage values to control pwm */
+static int corsairpsu_dutycycle_to_pwm(const long dutycycle)
+{
+ const int result = (256 << 16) / 100;
+
+ return (result * dutycycle) >> 16;
+}
+
static int corsairpsu_usb_cmd(struct corsairpsu_data *priv, u8 p0, u8 p1, u8 p2, void *data)
{
unsigned long time;
@@ -264,6 +274,24 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8 rail, l
case PSU_CMD_FAN:
*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
break;
+ case PSU_CMD_FAN_PWM_ENABLE:
+ *val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
+ /*
+ * 0 = automatic mode, means the micro-controller controls the fan using a plan
+ * which can be modified, but changing this plan is not supported by this
+ * driver, the matching PWM mode is automatic fan speed control = PWM 2
+ * 1 = fixed mode, fan runs at a fixed speed represented by a percentage
+ * value 0-100, this matches the PWM manual fan speed control = PWM 1
+ * technically there is no PWM no fan speed control mode, it would be a combination
+ * of 1 at 100%
+ */
+ if (*val == 0)
+ *val = 2;
+ break;
+ case PSU_CMD_FAN_PWM:
+ *val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
+ *val = corsairpsu_dutycycle_to_pwm(*val);
+ break;
case PSU_CMD_RAIL_WATTS:
case PSU_CMD_TOTAL_WATTS:
*val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000000);
@@ -349,6 +377,18 @@ static umode_t corsairpsu_hwmon_fan_is_visible(const struct corsairpsu_data *pri
}
}
+static umode_t corsairpsu_hwmon_pwm_is_visible(const struct corsairpsu_data *priv, u32 attr,
+ int channel)
+{
+ switch (attr) {
+ case hwmon_pwm_input:
+ case hwmon_pwm_enable:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
static umode_t corsairpsu_hwmon_power_is_visible(const struct corsairpsu_data *priv, u32 attr,
int channel)
{
@@ -416,6 +456,8 @@ static umode_t corsairpsu_hwmon_ops_is_visible(const void *data, enum hwmon_sens
return corsairpsu_hwmon_temp_is_visible(priv, attr, channel);
case hwmon_fan:
return corsairpsu_hwmon_fan_is_visible(priv, attr, channel);
+ case hwmon_pwm:
+ return corsairpsu_hwmon_pwm_is_visible(priv, attr, channel);
case hwmon_power:
return corsairpsu_hwmon_power_is_visible(priv, attr, channel);
case hwmon_in:
@@ -447,6 +489,20 @@ static int corsairpsu_hwmon_temp_read(struct corsairpsu_data *priv, u32 attr, in
return err;
}
+static int corsairpsu_hwmon_pwm_read(struct corsairpsu_data *priv, u32 attr, int channel, long *val)
+{
+ switch (attr) {
+ case hwmon_pwm_input:
+ return corsairpsu_get_value(priv, PSU_CMD_FAN_PWM, 0, val);
+ case hwmon_pwm_enable:
+ return corsairpsu_get_value(priv, PSU_CMD_FAN_PWM_ENABLE, 0, val);
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
static int corsairpsu_hwmon_power_read(struct corsairpsu_data *priv, u32 attr, int channel,
long *val)
{
@@ -531,6 +587,8 @@ static int corsairpsu_hwmon_ops_read(struct device *dev, enum hwmon_sensor_types
if (attr == hwmon_fan_input)
return corsairpsu_get_value(priv, PSU_CMD_FAN, 0, val);
return -EOPNOTSUPP;
+ case hwmon_pwm:
+ return corsairpsu_hwmon_pwm_read(priv, attr, channel, val);
case hwmon_power:
return corsairpsu_hwmon_power_read(priv, attr, channel, val);
case hwmon_in:
@@ -579,6 +637,8 @@ static const struct hwmon_channel_info * const corsairpsu_info[] = {
HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT),
HWMON_CHANNEL_INFO(fan,
HWMON_F_INPUT | HWMON_F_LABEL),
+ HWMON_CHANNEL_INFO(pwm,
+ HWMON_PWM_INPUT | HWMON_PWM_ENABLE),
HWMON_CHANNEL_INFO(power,
HWMON_P_INPUT | HWMON_P_LABEL,
HWMON_P_INPUT | HWMON_P_LABEL,
--
2.41.0
On Thu, Jun 22, 2023 at 05:09:28PM +0000, Wilken Gottwalt wrote:
> Also updates the documentation accordingly.
>
> Signed-off-by: Wilken Gottwalt <[email protected]>
> ---
> Changes in v2:
> - removed cleanup and typo fixes, sticking to feature adding only
> ---
Applied to hwmon-next.
Note: The patch description should describe what the patch does,
which isn't just "Also updates the documentation accordingly."
I'll fix that up, but please keep in mind for later.
Thanks,
Guenter
> Documentation/hwmon/corsair-psu.rst | 2 +
> drivers/hwmon/corsair-psu.c | 62 ++++++++++++++++++++++++++++-
> 2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> index c389bd21f4f2..fc798c3df1d0 100644
> --- a/Documentation/hwmon/corsair-psu.rst
> +++ b/Documentation/hwmon/corsair-psu.rst
> @@ -69,6 +69,8 @@ power1_input Total power usage
> power2_input Power usage of the 12v psu rail
> power3_input Power usage of the 5v psu rail
> power4_input Power usage of the 3.3v psu rail
> +pwm1 PWM value, read only
> +pwm1_enable PWM mode, read only
> temp1_input Temperature of the psu vrm component
> temp1_crit Temperature max cirtical value of the psu vrm component
> temp2_input Temperature of the psu case
> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index dc24c566d08b..2389f605ca16 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -58,7 +58,8 @@
> #define OCP_MULTI_RAIL 0x02
>
> #define PSU_CMD_SELECT_RAIL 0x00 /* expects length 2 */
> -#define PSU_CMD_RAIL_VOLTS_HCRIT 0x40 /* the rest of the commands expect length 3 */
> +#define PSU_CMD_FAN_PWM 0x3B /* the rest of the commands expect length 3 */
> +#define PSU_CMD_RAIL_VOLTS_HCRIT 0x40
> #define PSU_CMD_RAIL_VOLTS_LCRIT 0x44
> #define PSU_CMD_RAIL_AMPS_HCRIT 0x46
> #define PSU_CMD_TEMP_HCRIT 0x4F
> @@ -76,6 +77,7 @@
> #define PSU_CMD_UPTIME 0xD2
> #define PSU_CMD_OCPMODE 0xD8
> #define PSU_CMD_TOTAL_WATTS 0xEE
> +#define PSU_CMD_FAN_PWM_ENABLE 0xF0
> #define PSU_CMD_INIT 0xFE
>
> #define L_IN_VOLTS "v_in"
> @@ -145,6 +147,14 @@ static int corsairpsu_linear11_to_int(const u16 val, const int scale)
> return (exp >= 0) ? (result << exp) : (result >> -exp);
> }
>
> +/* the micro-controller uses percentage values to control pwm */
> +static int corsairpsu_dutycycle_to_pwm(const long dutycycle)
> +{
> + const int result = (256 << 16) / 100;
> +
> + return (result * dutycycle) >> 16;
> +}
> +
> static int corsairpsu_usb_cmd(struct corsairpsu_data *priv, u8 p0, u8 p1, u8 p2, void *data)
> {
> unsigned long time;
> @@ -264,6 +274,24 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8 rail, l
> case PSU_CMD_FAN:
> *val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> break;
> + case PSU_CMD_FAN_PWM_ENABLE:
> + *val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> + /*
> + * 0 = automatic mode, means the micro-controller controls the fan using a plan
> + * which can be modified, but changing this plan is not supported by this
> + * driver, the matching PWM mode is automatic fan speed control = PWM 2
> + * 1 = fixed mode, fan runs at a fixed speed represented by a percentage
> + * value 0-100, this matches the PWM manual fan speed control = PWM 1
> + * technically there is no PWM no fan speed control mode, it would be a combination
> + * of 1 at 100%
> + */
> + if (*val == 0)
> + *val = 2;
> + break;
> + case PSU_CMD_FAN_PWM:
> + *val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1);
> + *val = corsairpsu_dutycycle_to_pwm(*val);
> + break;
> case PSU_CMD_RAIL_WATTS:
> case PSU_CMD_TOTAL_WATTS:
> *val = corsairpsu_linear11_to_int(tmp & 0xFFFF, 1000000);
> @@ -349,6 +377,18 @@ static umode_t corsairpsu_hwmon_fan_is_visible(const struct corsairpsu_data *pri
> }
> }
>
> +static umode_t corsairpsu_hwmon_pwm_is_visible(const struct corsairpsu_data *priv, u32 attr,
> + int channel)
> +{
> + switch (attr) {
> + case hwmon_pwm_input:
> + case hwmon_pwm_enable:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> static umode_t corsairpsu_hwmon_power_is_visible(const struct corsairpsu_data *priv, u32 attr,
> int channel)
> {
> @@ -416,6 +456,8 @@ static umode_t corsairpsu_hwmon_ops_is_visible(const void *data, enum hwmon_sens
> return corsairpsu_hwmon_temp_is_visible(priv, attr, channel);
> case hwmon_fan:
> return corsairpsu_hwmon_fan_is_visible(priv, attr, channel);
> + case hwmon_pwm:
> + return corsairpsu_hwmon_pwm_is_visible(priv, attr, channel);
> case hwmon_power:
> return corsairpsu_hwmon_power_is_visible(priv, attr, channel);
> case hwmon_in:
> @@ -447,6 +489,20 @@ static int corsairpsu_hwmon_temp_read(struct corsairpsu_data *priv, u32 attr, in
> return err;
> }
>
> +static int corsairpsu_hwmon_pwm_read(struct corsairpsu_data *priv, u32 attr, int channel, long *val)
> +{
> + switch (attr) {
> + case hwmon_pwm_input:
> + return corsairpsu_get_value(priv, PSU_CMD_FAN_PWM, 0, val);
> + case hwmon_pwm_enable:
> + return corsairpsu_get_value(priv, PSU_CMD_FAN_PWM_ENABLE, 0, val);
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> static int corsairpsu_hwmon_power_read(struct corsairpsu_data *priv, u32 attr, int channel,
> long *val)
> {
> @@ -531,6 +587,8 @@ static int corsairpsu_hwmon_ops_read(struct device *dev, enum hwmon_sensor_types
> if (attr == hwmon_fan_input)
> return corsairpsu_get_value(priv, PSU_CMD_FAN, 0, val);
> return -EOPNOTSUPP;
> + case hwmon_pwm:
> + return corsairpsu_hwmon_pwm_read(priv, attr, channel, val);
> case hwmon_power:
> return corsairpsu_hwmon_power_read(priv, attr, channel, val);
> case hwmon_in:
> @@ -579,6 +637,8 @@ static const struct hwmon_channel_info * const corsairpsu_info[] = {
> HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_CRIT),
> HWMON_CHANNEL_INFO(fan,
> HWMON_F_INPUT | HWMON_F_LABEL),
> + HWMON_CHANNEL_INFO(pwm,
> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE),
> HWMON_CHANNEL_INFO(power,
> HWMON_P_INPUT | HWMON_P_LABEL,
> HWMON_P_INPUT | HWMON_P_LABEL,
On Thu, 22 Jun 2023 10:42:31 -0700
Guenter Roeck <[email protected]> wrote:
> On Thu, Jun 22, 2023 at 05:09:28PM +0000, Wilken Gottwalt wrote:
> > Also updates the documentation accordingly.
> >
> > Signed-off-by: Wilken Gottwalt <[email protected]>
> > ---
> > Changes in v2:
> > - removed cleanup and typo fixes, sticking to feature adding only
> > ---
>
> Applied to hwmon-next.
>
> Note: The patch description should describe what the patch does,
> which isn't just "Also updates the documentation accordingly."
> I'll fix that up, but please keep in mind for later.
Thank you. So what is the best practice if the title tells it all? I'm
always a bit uncertain what to add to the body if there is nothing more.
Just repeat it?
greetings,
Will
On 6/22/23 10:51, Wilken Gottwalt wrote:
> On Thu, 22 Jun 2023 10:42:31 -0700
> Guenter Roeck <[email protected]> wrote:
>
>> On Thu, Jun 22, 2023 at 05:09:28PM +0000, Wilken Gottwalt wrote:
>>> Also updates the documentation accordingly.
>>>
>>> Signed-off-by: Wilken Gottwalt <[email protected]>
>>> ---
>>> Changes in v2:
>>> - removed cleanup and typo fixes, sticking to feature adding only
>>> ---
>>
>> Applied to hwmon-next.
>>
>> Note: The patch description should describe what the patch does,
>> which isn't just "Also updates the documentation accordingly."
>> I'll fix that up, but please keep in mind for later.
>
> Thank you. So what is the best practice if the title tells it all? I'm
> always a bit uncertain what to add to the body if there is nothing more.
> Just repeat it?
>
Pretty much yes. Plus added prosa if you like.
Guenter