All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages
@ 2018-07-30 10:51 Kamlakant Patel
  2018-07-30 12:56 ` Corey Minyard
  0 siblings, 1 reply; 3+ messages in thread
From: Kamlakant Patel @ 2018-07-30 10:51 UTC (permalink / raw)
  To: Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Kamlakant Patel, openipmi-developer, linux-kernel,
	George Cherian, jayachandran.nair

Currently, by setting the xmit_msg_size to 63, the ssif driver never
does a SSIF_MULTI_n_PART, it falls back to only SSIF_MULTI_2_PART.
Due to this all IPMI commands with request size more than 63 bytes
will not work.

As per IPMI spec 2.0 (section 12.15, Table 10), SSIF supports message
length up to 255 bytes. In a multi-part message, the first part must
carry 32 bytes with command 06h. All intermediate ("middle") parts must
carry 32 bytes with command 07h and the number of message data bytes in
the End transaction can range from 1 to 32 bytes with command 08h.

Update ssif max_xmit_msg_size to handle multi-part messages up
to 255 bytes.
Enable command(08h) for End part transaction.

Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
Suggested-by: Robert Richter <robert.richter@cavium.com>
Reported-by: Karthikeyan M <karthikeyanm@amiindia.co.in>
---
 drivers/char/ipmi/ipmi_ssif.c | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 18e4650..ada59b7 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -60,6 +60,7 @@
 #define	SSIF_IPMI_REQUEST			2
 #define	SSIF_IPMI_MULTI_PART_REQUEST_START	6
 #define	SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE	7
+#define	SSIF_IPMI_MULTI_PART_REQUEST_END	8
 #define	SSIF_IPMI_RESPONSE			3
 #define	SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE	9
 
@@ -88,6 +89,8 @@
 #define SSIF_MSG_JIFFIES	((SSIF_MSG_USEC * 1000) / TICK_NSEC)
 #define SSIF_MSG_PART_JIFFIES	((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)
 
+#define SSIF_MAX_MSG_LENGTH	255
+
 enum ssif_intf_state {
 	SSIF_NORMAL,
 	SSIF_GETTING_FLAGS,
@@ -887,28 +890,39 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
 		 */
 		int left;
 		unsigned char *data_to_send;
+		int command;
 
 		ssif_inc_stat(ssif_info, sent_messages_parts);
 
 		left = ssif_info->multi_len - ssif_info->multi_pos;
-		if (left > 32)
-			left = 32;
+
 		/* Length byte. */
-		ssif_info->multi_data[ssif_info->multi_pos] = left;
+		ssif_info->multi_data[ssif_info->multi_pos] =
+						left > 32 ? 32 : left;
 		data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
-		ssif_info->multi_pos += left;
-		if (left < 32)
+		ssif_info->multi_pos += left > 32 ? 32 : left;
+		command = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
+		if (left <= 32) {
 			/*
 			 * Write is finished.  Note that we must end
-			 * with a write of less than 32 bytes to
-			 * complete the transaction, even if it is
-			 * zero bytes.
+			 * with a write up to 32 bytes to complete the
+			 * transaction, even if it is zero bytes.
+			 * The number of message data bytes in the End
+			 * transaction can range from 1 to 32 bytes.
+			 * As per IPMI spec 2.0(section 12.15,Table 12-10),
+			 * the BMC Multi-part Write commands are:
+			 * Start-first part	- 06h
+			 * Middle-part(s)	- 07h
+			 * End-part		- 08h
+			 * Update command for END part transaction.
 			 */
 			ssif_info->multi_data = NULL;
+			command = SSIF_IPMI_MULTI_PART_REQUEST_END;
+		}
 
 		rv = ssif_i2c_send(ssif_info, msg_written_handler,
 				  I2C_SMBUS_WRITE,
-				  SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
+				  command,
 				  data_to_send,
 				  I2C_SMBUS_BLOCK_DATA);
 		if (rv < 0) {
@@ -1499,9 +1513,11 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 			 * start and the next message is always going
 			 * to be 1-31 bytes in length.  Not ideal, but
 			 * it should work.
+			 * As per IPMI spec 2.0, SSIF interface supports
+			 * message size up to 255 bytes.
 			 */
-			if (ssif_info->max_xmit_msg_size > 63)
-				ssif_info->max_xmit_msg_size = 63;
+			if (ssif_info->max_xmit_msg_size > SSIF_MAX_MSG_LENGTH)
+				ssif_info->max_xmit_msg_size = SSIF_MAX_MSG_LENGTH;
 			break;
 
 		default:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages
  2018-07-30 10:51 [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages Kamlakant Patel
@ 2018-07-30 12:56 ` Corey Minyard
  2018-07-31  7:37   ` Kamlakant Patel
  0 siblings, 1 reply; 3+ messages in thread
From: Corey Minyard @ 2018-07-30 12:56 UTC (permalink / raw)
  To: Kamlakant Patel, Arnd Bergmann, Greg Kroah-Hartman
  Cc: openipmi-developer, linux-kernel, George Cherian, jayachandran.nair

On 07/30/2018 05:51 AM, Kamlakant Patel wrote:
> Currently, by setting the xmit_msg_size to 63, the ssif driver never
> does a SSIF_MULTI_n_PART, it falls back to only SSIF_MULTI_2_PART.
> Due to this all IPMI commands with request size more than 63 bytes
> will not work.
>
> As per IPMI spec 2.0 (section 12.15, Table 10), SSIF supports message
> length up to 255 bytes. In a multi-part message, the first part must
> carry 32 bytes with command 06h. All intermediate ("middle") parts must
> carry 32 bytes with command 07h and the number of message data bytes in
> the End transaction can range from 1 to 32 bytes with command 08h.
>
> Update ssif max_xmit_msg_size to handle multi-part messages up
> to 255 bytes.
> Enable command(08h) for End part transaction.

You don't think it's a good idea to test what the BMC is capable of 
doing, per
the patch I sent you earlier?

-corey

> Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
> Suggested-by: Robert Richter <robert.richter@cavium.com>
> Reported-by: Karthikeyan M <karthikeyanm@amiindia.co.in>
> ---
>   drivers/char/ipmi/ipmi_ssif.c | 38 +++++++++++++++++++++++++++-----------
>   1 file changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> index 18e4650..ada59b7 100644
> --- a/drivers/char/ipmi/ipmi_ssif.c
> +++ b/drivers/char/ipmi/ipmi_ssif.c
> @@ -60,6 +60,7 @@
>   #define	SSIF_IPMI_REQUEST			2
>   #define	SSIF_IPMI_MULTI_PART_REQUEST_START	6
>   #define	SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE	7
> +#define	SSIF_IPMI_MULTI_PART_REQUEST_END	8
>   #define	SSIF_IPMI_RESPONSE			3
>   #define	SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE	9
>   
> @@ -88,6 +89,8 @@
>   #define SSIF_MSG_JIFFIES	((SSIF_MSG_USEC * 1000) / TICK_NSEC)
>   #define SSIF_MSG_PART_JIFFIES	((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)
>   
> +#define SSIF_MAX_MSG_LENGTH	255
> +
>   enum ssif_intf_state {
>   	SSIF_NORMAL,
>   	SSIF_GETTING_FLAGS,
> @@ -887,28 +890,39 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
>   		 */
>   		int left;
>   		unsigned char *data_to_send;
> +		int command;
>   
>   		ssif_inc_stat(ssif_info, sent_messages_parts);
>   
>   		left = ssif_info->multi_len - ssif_info->multi_pos;
> -		if (left > 32)
> -			left = 32;
> +
>   		/* Length byte. */
> -		ssif_info->multi_data[ssif_info->multi_pos] = left;
> +		ssif_info->multi_data[ssif_info->multi_pos] =
> +						left > 32 ? 32 : left;
>   		data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
> -		ssif_info->multi_pos += left;
> -		if (left < 32)
> +		ssif_info->multi_pos += left > 32 ? 32 : left;
> +		command = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
> +		if (left <= 32) {
>   			/*
>   			 * Write is finished.  Note that we must end
> -			 * with a write of less than 32 bytes to
> -			 * complete the transaction, even if it is
> -			 * zero bytes.
> +			 * with a write up to 32 bytes to complete the
> +			 * transaction, even if it is zero bytes.
> +			 * The number of message data bytes in the End
> +			 * transaction can range from 1 to 32 bytes.
> +			 * As per IPMI spec 2.0(section 12.15,Table 12-10),
> +			 * the BMC Multi-part Write commands are:
> +			 * Start-first part	- 06h
> +			 * Middle-part(s)	- 07h
> +			 * End-part		- 08h
> +			 * Update command for END part transaction.
>   			 */
>   			ssif_info->multi_data = NULL;
> +			command = SSIF_IPMI_MULTI_PART_REQUEST_END;
> +		}
>   
>   		rv = ssif_i2c_send(ssif_info, msg_written_handler,
>   				  I2C_SMBUS_WRITE,
> -				  SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
> +				  command,
>   				  data_to_send,
>   				  I2C_SMBUS_BLOCK_DATA);
>   		if (rv < 0) {
> @@ -1499,9 +1513,11 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
>   			 * start and the next message is always going
>   			 * to be 1-31 bytes in length.  Not ideal, but
>   			 * it should work.
> +			 * As per IPMI spec 2.0, SSIF interface supports
> +			 * message size up to 255 bytes.
>   			 */
> -			if (ssif_info->max_xmit_msg_size > 63)
> -				ssif_info->max_xmit_msg_size = 63;
> +			if (ssif_info->max_xmit_msg_size > SSIF_MAX_MSG_LENGTH)
> +				ssif_info->max_xmit_msg_size = SSIF_MAX_MSG_LENGTH;
>   			break;
>   
>   		default:



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages
  2018-07-30 12:56 ` Corey Minyard
