Re: [PATCH] usb: typec: ucsi: Add support for READ_POWER_LEVEL command

["Jayaraman, Venkat" <venkat.jayaraman@linux.intel.com>]

Hi Greg,

On Sun, Jun 29, 2025 10:04:14PM -0700 , Greg KH wrote:
> On Sun, Jun 29, 2025 at 05:15:56PM -0700, Venkat Jayaraman wrote:
>> @@ -89,6 +117,12 @@ void ucsi_debugfs_register(struct ucsi *ucsi)
>>  	ucsi->debugfs->dentry = debugfs_create_dir(dev_name(ucsi->dev), ucsi_debugfs_root);
>>  	debugfs_create_file("command", 0200, ucsi->debugfs->dentry, ucsi, &ucsi_cmd_fops);
>>  	debugfs_create_file("response", 0400, ucsi->debugfs->dentry, ucsi, &ucsi_resp_fops);
>> +	debugfs_create_file("peak_current", 0400,
>> +			    ucsi->debugfs->dentry, ucsi, &ucsi_peak_curr_fops);
>> +	debugfs_create_file("avg_current", 0400,
>> +			    ucsi->debugfs->dentry, ucsi, &ucsi_avg_curr_fops);
>> +	debugfs_create_file("vbus_voltage", 0400,
>> +			    ucsi->debugfs->dentry, ucsi, &ucsi_vbus_volt_fops);
> You can have these all on one line, but not a big deal :)


I can change that in the next revision.


> And what are the units of these files?
>
>> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
>> index 01ce858a1a2b..8e0f0289a11a 100644
>> --- a/drivers/usb/typec/ucsi/ucsi.c
>> +++ b/drivers/usb/typec/ucsi/ucsi.c
>> @@ -1218,9 +1218,11 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>>  	struct ucsi_connector *con = container_of(work, struct ucsi_connector,
>>  						  work);
>>  	struct ucsi *ucsi = con->ucsi;
>> +	u8  curr_scale, volt_scale;
> Odd extra ' '


Will fix this in the next version.


>> +	if (UCSI_CONSTAT(con, PWR_READING_READY_V2_1)) {
>> +		curr_scale = UCSI_CONSTAT(con, CURRENT_SCALE_V2_1);
>> +		volt_scale = UCSI_CONSTAT(con, VOLTAGE_SCALE_V2_1);
>> +
>> +		val = UCSI_CONSTAT(con, PEAK_CURRENT_V2_1);
>> +		con->peak_current = UCSI_CONSTAT_CURR_SCALE_MULT * curr_scale * val;
>> +
>> +		val = UCSI_CONSTAT(con, AVG_CURRENT_V2_1);
>> +		con->avg_current = UCSI_CONSTAT_CURR_SCALE_MULT * curr_scale * val;
>> +
>> +		val = UCSI_CONSTAT(con, VBUS_VOLTAGE_V2_1);
>> +		con->vbus_voltage = UCSI_CONSTAT_VOLT_SCALE_MULT * volt_scale * val;
> These will never overflow, right?

The values (val) are 16-bit max and there should be no overflow concern with u32 fields declared.

>
> thanks,
>
> greg k-h
>