All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] net: ipa: QMI fixes
@ 2021-03-15 15:21 Alex Elder
  2021-03-15 15:21 ` [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value Alex Elder
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Alex Elder @ 2021-03-15 15:21 UTC (permalink / raw)
  To: davem, kuba
  Cc: manivannan.sadhasivam, bjorn.andersson, evgreen, cpratapa, elder,
	netdev, linux-kernel

Mani Sadhasivam discovered some errors in the definitions of some
QMI messages used for IPA.  This series addresses those errors,
and extends the definition of one message type to include some
newly-defined fields.

					-Alex

Alex Elder (3):
  net: ipa: fix a duplicated tlv_type value
  net: ipa: fix another QMI message definition
  net: ipa: extend the INDICATION_REGISTER request

 drivers/net/ipa/ipa_qmi_msg.c | 78 +++++++++++++++++++++++++++++++----
 drivers/net/ipa/ipa_qmi_msg.h |  6 ++-
 2 files changed, 74 insertions(+), 10 deletions(-)

-- 
2.27.0


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

* [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value
  2021-03-15 15:21 [PATCH net-next 0/3] net: ipa: QMI fixes Alex Elder
@ 2021-03-15 15:21 ` Alex Elder
  2021-03-16  3:37   ` Manivannan Sadhasivam
  2021-03-15 15:21 ` [PATCH net-next 2/3] net: ipa: fix another QMI message definition Alex Elder
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2021-03-15 15:21 UTC (permalink / raw)
  To: davem, kuba
  Cc: manivannan.sadhasivam, bjorn.andersson, evgreen, cpratapa, elder,
	netdev, linux-kernel

In the ipa_indication_register_req_ei[] encoding array, the tlv_type
associated with the ipa_mhi_ready_ind field is wrong.  It duplicates
the value used for the data_usage_quota_reached field (0x11) and
should use value 0x12 instead.  Fix this bug.

Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_qmi_msg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipa/ipa_qmi_msg.c b/drivers/net/ipa/ipa_qmi_msg.c
index 73413371e3d3e..e00f829a783f6 100644
--- a/drivers/net/ipa/ipa_qmi_msg.c
+++ b/drivers/net/ipa/ipa_qmi_msg.c
@@ -56,7 +56,7 @@ struct qmi_elem_info ipa_indication_register_req_ei[] = {
 		.elem_size	=
 			sizeof_field(struct ipa_indication_register_req,
 				     ipa_mhi_ready_ind_valid),
-		.tlv_type	= 0x11,
+		.tlv_type	= 0x12,
 		.offset		= offsetof(struct ipa_indication_register_req,
 					   ipa_mhi_ready_ind_valid),
 	},
@@ -66,7 +66,7 @@ struct qmi_elem_info ipa_indication_register_req_ei[] = {
 		.elem_size	=
 			sizeof_field(struct ipa_indication_register_req,
 				     ipa_mhi_ready_ind),
-		.tlv_type	= 0x11,
+		.tlv_type	= 0x12,
 		.offset		= offsetof(struct ipa_indication_register_req,
 					   ipa_mhi_ready_ind),
 	},
-- 
2.27.0


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

