[PATCH] ipmi: ssif: Add msleep in multipart test

[PATCH] ipmi: ssif: Add msleep in multipart test

[Kunkun Li]

During multipart test, cmd(6,7,8) or cmd(6,7,7) will
be sent continuously.

The pressure test found some BMC systems cannot process
messages in time, resulting in read_response continues to receive
error messages from i2c.
Retry mechanism will takes 10s, and finally set not support
multipart transmit.

So, to work around this，add msleep after sending cmd 6 and
cmd 7 respectively. The problem did not appear again in
pressure test.

Signed-off-by: Kunkun Li <likunkun@bytedance.com>
---
 drivers/char/ipmi/ipmi_ssif.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 20d5af92966d..65841798fafe 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1453,6 +1453,7 @@ static int start_multipart_test(struct i2c_client *client,
 	ret = i2c_smbus_write_block_data(client,
 					 SSIF_IPMI_MULTI_PART_REQUEST_START,
 					 32, msg);
+	msleep(SSIF_MSG_MSEC);
 	if (ret) {
 		retry_cnt--;
 		if (retry_cnt > 0)
@@ -1467,6 +1468,7 @@ static int start_multipart_test(struct i2c_client *client,
 	ret = i2c_smbus_write_block_data(client,
 					 SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
 					 32, msg + 32);
+	msleep(SSIF_MSG_MSEC);
 	if (ret) {
 		dev_err(&client->dev, "Could not write multi-part middle, though the BMC said it could handle it.  Just limit sends to one part.\n");
 		return ret;
-- 
2.11.0

Re: [PATCH] ipmi: ssif: Add msleep in multipart test

[Corey Minyard]

On Tue, Oct 26, 2021 at 10:58:34AM +0800, Kunkun Li wrote:
> During multipart test, cmd(6,7,8) or cmd(6,7,7) will
> be sent continuously.

This is not useful information, we don't have access to your tests, so
this is meaningless to us.

> 
> The pressure test found some BMC systems cannot process
> messages in time, resulting in read_response continues to receive
> error messages from i2c.
> Retry mechanism will takes 10s, and finally set not support
> multipart transmit.
> 
> So, to work around this，add msleep after sending cmd 6 and
> cmd 7 respectively. The problem did not appear again in
> pressure test.

No, you can't slow down everyone because you have one dodgy BMC.  You
need to detect that this is a BMC that has the problem and only do it
for those BMCs.

-corey

> 
> Signed-off-by: Kunkun Li <likunkun@bytedance.com>
> ---
>  drivers/char/ipmi/ipmi_ssif.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> index 20d5af92966d..65841798fafe 100644
> --- a/drivers/char/ipmi/ipmi_ssif.c
> +++ b/drivers/char/ipmi/ipmi_ssif.c
> @@ -1453,6 +1453,7 @@ static int start_multipart_test(struct i2c_client *client,
>  	ret = i2c_smbus_write_block_data(client,
>  					 SSIF_IPMI_MULTI_PART_REQUEST_START,
>  					 32, msg);
> +	msleep(SSIF_MSG_MSEC);
>  	if (ret) {
>  		retry_cnt--;
>  		if (retry_cnt > 0)
> @@ -1467,6 +1468,7 @@ static int start_multipart_test(struct i2c_client *client,
>  	ret = i2c_smbus_write_block_data(client,
>  					 SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
>  					 32, msg + 32);
> +	msleep(SSIF_MSG_MSEC);
>  	if (ret) {
>  		dev_err(&client->dev, "Could not write multi-part middle, though the BMC said it could handle it.  Just limit sends to one part.\n");
>  		return ret;
> -- 
> 2.11.0
> 

Re: [Openipmi-developer] [PATCH] ipmi: ssif: Add msleep in multipart test

[Corey Minyard]

On Tue, Oct 26, 2021 at 11:50:09AM -0500, Corey Minyard wrote:
> On Tue, Oct 26, 2021 at 10:58:34AM +0800, Kunkun Li wrote:
> > During multipart test, cmd(6,7,8) or cmd(6,7,7) will
> > be sent continuously.
> 
> This is not useful information, we don't have access to your tests, so
> this is meaningless to us.

I realized what you meant above.  The "multipart test" is what the ssif
driver does to detect multipart message support.

My suggestion would be to use the results of get device id (manufacturer
and product id) and compare it against a list of of BMCs that behave
badly when the multipart test is done against them.  Then just skip that
test for those BMCs.

I know that adding 40ms to the detection doesn't seem like much, but
every little bit matters here.  It doesn't take a lot of those to really
add up.

-corey

> 
> > 
> > The pressure test found some BMC systems cannot process
> > messages in time, resulting in read_response continues to receive
> > error messages from i2c.
> > Retry mechanism will takes 10s, and finally set not support
> > multipart transmit.
> > 
> > So, to work around this，add msleep after sending cmd 6 and
> > cmd 7 respectively. The problem did not appear again in
> > pressure test.
> 
> No, you can't slow down everyone because you have one dodgy BMC.  You
> need to detect that this is a BMC that has the problem and only do it
> for those BMCs.
> 
> -corey
> 
> > 
> > Signed-off-by: Kunkun Li <likunkun@bytedance.com>
> > ---
> >  drivers/char/ipmi/ipmi_ssif.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> > index 20d5af92966d..65841798fafe 100644
> > --- a/drivers/char/ipmi/ipmi_ssif.c
> > +++ b/drivers/char/ipmi/ipmi_ssif.c
> > @@ -1453,6 +1453,7 @@ static int start_multipart_test(struct i2c_client *client,
> >  	ret = i2c_smbus_write_block_data(client,
> >  					 SSIF_IPMI_MULTI_PART_REQUEST_START,
> >  					 32, msg);
> > +	msleep(SSIF_MSG_MSEC);
> >  	if (ret) {
> >  		retry_cnt--;
> >  		if (retry_cnt > 0)
> > @@ -1467,6 +1468,7 @@ static int start_multipart_test(struct i2c_client *client,
> >  	ret = i2c_smbus_write_block_data(client,
> >  					 SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
> >  					 32, msg + 32);
> > +	msleep(SSIF_MSG_MSEC);
> >  	if (ret) {
> >  		dev_err(&client->dev, "Could not write multi-part middle, though the BMC said it could handle it.  Just limit sends to one part.\n");
> >  		return ret;
> > -- 
> > 2.11.0
> > 
> 
> 
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer