Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Message-ID: <ace70782-777f-ab7c-d190-735f5c65a5e4@roeck-us.net>
Date:   Wed, 10 Aug 2022 09:31:21 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2] hwmon: corsair-psu: add reporting of rail mode via
 debugfs
Content-Language: en-US
To:     Wilken Gottwalt <wilken.gottwalt@posteo.net>,
        linux-kernel@vger.kernel.org
Cc:     Jean Delvare <jdelvare@suse.com>, Jonathan Corbet <corbet@lwn.net>,
        linux-hwmon@vger.kernel.org
References: <YvO4cfx12Q9gcmPg@monster.localdomain>
From:   Guenter Roeck <linux@roeck-us.net>
In-Reply-To: <YvO4cfx12Q9gcmPg@monster.localdomain>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/10/22 06:53, Wilken Gottwalt wrote:
> Add reporting if the PSU is running in single or multi rail mode via
> ocpmode debugfs entry. Also update the documentation accordingly.
> 
> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> ---
> Changes in v2:
>    - fixed spelling issues in commit message

You did not address or even provide feedback on my second comment.

Guenter

> ---
>   Documentation/hwmon/corsair-psu.rst |  5 +++--
>   drivers/hwmon/corsair-psu.c         | 21 ++++++++++++++++++++-
>   2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/hwmon/corsair-psu.rst b/Documentation/hwmon/corsair-psu.rst
> index e8378e7a1d8c..c3a76305c587 100644
> --- a/Documentation/hwmon/corsair-psu.rst
> +++ b/Documentation/hwmon/corsair-psu.rst
> @@ -86,8 +86,9 @@ Debugfs entries
>   ---------------
>   
>   =======================	========================================================
> -uptime			Current uptime of the psu
> +ocpmode                 Single or multi rail mode of the PCIe power connectors
> +product                 Product name of the psu
> +uptime			Session uptime of the psu
>   uptime_total		Total uptime of the psu
>   vendor			Vendor name of the psu
> -product			Product name of the psu
>   =======================	========================================================
> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index 14389fd7afb8..9d103613db39 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -71,9 +71,10 @@
>   #define PSU_CMD_RAIL_WATTS	0x96
>   #define PSU_CMD_VEND_STR	0x99
>   #define PSU_CMD_PROD_STR	0x9A
> -#define PSU_CMD_TOTAL_WATTS	0xEE
>   #define PSU_CMD_TOTAL_UPTIME	0xD1
>   #define PSU_CMD_UPTIME		0xD2
> +#define PSU_CMD_OCPMODE		0xD8
> +#define PSU_CMD_TOTAL_WATTS	0xEE
>   #define PSU_CMD_INIT		0xFE
>   
>   #define L_IN_VOLTS		"v_in"
> @@ -268,6 +269,7 @@ static int corsairpsu_get_value(struct corsairpsu_data *priv, u8 cmd, u8 rail, l
>   		break;
>   	case PSU_CMD_TOTAL_UPTIME:
>   	case PSU_CMD_UPTIME:
> +	case PSU_CMD_OCPMODE:
>   		*val = tmp;
>   		break;
>   	default:
> @@ -660,6 +662,22 @@ static int product_show(struct seq_file *seqf, void *unused)
>   }
>   DEFINE_SHOW_ATTRIBUTE(product);
>   
> +static int ocpmode_show(struct seq_file *seqf, void *unused)
> +{
> +	struct corsairpsu_data *priv = seqf->private;
> +	long val;
> +	int ret;
> +
> +	ret = corsairpsu_get_value(priv, PSU_CMD_OCPMODE, 0, &val);
> +	if (ret < 0)
> +		seq_puts(seqf, "N/A\n");
> +	else
> +		seq_printf(seqf, "%s\n", (val == 0x02) ? "multi rail" : "single rail");
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(ocpmode);
> +
>   static void corsairpsu_debugfs_init(struct corsairpsu_data *priv)
>   {
>   	char name[32];
> @@ -671,6 +689,7 @@ static void corsairpsu_debugfs_init(struct corsairpsu_data *priv)
>   	debugfs_create_file("uptime_total", 0444, priv->debugfs, priv, &uptime_total_fops);
>   	debugfs_create_file("vendor", 0444, priv->debugfs, priv, &vendor_fops);
>   	debugfs_create_file("product", 0444, priv->debugfs, priv, &product_fops);
> +	debugfs_create_file("ocpmode", 0444, priv->debugfs, priv, &ocpmode_fops);
>   }
>   
>   #else