From nobody Thu Apr 9 15:42:03 2026 Received: from mail-dy1-f170.google.com (mail-dy1-f170.google.com [74.125.82.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5FCA12F25E4 for ; Sat, 7 Mar 2026 22:46:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.170 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772923570; cv=none; b=KHbswkXz08gstdcxG1vIJ8F+DfObnTOryfEkWaw5w/SVLpFVVcxPlnq2GNfirRWz2L99vawSg77umHVvYGLBZnAUrJ0F+xiTOh1JyJYutgacoeMbkDCKnQQGo2sx6EPknZLJF/Wc0vy8RtMXNmUvC/NgHmfp5r6puBl19e+11bM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772923570; c=relaxed/simple; bh=oAOlPws8EP8PUhAHfq9ajzPvFqDm8bjO312/SkoWd1I=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=okOAGG15s6HnQVMD06+ywuWXB8jB3ej0sUhXvZOr68QgrTwUfgQ+x7IRH4IjA7x/42zh0LXZqeAwkkoV3YR3e0iW9C75MYFCO03e7mHlAOI66RUDEmm1aKvAerW7DDq5ngjthUNgVNpvbM7ATH0K1xcUdDte/sq6zP1KiJlZH2Y= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=IMNT5WtF; arc=none smtp.client-ip=74.125.82.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="IMNT5WtF" Received: by mail-dy1-f170.google.com with SMTP id 5a478bee46e88-2be4781d2baso3502238eec.0 for ; Sat, 07 Mar 2026 14:46:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772923568; x=1773528368; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=t3pZ4NzWiqslROXZ3lXgUVAHQhP2+7utDLaCs4aywEA=; b=IMNT5WtFrVIO6npwL83041rR0kZn3mJngbWnBoy+dN8/3qLZFExG1+5vVNKyKqpv2T 6eQxT2LpkcbBPbhOMc/65rNg4AxijUk6caTEnR9aKBCmwieVZP3vIU1QaPWZxjS+Ba/d hg37JqnTO41uvfrsoaD/zX2/gYeLyk2U+uSi9LfCYGXzDils38T4l5uYSS3Es9YtXUA2 iyYiN51TF+WdTY34eWrmnfOhdgKFhc1rDxzc8STQqNzuogJ6daBtH654dUCi7/AEE8Ar xBz9h72fqoitjXzzT2o1NOrWthn8XS+FIEjXLq+Oako+I2q5MO4DSm8OV79eoyFYtG+O 7tWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772923568; x=1773528368; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=t3pZ4NzWiqslROXZ3lXgUVAHQhP2+7utDLaCs4aywEA=; b=DutajSfyrH2KGWP7SG+hXXk1UKZaldWpGRQ1mUdE70Q6IB2CstF3QRJCu7/0yuZwsO NMrK6siXTKsQ+U04NL81hDIujWrkJMCVsHsKAWvM4qeAAHFrh2/aZ/Hi1yLJE0JcYiIR MvbhVqxsIDtLNFQjj7TNQlGcNW08marCutBT1AoQHqvfutZDs/xsmvtSzWhYYqvwfM4k wDn8ZHq7BHlKAhrK46ybS7m1wqVOeBq1gtMFk7GronC9TORTMSCABtJueca0XoNO4fjM 7RwNFRBoGNMSsH+lghUavMfsCtAajaJn4AdrPP8O92UMg5Cd22e6JEYYLld2dFLAPVk4 Eg5Q== X-Forwarded-Encrypted: i=1; AJvYcCUoqVEBY4fbr3ZzMxaCML+V18EHUqz8Jk6/BvEmQn4Sr6nNa7HUH94E0nxgoDuI+gekgiv5p+PyzwEFTb0=@vger.kernel.org X-Gm-Message-State: AOJu0YwKbAUJs1pfmOvKhcPbQJVuSDicxYXLIkV7QJA5MpxlEGagZKg1 qVg8pk2RPGooL6ctgrVdx5/GuKB4g9cPZ4wBpk6/0UGrnNzPA11CJIWG X-Gm-Gg: ATEYQzzZ+yUVohFuhBjURwwjO1dmKqZHn3FuF08vPc73lserIMPQCmg9r92b8gqh6hT 3pCvbBTlhmo8plagIHDlXmQ6A1Xj7aGeBgjYXyEWJY/qNSYCtgVCQIV38AFJBRLfnUKitplPwWA gz1HLMLAq/mtGBgD9cm/lFdSGsxHkL47vG1EQNi7Lx5q3GH1aysdqQAGQpJrlk9m5NPG9sUylJD KFO4OSUDFJPEz05tcV4nP3o5ijb1CZrSQtvLhmFhLiDAxKaeDvijBWhVWSn+ubN8IElAEmpO74h yORksUvFopwOdPQpnYZvouV3KfuDTEvDU1pWM4dT7BudDpnhcmQ7ZkC8JWRAlryVlhd+U/SrDLK 2a17Cxl3CXZX/CiyZHeRXov6axUS7xPKUGjzKQyphdvzZr4PT4B0pP6eTjVl3TeFxDBnlDwu/WE LAat6Gxk0PDboRLCTM38J+MVQwwqMFgYXjKc3ad9NrlkF1FxJr/I34peHbsEqdCO3eOUG6dUNzk Kv0Ikl73sHcMI/9nQ== X-Received: by 2002:a05:693c:3104:b0:2b8:26b8:3444 with SMTP id 5a478bee46e88-2be4e065b0fmr2783091eec.19.1772923568412; Sat, 07 Mar 2026 14:46:08 -0800 (PST) Received: from localhost.localdomain (c-67-164-93-214.hsd1.ca.comcast.net. [67.164.93.214]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-2be4f96aec4sm4857344eec.25.2026.03.07.14.46.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 07 Mar 2026 14:46:07 -0800 (PST) From: Sanman Pradhan To: Guenter Roeck Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, Sanman Pradhan Subject: [PATCH 2/2] hwmon: (pmbus/max31785) use access_delay for PMBus-mediated accesses Date: Sat, 7 Mar 2026 14:45:20 -0800 Message-Id: <20260307224517.38316-3-sanman.p211993@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20260307224517.38316-1-sanman.p211993@gmail.com> References: <20260307224517.38316-1-sanman.p211993@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Sanman Pradhan The MAX31785 fan controller occasionally NACKs master transactions if accesses are too tightly spaced. To avoid this, the driver currently enforces a 250us inter-access delay with a private timestamp and wrapper functions around both raw SMBus accesses and PMBus helper paths. Use pmbus_driver_info.access_delay for normal PMBus-mediated accesses instead, and remove the driver-local PMBus read/write wrappers. Keep local delay handling for raw SMBus accesses used before pmbus_do_probe(). Also add explicit delays around the raw i2c_transfer() long-read path, since it bypasses PMBus core timing and still needs to be spaced from surrounding transactions. Update max31785_read_byte_data() so PMBUS_FAN_CONFIG_12 accesses are only remapped for virtual pages, allowing physical-page accesses to fall back to the PMBus core. With that change, use pmbus_update_fan() for fan configuration updates. Also use the delayed raw read helper for MFR_REVISION during probe, drop the unused to_max31785_data() macro, and rename the local variable "virtual" to "vpage". Signed-off-by: Sanman Pradhan --- drivers/hwmon/pmbus/max31785.c | 186 +++++++++++---------------------- 1 file changed, 59 insertions(+), 127 deletions(-) diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c index 50073fe0c5e8..e9c3c36c8663 100644 --- a/drivers/hwmon/pmbus/max31785.c +++ b/drivers/hwmon/pmbus/max31785.c @@ -31,7 +31,6 @@ struct max31785_data { struct pmbus_driver_info info; }; =20 -#define to_max31785_data(x) container_of(x, struct max31785_data, info) =20 /* * MAX31785 Driver Workaround @@ -40,9 +39,8 @@ struct max31785_data { * These issues are not indicated by the device itself, except for occasio= nal * NACK responses during master transactions. No error bits are set in STA= TUS_BYTE. * - * To address this, we introduce a delay of 250us between consecutive acce= sses - * to the fan controller. This delay helps mitigate communication problems= by - * allowing sufficient time between accesses. + * Keep minimal local delay handling for raw pre-probe SMBus accesses. + * Normal PMBus-mediated accesses use pmbus_driver_info.access_delay inste= ad. */ static inline void max31785_wait(const struct max31785_data *data) { @@ -54,89 +52,42 @@ static inline void max31785_wait(const struct max31785_= data *data) } =20 static int max31785_i2c_write_byte_data(struct i2c_client *client, - struct max31785_data *driver_data, - int command, u8 data) + struct max31785_data *data, + int command, u8 value) { int rc; =20 - max31785_wait(driver_data); - rc =3D i2c_smbus_write_byte_data(client, command, data); - driver_data->access =3D ktime_get(); + max31785_wait(data); + rc =3D i2c_smbus_write_byte_data(client, command, value); + data->access =3D ktime_get(); return rc; } =20 static int max31785_i2c_read_word_data(struct i2c_client *client, - struct max31785_data *driver_data, + struct max31785_data *data, int command) { int rc; =20 - max31785_wait(driver_data); + max31785_wait(data); rc =3D i2c_smbus_read_word_data(client, command); - driver_data->access =3D ktime_get(); - return rc; -} - -static int _max31785_read_byte_data(struct i2c_client *client, - struct max31785_data *driver_data, - int page, int command) -{ - int rc; - - max31785_wait(driver_data); - rc =3D pmbus_read_byte_data(client, page, command); - driver_data->access =3D ktime_get(); - return rc; -} - -static int _max31785_write_byte_data(struct i2c_client *client, - struct max31785_data *driver_data, - int page, int command, u16 data) -{ - int rc; - - max31785_wait(driver_data); - rc =3D pmbus_write_byte_data(client, page, command, data); - driver_data->access =3D ktime_get(); - return rc; -} - -static int _max31785_read_word_data(struct i2c_client *client, - struct max31785_data *driver_data, - int page, int phase, int command) -{ - int rc; - - max31785_wait(driver_data); - rc =3D pmbus_read_word_data(client, page, phase, command); - driver_data->access =3D ktime_get(); - return rc; -} - -static int _max31785_write_word_data(struct i2c_client *client, - struct max31785_data *driver_data, - int page, int command, u16 data) -{ - int rc; - - max31785_wait(driver_data); - rc =3D pmbus_write_word_data(client, page, command, data); - driver_data->access =3D ktime_get(); + data->access =3D ktime_get(); return rc; } =20 static int max31785_read_byte_data(struct i2c_client *client, int page, in= t reg) { - const struct pmbus_driver_info *info =3D pmbus_get_driver_info(client); - struct max31785_data *driver_data =3D to_max31785_data(info); =20 switch (reg) { case PMBUS_VOUT_MODE: return -ENOTSUPP; case PMBUS_FAN_CONFIG_12: - return _max31785_read_byte_data(client, driver_data, - page - MAX31785_NR_PAGES, - reg); + if (page < MAX31785_NR_PAGES) + return -ENODATA; + + return pmbus_read_byte_data(client, + page - MAX31785_NR_PAGES, + reg); } =20 return -ENODATA; @@ -178,9 +129,22 @@ static int max31785_read_long_data(struct i2c_client *= client, int page, if (rc < 0) return rc; =20 + /* + * The following raw i2c_transfer() bypasses PMBus core access timing. + * Add an explicit delay before the transfer so it is properly spaced + * from the preceding PMBus transaction. + */ + usleep_range(MAX31785_WAIT_DELAY_US, MAX31785_WAIT_DELAY_US + 50); + rc =3D i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); if (rc < 0) return rc; + /* + * The preceding raw i2c_transfer() bypasses PMBus core access timing. + * Add an explicit delay after the transfer so the next PMBus access + * preserves the required inter-transaction spacing. + */ + usleep_range(MAX31785_WAIT_DELAY_US, MAX31785_WAIT_DELAY_US + 50); =20 *data =3D (rspbuf[0] << (0 * 8)) | (rspbuf[1] << (1 * 8)) | (rspbuf[2] << (2 * 8)) | (rspbuf[3] << (3 * 8)); @@ -203,19 +167,18 @@ static int max31785_get_pwm(struct i2c_client *client= , int page) return rv; } =20 -static int max31785_get_pwm_mode(struct i2c_client *client, - struct max31785_data *driver_data, int page) +static int max31785_get_pwm_mode(struct i2c_client *client, int page) { int config; int command; =20 - config =3D _max31785_read_byte_data(client, driver_data, page, - PMBUS_FAN_CONFIG_12); + config =3D pmbus_read_byte_data(client, page, + PMBUS_FAN_CONFIG_12); if (config < 0) return config; =20 - command =3D _max31785_read_word_data(client, driver_data, page, 0xff, - PMBUS_FAN_COMMAND_1); + command =3D pmbus_read_word_data(client, page, 0xff, + PMBUS_FAN_COMMAND_1); if (command < 0) return command; =20 @@ -233,8 +196,6 @@ static int max31785_get_pwm_mode(struct i2c_client *cli= ent, static int max31785_read_word_data(struct i2c_client *client, int page, int phase, int reg) { - const struct pmbus_driver_info *info =3D pmbus_get_driver_info(client); - struct max31785_data *driver_data =3D to_max31785_data(info); u32 val; int rv; =20 @@ -263,7 +224,7 @@ static int max31785_read_word_data(struct i2c_client *c= lient, int page, rv =3D max31785_get_pwm(client, page); break; case PMBUS_VIRT_PWM_ENABLE_1: - rv =3D max31785_get_pwm_mode(client, driver_data, page); + rv =3D max31785_get_pwm_mode(client, page); break; default: rv =3D -ENODATA; @@ -294,35 +255,7 @@ static inline u32 max31785_scale_pwm(u32 sensor_val) return (sensor_val * 100) / 255; } =20 -static int max31785_update_fan(struct i2c_client *client, - struct max31785_data *driver_data, int page, - u8 config, u8 mask, u16 command) -{ - int from, rv; - u8 to; - - from =3D _max31785_read_byte_data(client, driver_data, page, - PMBUS_FAN_CONFIG_12); - if (from < 0) - return from; - - to =3D (from & ~mask) | (config & mask); - - if (to !=3D from) { - rv =3D _max31785_write_byte_data(client, driver_data, page, - PMBUS_FAN_CONFIG_12, to); - if (rv < 0) - return rv; - } - - rv =3D _max31785_write_word_data(client, driver_data, page, - PMBUS_FAN_COMMAND_1, command); - - return rv; -} - -static int max31785_pwm_enable(struct i2c_client *client, - struct max31785_data *driver_data, int page, +static int max31785_pwm_enable(struct i2c_client *client, int page, u16 word) { int config =3D 0; @@ -351,23 +284,21 @@ static int max31785_pwm_enable(struct i2c_client *cli= ent, return -EINVAL; } =20 - return max31785_update_fan(client, driver_data, page, config, + return pmbus_update_fan(client, page, 0, config, PB_FAN_1_RPM, rate); } =20 static int max31785_write_word_data(struct i2c_client *client, int page, int reg, u16 word) { - const struct pmbus_driver_info *info =3D pmbus_get_driver_info(client); - struct max31785_data *driver_data =3D to_max31785_data(info); =20 switch (reg) { case PMBUS_VIRT_PWM_1: - return max31785_update_fan(client, driver_data, page, 0, - PB_FAN_1_RPM, - max31785_scale_pwm(word)); + return pmbus_update_fan(client, page, 0, 0, + PB_FAN_1_RPM, + max31785_scale_pwm(word)); case PMBUS_VIRT_PWM_ENABLE_1: - return max31785_pwm_enable(client, driver_data, page, word); + return max31785_pwm_enable(client, page, word); default: break; } @@ -391,6 +322,7 @@ static const struct pmbus_driver_info max31785_info =3D= { .read_byte_data =3D max31785_read_byte_data, .read_word_data =3D max31785_read_word_data, .write_byte =3D max31785_write_byte, + .access_delay =3D MAX31785_WAIT_DELAY_US, =20 /* RPM */ .format[PSC_FAN] =3D direct, @@ -438,29 +370,29 @@ static const struct pmbus_driver_info max31785_info = =3D { }; =20 static int max31785_configure_dual_tach(struct i2c_client *client, - struct pmbus_driver_info *info) + struct max31785_data *data) { + struct pmbus_driver_info *info =3D &data->info; int ret; int i; - struct max31785_data *driver_data =3D to_max31785_data(info); =20 for (i =3D 0; i < MAX31785_NR_FAN_PAGES; i++) { - ret =3D max31785_i2c_write_byte_data(client, driver_data, + ret =3D max31785_i2c_write_byte_data(client, data, PMBUS_PAGE, i); if (ret < 0) return ret; =20 - ret =3D max31785_i2c_read_word_data(client, driver_data, + ret =3D max31785_i2c_read_word_data(client, data, MFR_FAN_CONFIG); if (ret < 0) return ret; =20 if (ret & MFR_FAN_CONFIG_DUAL_TACH) { - int virtual =3D MAX31785_NR_PAGES + i; + int vpage =3D MAX31785_NR_PAGES + i; =20 - info->pages =3D virtual + 1; - info->func[virtual] |=3D PMBUS_HAVE_FAN12; - info->func[virtual] |=3D PMBUS_PAGE_VIRTUAL; + info->pages =3D vpage + 1; + info->func[vpage] |=3D PMBUS_HAVE_FAN12; + info->func[vpage] |=3D PMBUS_PAGE_VIRTUAL; } } =20 @@ -471,7 +403,7 @@ static int max31785_probe(struct i2c_client *client) { struct device *dev =3D &client->dev; struct pmbus_driver_info *info; - struct max31785_data *driver_data; + struct max31785_data *data; bool dual_tach =3D false; int ret; =20 @@ -480,20 +412,20 @@ static int max31785_probe(struct i2c_client *client) I2C_FUNC_SMBUS_WORD_DATA)) return -ENODEV; =20 - driver_data =3D devm_kzalloc(dev, sizeof(struct max31785_data), GFP_KERNE= L); - if (!driver_data) + data =3D devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) return -ENOMEM; =20 - info =3D &driver_data->info; - driver_data->access =3D ktime_get(); + data->access =3D ktime_get(); + info =3D &data->info; *info =3D max31785_info; =20 - ret =3D max31785_i2c_write_byte_data(client, driver_data, - PMBUS_PAGE, 255); + ret =3D max31785_i2c_write_byte_data(client, data, + PMBUS_PAGE, 0xff); if (ret < 0) return ret; =20 - ret =3D i2c_smbus_read_word_data(client, MFR_REVISION); + ret =3D max31785_i2c_read_word_data(client, data, MFR_REVISION); if (ret < 0) return ret; =20 @@ -509,7 +441,7 @@ static int max31785_probe(struct i2c_client *client) } =20 if (dual_tach) { - ret =3D max31785_configure_dual_tach(client, info); + ret =3D max31785_configure_dual_tach(client, data); if (ret < 0) return ret; } --=20 2.34.1