@ 2018-07-31  7:37   ` Kamlakant Patel
  0 siblings, 0 replies; 3+ messages in thread
From: Kamlakant Patel @ 2018-07-31  7:37 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Kamlakant Patel, Arnd Bergmann, Greg Kroah-Hartman,
	openipmi-developer, linux-kernel, George Cherian,
	jayachandran.nair

On Mon, Jul 30, 2018 at 07:56:02AM -0500, Corey Minyard wrote:
Hi Corey,
> 
> On 07/30/2018 05:51 AM, Kamlakant Patel wrote:
> >Currently, by setting the xmit_msg_size to 63, the ssif driver never
> >does a SSIF_MULTI_n_PART, it falls back to only SSIF_MULTI_2_PART.
> >Due to this all IPMI commands with request size more than 63 bytes
> >will not work.
> >
> >As per IPMI spec 2.0 (section 12.15, Table 10), SSIF supports message
> >length up to 255 bytes. In a multi-part message, the first part must
> >carry 32 bytes with command 06h. All intermediate ("middle") parts must
> >carry 32 bytes with command 07h and the number of message data bytes in
> >the End transaction can range from 1 to 32 bytes with command 08h.
> >
> >Update ssif max_xmit_msg_size to handle multi-part messages up
> >to 255 bytes.
> >Enable command(08h) for End part transaction.
> 
> You don't think it's a good idea to test what the BMC is capable of
> doing, per
> the patch I sent you earlier?
I have not received the patch in unicast to me. If you have sent it to
mailing list I will search and test it. If not, could you please resend
it to me, I will test it and let you know.

Thanks,
Kamlakant Patel
> 
> -corey
> 
> >Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
> >Suggested-by: Robert Richter <robert.richter@cavium.com>
> >Reported-by: Karthikeyan M <karthikeyanm@amiindia.co.in>
> >---
> >  drivers/char/ipmi/ipmi_ssif.c | 38 +++++++++++++++++++++++++++-----------
> >  1 file changed, 27 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
> >index 18e4650..ada59b7 100644
> >--- a/drivers/char/ipmi/ipmi_ssif.c
> >+++ b/drivers/char/ipmi/ipmi_ssif.c
> >@@ -60,6 +60,7 @@
> >  #define     SSIF_IPMI_REQUEST                       2
> >  #define     SSIF_IPMI_MULTI_PART_REQUEST_START      6
> >  #define     SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE     7
> >+#define      SSIF_IPMI_MULTI_PART_REQUEST_END        8
> >  #define     SSIF_IPMI_RESPONSE                      3
> >  #define     SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE    9
> >
> >@@ -88,6 +89,8 @@
> >  #define SSIF_MSG_JIFFIES    ((SSIF_MSG_USEC * 1000) / TICK_NSEC)
> >  #define SSIF_MSG_PART_JIFFIES       ((SSIF_MSG_PART_USEC * 1000) / TICK_NSEC)
> >
> >+#define SSIF_MAX_MSG_LENGTH  255
> >+
> >  enum ssif_intf_state {
> >      SSIF_NORMAL,
> >      SSIF_GETTING_FLAGS,
> >@@ -887,28 +890,39 @@ static void msg_written_handler(struct ssif_info *ssif_info, int result,
> >               */
> >              int left;
> >              unsigned char *data_to_send;
> >+             int command;
> >
> >              ssif_inc_stat(ssif_info, sent_messages_parts);
> >
> >              left = ssif_info->multi_len - ssif_info->multi_pos;
> >-             if (left > 32)
> >-                     left = 32;
> >+
> >              /* Length byte. */
> >-             ssif_info->multi_data[ssif_info->multi_pos] = left;
> >+             ssif_info->multi_data[ssif_info->multi_pos] =
> >+                                             left > 32 ? 32 : left;
> >              data_to_send = ssif_info->multi_data + ssif_info->multi_pos;
> >-             ssif_info->multi_pos += left;
> >-             if (left < 32)
> >+             ssif_info->multi_pos += left > 32 ? 32 : left;
> >+             command = SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE;
> >+             if (left <= 32) {
> >                      /*
> >                       * Write is finished.  Note that we must end
> >-                      * with a write of less than 32 bytes to
> >-                      * complete the transaction, even if it is
> >-                      * zero bytes.
> >+                      * with a write up to 32 bytes to complete the
> >+                      * transaction, even if it is zero bytes.
> >+                      * The number of message data bytes in the End
> >+                      * transaction can range from 1 to 32 bytes.
> >+                      * As per IPMI spec 2.0(section 12.15,Table 12-10),
> >+                      * the BMC Multi-part Write commands are:
> >+                      * Start-first part     - 06h
> >+                      * Middle-part(s)       - 07h
> >+                      * End-part             - 08h
> >+                      * Update command for END part transaction.
> >                       */
> >                      ssif_info->multi_data = NULL;
> >+                     command = SSIF_IPMI_MULTI_PART_REQUEST_END;
> >+             }
> >
> >              rv = ssif_i2c_send(ssif_info, msg_written_handler,
> >                                I2C_SMBUS_WRITE,
> >-                               SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE,
> >+                               command,
> >                                data_to_send,
> >                                I2C_SMBUS_BLOCK_DATA);
> >              if (rv < 0) {
> >@@ -1499,9 +1513,11 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >                       * start and the next message is always going
> >                       * to be 1-31 bytes in length.  Not ideal, but
> >                       * it should work.
> >+                      * As per IPMI spec 2.0, SSIF interface supports
> >+                      * message size up to 255 bytes.
> >                       */
> >-                     if (ssif_info->max_xmit_msg_size > 63)
> >-                             ssif_info->max_xmit_msg_size = 63;
> >+                     if (ssif_info->max_xmit_msg_size > SSIF_MAX_MSG_LENGTH)
> >+                             ssif_info->max_xmit_msg_size = SSIF_MAX_MSG_LENGTH;
> >                      break;
> >
> >              default:
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-07-31  7:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30 10:51 [PATCH v3] ipmi: update ssif max_xmit_msg_size limit for multi-part messages Kamlakant Patel
2018-07-30 12:56 ` Corey Minyard
2018-07-31  7:37   ` Kamlakant Patel

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.