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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.