From: Corey Minyard <corey@minyard.net>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: "Mika Westerberg" <westeri@kernel.org>,
"Andi Shyti" <andi.shyti@kernel.org>,
"Hans de Goede" <hansg@kernel.org>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Sebastian Reichel" <sre@kernel.org>,
linux-i2c@vger.kernel.org, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
openipmi-developer@lists.sourceforge.net,
platform-driver-x86@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH 6/8] ipmi: ssif: use 4-argument i2c_smbus_read_block_data()
Date: Wed, 28 Oct 2026 14:30:56 -0500 [thread overview]
Message-ID: <auJNcEiVgwCW5D1Y@mail.minyard.net> (raw)
In-Reply-To: <20261024-i2c-smbus-read-block-data-v1-6-19bb97467982@gmail.com>
On Sat, Oct 24, 2026 at 07:48:46PM -0700, Dmitry Torokhov wrote:
> Pass the receive buffer capacity to i2c_smbus_read_block_data() in
> ipmi_ssif_thread() via the i2c_size field, and update ssif_i2c_send()
> callers to pass the receive buffer size.
>
> Update read_response(), do_cmd(), and test_multipart_messages() to
> accept the response buffer length, clamping the transfer size to
> I2C_SMBUS_BLOCK_MAX when reading from the device.
This is in my for-next tree. It passes all tests.
The reuse of the "size" parameter to ssif_i2c_send() is ok. That's not
really what it was originally meant for, it was the transaction type,
but it wasn't used, so all is good.
-corey
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/char/ipmi/ipmi_ssif.c | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> index 2361103c5edc..9c0aebec2946 100644
> --- a/drivers/char/ipmi/ipmi_ssif.c
> +++ b/drivers/char/ipmi/ipmi_ssif.c
> @@ -503,7 +503,7 @@ static int ipmi_ssif_thread(void *data)
> } else {
> result = i2c_smbus_read_block_data(
> ssif_info->client, ssif_info->i2c_command,
> - ssif_info->i2c_data);
> + ssif_info->i2c_size, ssif_info->i2c_data);
> if (result < 0)
> ssif_info->done_handler(ssif_info, result,
> NULL, 0);
> @@ -541,7 +541,7 @@ static void start_get(struct ssif_info *ssif_info)
>
> ssif_i2c_send(ssif_info, msg_done_handler, I2C_SMBUS_READ,
> SSIF_IPMI_RESPONSE,
> - ssif_info->recv, I2C_SMBUS_BLOCK_DATA);
> + ssif_info->recv, sizeof(ssif_info->recv));
> }
>
> static void start_resend(struct ssif_info *ssif_info);
> @@ -669,7 +669,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
>
> ssif_i2c_send(ssif_info, msg_done_handler, I2C_SMBUS_READ,
> SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE,
> - ssif_info->recv, I2C_SMBUS_BLOCK_DATA);
> + ssif_info->recv, sizeof(ssif_info->recv));
> return;
> } else if (ssif_info->multi_pos) {
> /* Middle of multi-part read. Start the next transaction. */
> @@ -736,7 +736,7 @@ static void msg_done_handler(struct ssif_info *ssif_info, int result,
> I2C_SMBUS_READ,
> SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE,
> ssif_info->recv,
> - I2C_SMBUS_BLOCK_DATA);
> + sizeof(ssif_info->recv));
> return;
> }
> }
> @@ -1314,12 +1314,15 @@ static void ssif_remove(struct i2c_client *client)
> kfree(ssif_info);
> }
>
> -static int read_response(struct i2c_client *client, unsigned char *resp)
> +static int read_response(struct i2c_client *client, unsigned char *resp,
> + size_t resp_length)
> {
> int ret = -ENODEV, retry_cnt = SSIF_RECV_RETRIES;
>
> while (retry_cnt > 0) {
> ret = i2c_smbus_read_block_data(client, SSIF_IPMI_RESPONSE,
> + min_t(size_t, resp_length,
> + I2C_SMBUS_BLOCK_MAX),
> resp);
> if (ret > 0)
> break;
> @@ -1333,7 +1336,7 @@ static int read_response(struct i2c_client *client, unsigned char *resp)
> }
>
> static int do_cmd(struct i2c_client *client, int len, unsigned char *msg,
> - int *resp_len, unsigned char *resp)
> + int *resp_len, unsigned char *resp, size_t resp_length)
> {
> int retry_cnt;
> int ret;
> @@ -1350,14 +1353,14 @@ static int do_cmd(struct i2c_client *client, int len, unsigned char *msg,
> return -ENODEV;
> }
>
> - ret = read_response(client, resp);
> + ret = read_response(client, resp, resp_length);
> if (ret > 0) {
> /* Validate that the response is correct. */
> if (ret < 3 ||
> (resp[0] != (msg[0] | (1 << 2))) ||
> (resp[1] != msg[1]))
> ret = -EINVAL;
> - else if (ret > IPMI_MAX_MSG_LENGTH) {
> + else if (ret > resp_length) {
> ret = -E2BIG;
> } else {
> *resp_len = ret;
> @@ -1382,7 +1385,7 @@ static int ssif_detect(struct i2c_client *client, struct i2c_board_info *info)
> /* Do a Get Device ID command, since it is required. */
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_GET_DEVICE_ID_CMD;
> - rv = do_cmd(client, 2, msg, &len, resp);
> + rv = do_cmd(client, 2, msg, &len, resp, IPMI_MAX_MSG_LENGTH);
> if (rv)
> rv = -ENODEV;
> else {
> @@ -1523,7 +1526,7 @@ static int start_multipart_test(struct i2c_client *client,
>
> static void test_multipart_messages(struct i2c_client *client,
> struct ssif_info *ssif_info,
> - unsigned char *resp)
> + unsigned char *resp, size_t resp_length)
> {
> unsigned char msg[65];
> int ret;
> @@ -1572,7 +1575,7 @@ static void test_multipart_messages(struct i2c_client *client,
> 1, msg + 64);
>
> if (!ret)
> - ret = read_response(client, resp);
> + ret = read_response(client, resp, resp_length);
>
> if (ret > 0) {
> /* End transactions work, we are good. */
> @@ -1590,7 +1593,7 @@ static void test_multipart_messages(struct i2c_client *client,
> SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
> 0, msg + 64);
> if (!ret)
> - ret = read_response(client, resp);
> + ret = read_response(client, resp, resp_length);
> if (ret > 0)
> /* Zero-size end parts work, use those. */
> return;
> @@ -1748,7 +1751,7 @@ static int ssif_probe(struct i2c_client *client)
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_GET_SYSTEM_INTERFACE_CAPABILITIES_CMD;
> msg[2] = 0; /* SSIF */
> - rv = do_cmd(client, 3, msg, &len, resp);
> + rv = do_cmd(client, 3, msg, &len, resp, IPMI_MAX_MSG_LENGTH);
> if (!rv && (len >= 3) && (resp[2] == 0)) {
> if (len < 7) {
> if (ssif_dbg_probe)
> @@ -1800,13 +1803,13 @@ static int ssif_probe(struct i2c_client *client)
> ssif_info->supports_pec = 0;
> }
>
> - test_multipart_messages(client, ssif_info, resp);
> + test_multipart_messages(client, ssif_info, resp, IPMI_MAX_MSG_LENGTH);
>
> /* Make sure the NMI timeout is cleared. */
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_CLEAR_MSG_FLAGS_CMD;
> msg[2] = WDT_PRE_TIMEOUT_INT;
> - rv = do_cmd(client, 3, msg, &len, resp);
> + rv = do_cmd(client, 3, msg, &len, resp, IPMI_MAX_MSG_LENGTH);
> if (rv || (len < 3) || (resp[2] != 0))
> dev_warn(&ssif_info->client->dev,
> "Unable to clear message flags: %d %d %2.2x\n",
> @@ -1815,7 +1818,7 @@ static int ssif_probe(struct i2c_client *client)
> /* Attempt to enable the event buffer. */
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_GET_BMC_GLOBAL_ENABLES_CMD;
> - rv = do_cmd(client, 2, msg, &len, resp);
> + rv = do_cmd(client, 2, msg, &len, resp, IPMI_MAX_MSG_LENGTH);
> if (rv || (len < 4) || (resp[2] != 0)) {
> dev_warn(&ssif_info->client->dev,
> "Error getting global enables: %d %d %2.2x\n",
> @@ -1835,7 +1838,7 @@ static int ssif_probe(struct i2c_client *client)
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_SET_BMC_GLOBAL_ENABLES_CMD;
> msg[2] = ssif_info->global_enables | IPMI_BMC_EVT_MSG_BUFF;
> - rv = do_cmd(client, 3, msg, &len, resp);
> + rv = do_cmd(client, 3, msg, &len, resp, IPMI_MAX_MSG_LENGTH);
> if (rv || (len < 2)) {
> dev_warn(&ssif_info->client->dev,
> "Error setting global enables: %d %d %2.2x\n",
> @@ -1857,7 +1860,7 @@ static int ssif_probe(struct i2c_client *client)
> msg[0] = IPMI_NETFN_APP_REQUEST << 2;
> msg[1] = IPMI_SET_BMC_GLOBAL_ENABLES_CMD;
> msg[2] = ssif_info->global_enables | IPMI_BMC_RCV_MSG_INTR;
> - rv = do_cmd(client, 3, msg, &len, resp);
> + rv = do_cmd(client, 3, msg, &len, resp, IPMI_MAX_MSG_LENGTH);
> if (rv || (len < 2)) {
> dev_warn(&ssif_info->client->dev,
> "Error setting global enables: %d %d %2.2x\n",
>
> --
> 2.56.0.257.gf7497c7d27-goog
>
next prev parent reply other threads:[~2026-10-28 19:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-25 2:48 [PATCH 0/8] i2c: convert i2c_smbus_read_block_data() users to pass buffer length Dmitry Torokhov
2026-10-25 2:48 ` [PATCH 1/8] i2c: acpi: use 4-argument i2c_smbus_read_block_data() Dmitry Torokhov
2026-10-26 8:52 ` Mika Westerberg
2026-10-25 2:48 ` [PATCH 2/8] i2c: docs: update i2c_smbus_read_block_data() prototype in writing-clients Dmitry Torokhov
2026-10-28 18:00 ` Andi Shyti
2026-10-25 2:48 ` [PATCH 3/8] Input: cyapa - use 4-argument i2c_smbus_read_block_data() Dmitry Torokhov
2026-10-25 2:48 ` [PATCH 4/8] Input: elan_i2c " Dmitry Torokhov
2026-10-25 2:48 ` [PATCH 5/8] Input: synaptics-rmi4 " Dmitry Torokhov
2026-10-25 2:48 ` [PATCH 6/8] ipmi: ssif: " Dmitry Torokhov
2026-10-28 19:30 ` Corey Minyard [this message]
2026-10-25 2:48 ` [PATCH 7/8] platform/x86: x86-android-tablets: " Dmitry Torokhov
2026-10-26 11:48 ` Hans de Goede
2026-10-25 2:48 ` [PATCH 8/8] power: supply: sbs-battery: " Dmitry Torokhov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=auJNcEiVgwCW5D1Y@mail.minyard.net \
--to=corey@minyard.net \
--cc=andi.shyti@kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=hansg@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=openipmi-developer@lists.sourceforge.net \
--cc=platform-driver-x86@vger.kernel.org \
--cc=sre@kernel.org \
--cc=westeri@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox