linux-kernel - Re: [PATCH 2/2] hwmon/pmbus: (isl68137) Add multiple-function pin for raa229141a

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <186a5116-da08-4ec3-a410-4e308fb0cf0b@roeck-us.net>
Date: Tue, 16 Dec 2025 01:56:03 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Jeff Lin <jefflin994697@...il.com>
Cc: grantpeltier93@...il.com, karanja99erick@...il.com,
 chiang.brian@...entec.com, krzk@...nel.org, william@...nnington.com,
 tzungbi@...nel.org, thorsten.blum@...ux.dev, ninad@...ux.ibm.com,
 linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] hwmon/pmbus: (isl68137) Add multiple-function pin for
 raa229141a

On 12/16/25 01:16, Jeff Lin wrote:
> In addition to supporting PMBus-based current monitoring, the RAA229141A
> also provides two multifunction pins(PIN44,PIN45) that can be used to
> sense the input and output current of nearby devices.
> 
> Readings from these multifunction pins are not reported using the
> standard PMBus direct or linear data formats. Instead, they must be
> retrieved via the Renesas-specific Dicrect Memory Access(DMA)
> command codes and scaled by a factor of 10.
> 
Is there a public datasheet to explain this ?

 From what I can see in this code, it does not even remotely look like
it is part of the PMBus standard. It looks like very vendor specific
additional current values. Also, multi-function implies that those are,
as the name says, multi-function pins. There needs to be additional
information explaining how they are associated to specific functionality
such as input or output  current. Just declaring that it be so is insufficient.

Guenter

> Signed-off-by: Jeff Lin <jefflin994697@...il.com>
> ---
>   drivers/hwmon/pmbus/isl68137.c | 45 ++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c
> index 97b61836f53a..e60771614941 100644
> --- a/drivers/hwmon/pmbus/isl68137.c
> +++ b/drivers/hwmon/pmbus/isl68137.c
> @@ -178,6 +178,33 @@ static const struct attribute_group *isl68137_attribute_groups[] = {
>   	NULL,
>   };
>   
> +#define RAA_READ_DMA_DATA 0xc5
> +#define RAA_WRITE_DMA_ADDRESS 0xc7
> +
> +/* DMA address for input current in PIN44 and output current in PIN45 */
> +static const unsigned char dma_address_in[] = { 0xD3, 0xE0 };
> +static const unsigned char dma_address_out[] = { 0x42, 0xEE };
> +static int read_val_route_by_dma(struct i2c_client *client, const char *addr)
> +{
> +	int ret;
> +	/* Set up DMA address */
> +	ret = i2c_smbus_write_i2c_block_data(client, RAA_WRITE_DMA_ADDRESS, 2, addr);
> +
> +	if (ret < 0) {
> +		dev_err(&client->dev,
> +			"Set DMA address failed for address 0x%02x 0x%02x\n", addr[0], addr[1]);
> +		return ret;
> +	}
> +	/* Read DMA data */
> +	u8 buf[2];
> +
> +	ret = i2c_smbus_read_i2c_block_data(client, RAA_READ_DMA_DATA, 2, buf);
> +	if (ret < 0)
> +		return ret;
> +	u16 value = ((u16)buf[1]<<8) | buf[0];
> +	return value;
> +};
> +
>   static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
>   				     int phase, int reg)
>   {
> @@ -207,6 +234,12 @@ static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page,
>   			ret = clamp_val(temp, 0, 0xffff);
>   		}
>   		break;
> +	case PMBUS_VIRT_READ_IIN:
> +		ret = read_val_route_by_dma(client, dma_address_in);
> +		break;
> +	case PMBUS_VIRT_READ_IOUT:
> +		ret = read_val_route_by_dma(client, dma_address_out);
> +		break;
>   	default:
>   		ret = -ENODATA;
>   		break;
> @@ -408,6 +441,18 @@ static int isl68137_probe(struct i2c_client *client)
>   		info->format[PSC_CURRENT_OUT] = linear;
>   		info->format[PSC_POWER] = linear;
>   		info->format[PSC_TEMPERATURE] = linear;
> +		info->format[PSC_VIRT_CURRENT_IN] = direct,
> +		info->format[PSC_VIRT_CURRENT_OUT] = direct,
> +		/* DIRECT read format 10mA/LSB */
> +		info->m[PSC_VIRT_CURRENT_IN] = 1,
> +		info->b[PSC_VIRT_CURRENT_IN] = 0,
> +		info->R[PSC_VIRT_CURRENT_IN] = 2,
> +		/* DIRECT read format 10mA/LSB */
> +		info->m[PSC_VIRT_CURRENT_OUT] = 1,
> +		info->b[PSC_VIRT_CURRENT_OUT] = 0,
> +		info->R[PSC_VIRT_CURRENT_OUT] = 2,
> +		info->func[0] |= PMBUS_HAVE_VIRT_IIN;
> +		info->func[0] |= PMBUS_HAVE_VIRT_IOUT;
>   		info->pages = 2;
>   		info->read_word_data = raa_dmpvr2_read_word_data;
>   		info->write_word_data = raa_dmpvr2_write_word_data;