On 12/25/24 01:51, ChiangBrian 江泳緻 TAO wrote:
From: Brian Chiang<chiang.brian@xxxxxxxxxxxx> Modify adm1266 blackbox access flow by only accessing the latest index of data to speed up the access Signed-off-by: Brian Chiang<chiang.brian@xxxxxxxxxxxx>
This patch is not acceptable. There must be any "#if 0" in upstream code. Yes, I know, there are already 1,419 instances of it, but that doesn't make it better. On top of that, changing driver functionality like this is not acceptable either. Add a new nvmem file if there is a desire to only read the last entry, and rework the code to avoid duplication. Alternatively, if the blackbox contents do not change, you could cache its contents locally in the driver to avoid having to read it each time the file is accessed. Either case, your patch is corrupt. Please run it through checkpatch and you'll see what I mean. Guenter
--- drivers/hwmon/pmbus/adm1266.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c index ec5f932fc6f0..d79b2208de86 100644 --- a/drivers/hwmon/pmbus/adm1266.c +++ b/drivers/hwmon/pmbus/adm1266.c @@ -352,6 +352,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff) char index; u8 buf[5]; int ret; + char latest_index = 0; ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO, buf); if (ret < 0) @@ -359,10 +360,16 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff) if (ret != 4) return -EIO; - + latest_index = buf[2]; record_count = buf[3]; - for (index = 0; index < record_count; index++) { + /*get latest index of blackbox data*/ + ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &latest_index, read_buff); + if (ret < 0) + return ret; + return 0; +#if 0 /*comment out the original one , this dump all the blackboxes , very time-consuming*/ + for (index = 0; index < record_count; index++) { ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, read_buff); if (ret < 0) return ret; @@ -374,6 +381,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff) } return 0; +#endif } static int adm1266_nvmem_read(void *priv, unsigned int offset, void *val, size_t bytes)