* [PATCH net-next 2/3] net: ipa: fix another QMI message definition
  2021-03-15 15:21 [PATCH net-next 0/3] net: ipa: QMI fixes Alex Elder
  2021-03-15 15:21 ` [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value Alex Elder
@ 2021-03-15 15:21 ` Alex Elder
  2021-03-16  3:42   ` Manivannan Sadhasivam
  2021-03-15 15:21 ` [PATCH net-next 3/3] net: ipa: extend the INDICATION_REGISTER request Alex Elder
  2021-03-15 16:38 ` [PATCH net-next 0/3] net: ipa: QMI fixes Manivannan Sadhasivam
  3 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2021-03-15 15:21 UTC (permalink / raw)
  To: davem, kuba
  Cc: manivannan.sadhasivam, bjorn.andersson, evgreen, cpratapa, elder,
	netdev, linux-kernel

The ipa_init_modem_driver_req_ei[] encoding array for the
INIT_MODEM_DRIVER request message has some errors in it.

First, the tlv_type associated with the hw_stats_quota_size field is
wrong; it duplicates the valiue used for the hw_stats_quota_base_addr
field (0x1f) and should use 0x20 instead.  The tlv_type value for
the hw_stats_drop_size field also uses the same duplicate value; it
should use 0x22 instead.

Second, there is no definition for the hw_stats_drop_base_addr
field.  It is an optional 32-bit enumerated type value.

Finally, the hw_stats_quota_base_addr, hw_stats_quota_size, and
hw_stats_drop_size fields are defined as enumerated types; they
should be unsigned 4-byte values.

Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_qmi_msg.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ipa/ipa_qmi_msg.c b/drivers/net/ipa/ipa_qmi_msg.c
index e00f829a783f6..e4a6efbe9bd00 100644
--- a/drivers/net/ipa/ipa_qmi_msg.c
+++ b/drivers/net/ipa/ipa_qmi_msg.c
@@ -530,7 +530,7 @@ struct qmi_elem_info ipa_init_modem_driver_req_ei[] = {
 					   hw_stats_quota_base_addr_valid),
 	},
 	{
-		.data_type	= QMI_SIGNED_4_BYTE_ENUM,
+		.data_type	= QMI_UNSIGNED_4_BYTE,
 		.elem_len	= 1,
 		.elem_size	=
 			sizeof_field(struct ipa_init_modem_driver_req,
@@ -545,37 +545,57 @@ struct qmi_elem_info ipa_init_modem_driver_req_ei[] = {
 		.elem_size	=
 			sizeof_field(struct ipa_init_modem_driver_req,
 				     hw_stats_quota_size_valid),
-		.tlv_type	= 0x1f,
+		.tlv_type	= 0x20,
 		.offset		= offsetof(struct ipa_init_modem_driver_req,
 					   hw_stats_quota_size_valid),
 	},
 	{
-		.data_type	= QMI_SIGNED_4_BYTE_ENUM,
+		.data_type	= QMI_UNSIGNED_4_BYTE,
 		.elem_len	= 1,
 		.elem_size	=
 			sizeof_field(struct ipa_init_modem_driver_req,
 				     hw_stats_quota_size),
-		.tlv_type	= 0x1f,
+		.tlv_type	= 0x20,
 		.offset		= offsetof(struct ipa_init_modem_driver_req,
 					   hw_stats_quota_size),
 	},
+	{
+		.data_type	= QMI_OPT_FLAG,
+		.elem_len	= 1,
+		.elem_size	=
+			sizeof_field(struct ipa_init_modem_driver_req,
+				     hw_stats_drop_base_addr_valid),
+		.tlv_type	= 0x21,
+		.offset		= offsetof(struct ipa_init_modem_driver_req,
+					   hw_stats_drop_base_addr_valid),
+	},
+	{
+		.data_type	= QMI_UNSIGNED_4_BYTE,
+		.elem_len	= 1,
+		.elem_size	=
+			sizeof_field(struct ipa_init_modem_driver_req,
+				     hw_stats_drop_base_addr),
+		.tlv_type	= 0x21,
+		.offset		= offsetof(struct ipa_init_modem_driver_req,
+					   hw_stats_drop_base_addr),
+	},
 	{
 		.data_type	= QMI_OPT_FLAG,
 		.elem_len	= 1,
 		.elem_size	=
 			sizeof_field(struct ipa_init_modem_driver_req,
 				     hw_stats_drop_size_valid),
-		.tlv_type	= 0x1f,
+		.tlv_type	= 0x22,
 		.offset		= offsetof(struct ipa_init_modem_driver_req,
 					   hw_stats_drop_size_valid),
 	},
 	{
-		.data_type	= QMI_SIGNED_4_BYTE_ENUM,
+		.data_type	= QMI_UNSIGNED_4_BYTE,
 		.elem_len	= 1,
 		.elem_size	=
 			sizeof_field(struct ipa_init_modem_driver_req,
 				     hw_stats_drop_size),
-		.tlv_type	= 0x1f,
+		.tlv_type	= 0x22,
 		.offset		= offsetof(struct ipa_init_modem_driver_req,
 					   hw_stats_drop_size),
 	},
-- 
2.27.0


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

* [PATCH net-next 3/3] net: ipa: extend the INDICATION_REGISTER request
  2021-03-15 15:21 [PATCH net-next 0/3] net: ipa: QMI fixes Alex Elder
  2021-03-15 15:21 ` [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value Alex Elder
  2021-03-15 15:21 ` [PATCH net-next 2/3] net: ipa: fix another QMI message definition Alex Elder
@ 2021-03-15 15:21 ` Alex Elder
  2021-03-16  3:43   ` Manivannan Sadhasivam
  2021-03-15 16:38 ` [PATCH net-next 0/3] net: ipa: QMI fixes Manivannan Sadhasivam
  3 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2021-03-15 15:21 UTC (permalink / raw)
  To: davem, kuba
  Cc: manivannan.sadhasivam, bjorn.andersson, evgreen, cpratapa, elder,
	netdev, linux-kernel

The specified format of the INDICATION_REGISTER QMI request message
has been extended to support two more optional fields:
  endpoint_desc_ind:
    sender wishes to receive endpoint descriptor information via
    an IPA ENDP_DESC indication QMI message
  bw_change_ind:
    sender wishes to receive bandwidth change information via
    an IPA BW_CHANGE indication QMI message

Add definitions that permit these fields to be formatted and parsed
by the QMI library code.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_qmi_msg.c | 40 +++++++++++++++++++++++++++++++++++
 drivers/net/ipa/ipa_qmi_msg.h |  6 +++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ipa/ipa_qmi_msg.c b/drivers/net/ipa/ipa_qmi_msg.c
index e4a6efbe9bd00..6838e8065072b 100644
--- a/drivers/net/ipa/ipa_qmi_msg.c
+++ b/drivers/net/ipa/ipa_qmi_msg.c
@@ -70,6 +70,46 @@ struct qmi_elem_info ipa_indication_register_req_ei[] = {
 		.offset		= offsetof(struct ipa_indication_register_req,
 					   ipa_mhi_ready_ind),
 	},
+	{
+		.data_type	= QMI_OPT_FLAG,
+		.elem_len	= 1,
+		.elem_size	=
+			sizeof_field(struct ipa_indication_register_req,
+				     endpoint_desc_ind_valid),
+		.tlv_type	= 0x13,
+		.offset		= offsetof(struct ipa_indication_register_req,
+					   endpoint_desc_ind_valid),
+	},
+	{
+		.data_type	= QMI_UNSIGNED_1_BYTE,
+		.elem_len	= 1,
+		.elem_size	=
+			sizeof_field(struct ipa_indication_register_req,
+				     endpoint_desc_ind),
+		.tlv_type	= 0x13,
+		.offset		= offsetof(struct ipa_indication_register_req,
+					   endpoint_desc_ind),
+	},
+	{
+		.data_type	= QMI_OPT_FLAG,
+		.elem_len	= 1,
+		.elem_size	=
+			sizeof_field(struct ipa_indication_register_req,
+				     bw_change_ind_valid),
+		.tlv_type	= 0x14,
+		.offset		= offsetof(struct ipa_indication_register_req,
+					   bw_change_ind_valid),
+	},
+	{
+		.data_type	= QMI_UNSIGNED_1_BYTE,
+		.elem_len	= 1,
+		.elem_size	=
+			sizeof_field(struct ipa_indication_register_req,
+				     bw_change_ind),
+		.tlv_type	= 0x14,
+		.offset		= offsetof(struct ipa_indication_register_req,
+					   bw_change_ind),
+	},
 	{
 		.data_type	= QMI_EOTI,
 	},
diff --git a/drivers/net/ipa/ipa_qmi_msg.h b/drivers/net/ipa/ipa_qmi_msg.h
index 12b6621f4b0e6..3233d145fd87c 100644
--- a/drivers/net/ipa/ipa_qmi_msg.h
+++ b/drivers/net/ipa/ipa_qmi_msg.h
@@ -24,7 +24,7 @@
  * information for each field.  The qmi_send_*() interfaces require
  * the message size to be provided.
  */
-#define IPA_QMI_INDICATION_REGISTER_REQ_SZ	12	/* -> server handle */
+#define IPA_QMI_INDICATION_REGISTER_REQ_SZ	20	/* -> server handle */
 #define IPA_QMI_INDICATION_REGISTER_RSP_SZ	7	/* <- server handle */
 #define IPA_QMI_INIT_DRIVER_REQ_SZ		162	/* client handle -> */
 #define IPA_QMI_INIT_DRIVER_RSP_SZ		25	/* client handle <- */
@@ -44,6 +44,10 @@ struct ipa_indication_register_req {
 	u8 data_usage_quota_reached;
 	u8 ipa_mhi_ready_ind_valid;
 	u8 ipa_mhi_ready_ind;
+	u8 endpoint_desc_ind_valid;
+	u8 endpoint_desc_ind;
+	u8 bw_change_ind_valid;
+	u8 bw_change_ind;
 };
 
 /* The response to a IPA_QMI_INDICATION_REGISTER request consists only of
-- 
2.27.0


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

* Re: [PATCH net-next 0/3] net: ipa: QMI fixes
  2021-03-15 15:21 [PATCH net-next 0/3] net: ipa: QMI fixes Alex Elder
                   ` (2 preceding siblings ...)
  2021-03-15 15:21 ` [PATCH net-next 3/3] net: ipa: extend the INDICATION_REGISTER request Alex Elder
@ 2021-03-15 16:38 ` Manivannan Sadhasivam
  2021-03-15 16:50   ` Alex Elder
  3 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-15 16:38 UTC (permalink / raw)
  To: Alex Elder
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

Hi Alex,

On Mon, Mar 15, 2021 at 10:21:09AM -0500, Alex Elder wrote:
> Mani Sadhasivam discovered some errors in the definitions of some
> QMI messages used for IPA.  This series addresses those errors,
> and extends the definition of one message type to include some
> newly-defined fields.
> 

Thanks for the patches. I guess you need to add Fixes tag for patches 1,2 and
they should be backported to stable.

Thanks,
Mani

> 					-Alex
> 
> Alex Elder (3):
>   net: ipa: fix a duplicated tlv_type value
>   net: ipa: fix another QMI message definition
>   net: ipa: extend the INDICATION_REGISTER request
> 
>  drivers/net/ipa/ipa_qmi_msg.c | 78 +++++++++++++++++++++++++++++++----
>  drivers/net/ipa/ipa_qmi_msg.h |  6 ++-
>  2 files changed, 74 insertions(+), 10 deletions(-)
> 
> -- 
> 2.27.0
> 

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

* Re: [PATCH net-next 0/3] net: ipa: QMI fixes
  2021-03-15 16:38 ` [PATCH net-next 0/3] net: ipa: QMI fixes Manivannan Sadhasivam
@ 2021-03-15 16:50   ` Alex Elder
  2021-03-16  3:25     ` Manivannan Sadhasivam
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2021-03-15 16:50 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On 3/15/21 11:38 AM, Manivannan Sadhasivam wrote:
> Hi Alex,
> 
> On Mon, Mar 15, 2021 at 10:21:09AM -0500, Alex Elder wrote:
>> Mani Sadhasivam discovered some errors in the definitions of some
>> QMI messages used for IPA.  This series addresses those errors,
>> and extends the definition of one message type to include some
>> newly-defined fields.
>>
> 
> Thanks for the patches. I guess you need to add Fixes tag for patches 1,2 and
> they should be backported to stable.

I did not do that, intentionally.  The reason is that the
existing code only supports IPA v3.5.1 and IPAv4.2.  And
these bugs seem to cause no problems there.

There are some patches coming very soon that will add
more formal support for IPA v4.5 (where I know you
found these issues).  Those will not be back-ported.

So these fixes don't appear to be necessary for existing
supported platforms.

If you still believe I should have these back-ported,
I have no objection to re-posting for that.  But I
wanted to explain my reasoning before doing it.

--> Do you still think I should have these back-ported?

Thanks.

					-Alex

> 
> Thanks,
> Mani
> 
>> 					-Alex
>>
>> Alex Elder (3):
>>    net: ipa: fix a duplicated tlv_type value
>>    net: ipa: fix another QMI message definition
>>    net: ipa: extend the INDICATION_REGISTER request
>>
>>   drivers/net/ipa/ipa_qmi_msg.c | 78 +++++++++++++++++++++++++++++++----
>>   drivers/net/ipa/ipa_qmi_msg.h |  6 ++-
>>   2 files changed, 74 insertions(+), 10 deletions(-)
>>
>> -- 
>> 2.27.0
>>


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

* Re: [PATCH net-next 0/3] net: ipa: QMI fixes
  2021-03-15 16:50   ` Alex Elder
@ 2021-03-16  3:25     ` Manivannan Sadhasivam
  2021-03-16 16:00       ` Alex Elder
  0 siblings, 1 reply; 12+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-16  3:25 UTC (permalink / raw)
  To: Alex Elder
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On Mon, Mar 15, 2021 at 11:50:15AM -0500, Alex Elder wrote:
> On 3/15/21 11:38 AM, Manivannan Sadhasivam wrote:
> > Hi Alex,
> > 
> > On Mon, Mar 15, 2021 at 10:21:09AM -0500, Alex Elder wrote:
> > > Mani Sadhasivam discovered some errors in the definitions of some
> > > QMI messages used for IPA.  This series addresses those errors,
> > > and extends the definition of one message type to include some
> > > newly-defined fields.
> > > 
> > 
> > Thanks for the patches. I guess you need to add Fixes tag for patches 1,2 and
> > they should be backported to stable.
> 
> I did not do that, intentionally.  The reason is that the
> existing code only supports IPA v3.5.1 and IPAv4.2.  And
> these bugs seem to cause no problems there.
> 
> There are some patches coming very soon that will add
> more formal support for IPA v4.5 (where I know you
> found these issues).  Those will not be back-ported.
> 
> So these fixes don't appear to be necessary for existing
> supported platforms.
> 

Hmm, okay. Then please mention this information in the commit description(s)
that the fix is only needed for IPA4.5.

Thanks,
Mani

> If you still believe I should have these back-ported,
> I have no objection to re-posting for that.  But I
> wanted to explain my reasoning before doing it.
> 
> --> Do you still think I should have these back-ported?
> 
> Thanks.
> 
> 					-Alex
> 
> > 
> > Thanks,
> > Mani
> > 
> > > 					-Alex
> > > 
> > > Alex Elder (3):
> > >    net: ipa: fix a duplicated tlv_type value
> > >    net: ipa: fix another QMI message definition
> > >    net: ipa: extend the INDICATION_REGISTER request
> > > 
> > >   drivers/net/ipa/ipa_qmi_msg.c | 78 +++++++++++++++++++++++++++++++----
> > >   drivers/net/ipa/ipa_qmi_msg.h |  6 ++-
> > >   2 files changed, 74 insertions(+), 10 deletions(-)
> > > 
> > > -- 
> > > 2.27.0
> > > 
> 

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

* Re: [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value
  2021-03-15 15:21 ` [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value Alex Elder
@ 2021-03-16  3:37   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-16  3:37 UTC (permalink / raw)
  To: Alex Elder
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On Mon, Mar 15, 2021 at 10:21:10AM -0500, Alex Elder wrote:
> In the ipa_indication_register_req_ei[] encoding array, the tlv_type
> associated with the ipa_mhi_ready_ind field is wrong.  It duplicates
> the value used for the data_usage_quota_reached field (0x11) and
> should use value 0x12 instead.  Fix this bug.
> 
> Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Alex Elder <elder@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/net/ipa/ipa_qmi_msg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ipa/ipa_qmi_msg.c b/drivers/net/ipa/ipa_qmi_msg.c
> index 73413371e3d3e..e00f829a783f6 100644
> --- a/drivers/net/ipa/ipa_qmi_msg.c
> +++ b/drivers/net/ipa/ipa_qmi_msg.c
> @@ -56,7 +56,7 @@ struct qmi_elem_info ipa_indication_register_req_ei[] = {
>  		.elem_size	=
>  			sizeof_field(struct ipa_indication_register_req,
>  				     ipa_mhi_ready_ind_valid),
> -		.tlv_type	= 0x11,
> +		.tlv_type	= 0x12,
>  		.offset		= offsetof(struct ipa_indication_register_req,
>  					   ipa_mhi_ready_ind_valid),
>  	},
> @@ -66,7 +66,7 @@ struct qmi_elem_info ipa_indication_register_req_ei[] = {
>  		.elem_size	=
>  			sizeof_field(struct ipa_indication_register_req,
>  				     ipa_mhi_ready_ind),
> -		.tlv_type	= 0x11,
> +		.tlv_type	= 0x12,
>  		.offset		= offsetof(struct ipa_indication_register_req,
>  					   ipa_mhi_ready_ind),
>  	},
> -- 
> 2.27.0
> 

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

* Re: [PATCH net-next 2/3] net: ipa: fix another QMI message definition
  2021-03-15 15:21 ` [PATCH net-next 2/3] net: ipa: fix another QMI message definition Alex Elder
@ 2021-03-16  3:42   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-16  3:42 UTC (permalink / raw)
  To: Alex Elder
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On Mon, Mar 15, 2021 at 10:21:11AM -0500, Alex Elder wrote:
> The ipa_init_modem_driver_req_ei[] encoding array for the
> INIT_MODEM_DRIVER request message has some errors in it.
> 
> First, the tlv_type associated with the hw_stats_quota_size field is
> wrong; it duplicates the valiue used for the hw_stats_quota_base_addr
> field (0x1f) and should use 0x20 instead.  The tlv_type value for
> the hw_stats_drop_size field also uses the same duplicate value; it
> should use 0x22 instead.
> 
> Second, there is no definition for the hw_stats_drop_base_addr
> field.  It is an optional 32-bit enumerated type value.
> 
> Finally, the hw_stats_quota_base_addr, hw_stats_quota_size, and
> hw_stats_drop_size fields are defined as enumerated types; they
> should be unsigned 4-byte values.
> 
> Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Alex Elder <elder@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/net/ipa/ipa_qmi_msg.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ipa/ipa_qmi_msg.c b/drivers/net/ipa/ipa_qmi_msg.c
> index e00f829a783f6..e4a6efbe9bd00 100644
> --- a/drivers/net/ipa/ipa_qmi_msg.c
> +++ b/drivers/net/ipa/ipa_qmi_msg.c
> @@ -530,7 +530,7 @@ struct qmi_elem_info ipa_init_modem_driver_req_ei[] = {
>  					   hw_stats_quota_base_addr_valid),
>  	},
>  	{
> -		.data_type	= QMI_SIGNED_4_BYTE_ENUM,
> +		.data_type	= QMI_UNSIGNED_4_BYTE,
>  		.elem_len	= 1,
>  		.elem_size	=
>  			sizeof_field(struct ipa_init_modem_driver_req,
> @@ -545,37 +545,57 @@ struct qmi_elem_info ipa_init_modem_driver_req_ei[] = {
>  		.elem_size	=
>  			sizeof_field(struct ipa_init_modem_driver_req,
>  				     hw_stats_quota_size_valid),
> -		.tlv_type	= 0x1f,
> +		.tlv_type	= 0x20,
>  		.offset		= offsetof(struct ipa_init_modem_driver_req,
>  					   hw_stats_quota_size_valid),
>  	},
>  	{
> -		.data_type	= QMI_SIGNED_4_BYTE_ENUM,
> +		.data_type	= QMI_UNSIGNED_4_BYTE,
>  		.elem_len	= 1,
>  		.elem_size	=
>  			sizeof_field(struct ipa_init_modem_driver_req,
>  				     hw_stats_quota_size),
> -		.tlv_type	= 0x1f,
> +		.tlv_type	= 0x20,
>  		.offset		= offsetof(struct ipa_init_modem_driver_req,
>  					   hw_stats_quota_size),
>  	},
> +	{
> +		.data_type	= QMI_OPT_FLAG,
> +		.elem_len	= 1,
> +		.elem_size	=
> +			sizeof_field(struct ipa_init_modem_driver_req,
> +				     hw_stats_drop_base_addr_valid),
> +		.tlv_type	= 0x21,
> +		.offset		= offsetof(struct ipa_init_modem_driver_req,
> +					   hw_stats_drop_base_addr_valid),
> +	},
> +	{
> +		.data_type	= QMI_UNSIGNED_4_BYTE,
> +		.elem_len	= 1,
> +		.elem_size	=
> +			sizeof_field(struct ipa_init_modem_driver_req,
> +				     hw_stats_drop_base_addr),
> +		.tlv_type	= 0x21,
> +		.offset		= offsetof(struct ipa_init_modem_driver_req,
> +					   hw_stats_drop_base_addr),
> +	},
>  	{
>  		.data_type	= QMI_OPT_FLAG,
>  		.elem_len	= 1,
>  		.elem_size	=
>  			sizeof_field(struct ipa_init_modem_driver_req,
>  				     hw_stats_drop_size_valid),
> -		.tlv_type	= 0x1f,
> +		.tlv_type	= 0x22,
>  		.offset		= offsetof(struct ipa_init_modem_driver_req,
>  					   hw_stats_drop_size_valid),
>  	},
>  	{
> -		.data_type	= QMI_SIGNED_4_BYTE_ENUM,
> +		.data_type	= QMI_UNSIGNED_4_BYTE,
>  		.elem_len	= 1,
>  		.elem_size	=
>  			sizeof_field(struct ipa_init_modem_driver_req,
>  				     hw_stats_drop_size),
> -		.tlv_type	= 0x1f,
> +		.tlv_type	= 0x22,
>  		.offset		= offsetof(struct ipa_init_modem_driver_req,
>  					   hw_stats_drop_size),
>  	},
> -- 
> 2.27.0
> 

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

* Re: [PATCH net-next 3/3] net: ipa: extend the INDICATION_REGISTER request
  2021-03-15 15:21 ` [PATCH net-next 3/3] net: ipa: extend the INDICATION_REGISTER request Alex Elder
@ 2021-03-16  3:43   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-16  3:43 UTC (permalink / raw)
  To: Alex Elder
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On Mon, Mar 15, 2021 at 10:21:12AM -0500, Alex Elder wrote:
> The specified format of the INDICATION_REGISTER QMI request message
> has been extended to support two more optional fields:
>   endpoint_desc_ind:
>     sender wishes to receive endpoint descriptor information via
>     an IPA ENDP_DESC indication QMI message
>   bw_change_ind:
>     sender wishes to receive bandwidth change information via
>     an IPA BW_CHANGE indication QMI message
> 
> Add definitions that permit these fields to be formatted and parsed
> by the QMI library code.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/net/ipa/ipa_qmi_msg.c | 40 +++++++++++++++++++++++++++++++++++
>  drivers/net/ipa/ipa_qmi_msg.h |  6 +++++-
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ipa/ipa_qmi_msg.c b/drivers/net/ipa/ipa_qmi_msg.c
> index e4a6efbe9bd00..6838e8065072b 100644
> --- a/drivers/net/ipa/ipa_qmi_msg.c
> +++ b/drivers/net/ipa/ipa_qmi_msg.c
> @@ -70,6 +70,46 @@ struct qmi_elem_info ipa_indication_register_req_ei[] = {
>  		.offset		= offsetof(struct ipa_indication_register_req,
>  					   ipa_mhi_ready_ind),
>  	},
> +	{
> +		.data_type	= QMI_OPT_FLAG,
> +		.elem_len	= 1,
> +		.elem_size	=
> +			sizeof_field(struct ipa_indication_register_req,
> +				     endpoint_desc_ind_valid),
> +		.tlv_type	= 0x13,
> +		.offset		= offsetof(struct ipa_indication_register_req,
> +					   endpoint_desc_ind_valid),
> +	},
> +	{
> +		.data_type	= QMI_UNSIGNED_1_BYTE,
> +		.elem_len	= 1,
> +		.elem_size	=
> +			sizeof_field(struct ipa_indication_register_req,
> +				     endpoint_desc_ind),
> +		.tlv_type	= 0x13,
> +		.offset		= offsetof(struct ipa_indication_register_req,
> +					   endpoint_desc_ind),
> +	},
> +	{
> +		.data_type	= QMI_OPT_FLAG,
> +		.elem_len	= 1,
> +		.elem_size	=
> +			sizeof_field(struct ipa_indication_register_req,
> +				     bw_change_ind_valid),
> +		.tlv_type	= 0x14,
> +		.offset		= offsetof(struct ipa_indication_register_req,
> +					   bw_change_ind_valid),
> +	},
> +	{
> +		.data_type	= QMI_UNSIGNED_1_BYTE,
> +		.elem_len	= 1,
> +		.elem_size	=
> +			sizeof_field(struct ipa_indication_register_req,
> +				     bw_change_ind),
> +		.tlv_type	= 0x14,
> +		.offset		= offsetof(struct ipa_indication_register_req,
> +					   bw_change_ind),
> +	},
>  	{
>  		.data_type	= QMI_EOTI,
>  	},
> diff --git a/drivers/net/ipa/ipa_qmi_msg.h b/drivers/net/ipa/ipa_qmi_msg.h
> index 12b6621f4b0e6..3233d145fd87c 100644
> --- a/drivers/net/ipa/ipa_qmi_msg.h
> +++ b/drivers/net/ipa/ipa_qmi_msg.h
> @@ -24,7 +24,7 @@
>   * information for each field.  The qmi_send_*() interfaces require
>   * the message size to be provided.
>   */
> -#define IPA_QMI_INDICATION_REGISTER_REQ_SZ	12	/* -> server handle */
> +#define IPA_QMI_INDICATION_REGISTER_REQ_SZ	20	/* -> server handle */
>  #define IPA_QMI_INDICATION_REGISTER_RSP_SZ	7	/* <- server handle */
>  #define IPA_QMI_INIT_DRIVER_REQ_SZ		162	/* client handle -> */
>  #define IPA_QMI_INIT_DRIVER_RSP_SZ		25	/* client handle <- */
> @@ -44,6 +44,10 @@ struct ipa_indication_register_req {
>  	u8 data_usage_quota_reached;
>  	u8 ipa_mhi_ready_ind_valid;
>  	u8 ipa_mhi_ready_ind;
> +	u8 endpoint_desc_ind_valid;
> +	u8 endpoint_desc_ind;
> +	u8 bw_change_ind_valid;
> +	u8 bw_change_ind;
>  };
>  
>  /* The response to a IPA_QMI_INDICATION_REGISTER request consists only of
> -- 
> 2.27.0
> 

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

* Re: [PATCH net-next 0/3] net: ipa: QMI fixes
  2021-03-16  3:25     ` Manivannan Sadhasivam
@ 2021-03-16 16:00       ` Alex Elder
  2021-03-16 16:38         ` Manivannan Sadhasivam
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2021-03-16 16:00 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On 3/15/21 10:25 PM, Manivannan Sadhasivam wrote:
> On Mon, Mar 15, 2021 at 11:50:15AM -0500, Alex Elder wrote:
>> On 3/15/21 11:38 AM, Manivannan Sadhasivam wrote:
>>> Hi Alex,
>>>
>>> On Mon, Mar 15, 2021 at 10:21:09AM -0500, Alex Elder wrote:
>>>> Mani Sadhasivam discovered some errors in the definitions of some
>>>> QMI messages used for IPA.  This series addresses those errors,
>>>> and extends the definition of one message type to include some
>>>> newly-defined fields.
>>>>
>>>
>>> Thanks for the patches. I guess you need to add Fixes tag for patches 1,2 and
>>> they should be backported to stable.
>>
>> I did not do that, intentionally.  The reason is that the
>> existing code only supports IPA v3.5.1 and IPAv4.2.  And
>> these bugs seem to cause no problems there.
>>
>> There are some patches coming very soon that will add
>> more formal support for IPA v4.5 (where I know you
>> found these issues).  Those will not be back-ported.
>>
>> So these fixes don't appear to be necessary for existing
>> supported platforms.
>>
> 
> Hmm, okay. Then please mention this information in the commit description(s)
> that the fix is only needed for IPA4.5.

Mani, you ACKed all three patches after you sent this.

Are you expecting me to send a new version of the code,
or are you willing to accept the series as-is?

Thanks.

					-Alex

> 
> Thanks,
> Mani
> 
>> If you still believe I should have these back-ported,
>> I have no objection to re-posting for that.  But I
>> wanted to explain my reasoning before doing it.
>>
>> --> Do you still think I should have these back-ported?
>>
>> Thanks.
>>
>> 					-Alex
>>
>>>
>>> Thanks,
>>> Mani
>>>
>>>> 					-Alex
>>>>
>>>> Alex Elder (3):
>>>>     net: ipa: fix a duplicated tlv_type value
>>>>     net: ipa: fix another QMI message definition
>>>>     net: ipa: extend the INDICATION_REGISTER request
>>>>
>>>>    drivers/net/ipa/ipa_qmi_msg.c | 78 +++++++++++++++++++++++++++++++----
>>>>    drivers/net/ipa/ipa_qmi_msg.h |  6 ++-
>>>>    2 files changed, 74 insertions(+), 10 deletions(-)
>>>>
>>>> -- 
>>>> 2.27.0
>>>>
>>


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

* Re: [PATCH net-next 0/3] net: ipa: QMI fixes
  2021-03-16 16:00       ` Alex Elder
@ 2021-03-16 16:38         ` Manivannan Sadhasivam
  0 siblings, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-16 16:38 UTC (permalink / raw)
  To: Alex Elder
  Cc: davem, kuba, bjorn.andersson, evgreen, cpratapa, elder, netdev,
	linux-kernel

On Tue, Mar 16, 2021 at 11:00:37AM -0500, Alex Elder wrote:
> On 3/15/21 10:25 PM, Manivannan Sadhasivam wrote:
> > On Mon, Mar 15, 2021 at 11:50:15AM -0500, Alex Elder wrote:
> > > On 3/15/21 11:38 AM, Manivannan Sadhasivam wrote:
> > > > Hi Alex,
> > > > 
> > > > On Mon, Mar 15, 2021 at 10:21:09AM -0500, Alex Elder wrote:
> > > > > Mani Sadhasivam discovered some errors in the definitions of some
> > > > > QMI messages used for IPA.  This series addresses those errors,
> > > > > and extends the definition of one message type to include some
> > > > > newly-defined fields.
> > > > > 
> > > > 
> > > > Thanks for the patches. I guess you need to add Fixes tag for patches 1,2 and
> > > > they should be backported to stable.
> > > 
> > > I did not do that, intentionally.  The reason is that the
> > > existing code only supports IPA v3.5.1 and IPAv4.2.  And
> > > these bugs seem to cause no problems there.
> > > 
> > > There are some patches coming very soon that will add
> > > more formal support for IPA v4.5 (where I know you
> > > found these issues).  Those will not be back-ported.
> > > 
> > > So these fixes don't appear to be necessary for existing
> > > supported platforms.
> > > 
> > 
> > Hmm, okay. Then please mention this information in the commit description(s)
> > that the fix is only needed for IPA4.5.
> 
> Mani, you ACKed all three patches after you sent this.
> 
> Are you expecting me to send a new version of the code,
> or are you willing to accept the series as-is?
> 

Are you asking me the question? I can't accept IPA code.

Thanks,
Mani

> Thanks.
> 
> 					-Alex
> 
> > 
> > Thanks,
> > Mani
> > 
> > > If you still believe I should have these back-ported,
> > > I have no objection to re-posting for that.  But I
> > > wanted to explain my reasoning before doing it.
> > > 
> > > --> Do you still think I should have these back-ported?
> > > 
> > > Thanks.
> > > 
> > > 					-Alex
> > > 
> > > > 
> > > > Thanks,
> > > > Mani
> > > > 
> > > > > 					-Alex
> > > > > 
> > > > > Alex Elder (3):
> > > > >     net: ipa: fix a duplicated tlv_type value
> > > > >     net: ipa: fix another QMI message definition
> > > > >     net: ipa: extend the INDICATION_REGISTER request
> > > > > 
> > > > >    drivers/net/ipa/ipa_qmi_msg.c | 78 +++++++++++++++++++++++++++++++----
> > > > >    drivers/net/ipa/ipa_qmi_msg.h |  6 ++-
> > > > >    2 files changed, 74 insertions(+), 10 deletions(-)
> > > > > 
> > > > > -- 
> > > > > 2.27.0
> > > > > 
> > > 
> 

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

end of thread, other threads:[~2021-03-16 16:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 15:21 [PATCH net-next 0/3] net: ipa: QMI fixes Alex Elder
2021-03-15 15:21 ` [PATCH net-next 1/3] net: ipa: fix a duplicated tlv_type value Alex Elder
2021-03-16  3:37   ` Manivannan Sadhasivam
2021-03-15 15:21 ` [PATCH net-next 2/3] net: ipa: fix another QMI message definition Alex Elder
2021-03-16  3:42   ` Manivannan Sadhasivam
2021-03-15 15:21 ` [PATCH net-next 3/3] net: ipa: extend the INDICATION_REGISTER request Alex Elder
2021-03-16  3:43   ` Manivannan Sadhasivam
2021-03-15 16:38 ` [PATCH net-next 0/3] net: ipa: QMI fixes Manivannan Sadhasivam
2021-03-15 16:50   ` Alex Elder
2021-03-16  3:25     ` Manivannan Sadhasivam
2021-03-16 16:00       ` Alex Elder
2021-03-16 16:38         ` Manivannan Sadhasivam

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.