[PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Shane Parslow]

This patch fixes the control channel mapping for the 7360, which was
previously the same as the 7560.

As shown by the reverse engineering efforts of James Wah [1], the layout
of channels on the 7360 is actually somewhat different from that of the
7560.

A new ipc_chnl_cfg is added specifically for the 7360. The new config
updates channel 7 to be an AT port and removes the mbim interface, as
it does not exist on the 7360. The config is otherwise left the same as
the 7560. ipc_chnl_cfg_get is updated to switch between the two configs.
In ipc_imem, a special case for the mbim port is removed as it no longer
exists in the 7360 ipc_chnl_cfg.

As a result of this, the second userspace AT port now functions whereas
previously it was routed to the trace channel. Modem crashes ("confused
phase", "msg timeout", "PORT open refused") resulting from garbage being
sent to the modem are also fixed.

[1] https://github.com/xmm7360/reversing

Signed-off-by: Shane Parslow <shaneparslow808@gmail.com>
---
 drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c | 58 ++++++++++++++++++++---
 drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.h |  7 ++-
 drivers/net/wwan/iosm/iosm_ipc_devlink.c  |  3 +-
 drivers/net/wwan/iosm/iosm_ipc_imem.c     | 12 ++---
 drivers/net/wwan/iosm/iosm_ipc_imem_ops.c |  3 +-
 drivers/net/wwan/iosm/iosm_ipc_trace.c    |  3 +-
 6 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c b/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c
index 128c999e08bb..011bae887f8b 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c
@@ -5,6 +5,7 @@
 
 #include <linux/wwan.h>
 
+#include "iosm_ipc_imem.h"
 #include "iosm_ipc_chnl_cfg.h"
 
 /* Max. sizes of a downlink buffers */
@@ -28,10 +29,10 @@
 /* MUX acc backoff 1ms */
 #define IRQ_ACC_BACKOFF_MUX 1000
 
-/* Modem channel configuration table
+/* 7560 modem channel configuration table
  * Always reserve element zero for flash channel.
  */
-static struct ipc_chnl_cfg modem_cfg[] = {
+static struct ipc_chnl_cfg modem_cfg_7560[IPC_MEM_MAX_CHANNELS] = {
 	/* IP Mux */
 	{ IPC_MEM_IP_CHL_ID_0, IPC_MEM_PIPE_0, IPC_MEM_PIPE_1,
 	  IPC_MEM_MAX_TDS_MUX_LITE_UL, IPC_MEM_MAX_TDS_MUX_LITE_DL,
@@ -66,11 +67,56 @@ static struct ipc_chnl_cfg modem_cfg[] = {
 	  IPC_MEM_MAX_DL_FLASH_BUF_SIZE, WWAN_PORT_UNKNOWN },
 };
 
-int ipc_chnl_cfg_get(struct ipc_chnl_cfg *chnl_cfg, int index)
+/* Channel layout for the 7360
+ * Copied from the 7560, and modified based on the reverse
+ * engineering efforts of James Wah
+ */
+static struct ipc_chnl_cfg modem_cfg_7360[IPC_MEM_MAX_CHANNELS] = {
+	/* IP Mux */
+	{ IPC_MEM_IP_CHL_ID_0, IPC_MEM_PIPE_0, IPC_MEM_PIPE_1,
+	  IPC_MEM_MAX_TDS_MUX_LITE_UL, IPC_MEM_MAX_TDS_MUX_LITE_DL,
+	  IPC_MEM_MAX_DL_MUX_LITE_BUF_SIZE, WWAN_PORT_UNKNOWN },
+	/* RPC - 0 */
+	{ IPC_MEM_CTRL_CHL_ID_1, IPC_MEM_PIPE_2, IPC_MEM_PIPE_3,
+	  IPC_MEM_MAX_TDS_RPC, IPC_MEM_MAX_TDS_RPC,
+	  IPC_MEM_MAX_DL_RPC_BUF_SIZE, WWAN_PORT_UNKNOWN },
+	/* Trace? */
+	{ IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
+	  IPC_MEM_TDS_TRC, IPC_MEM_TDS_TRC, IPC_MEM_MAX_DL_TRC_BUF_SIZE,
+	  WWAN_PORT_UNKNOWN },
+	/* Trace? */
+	{ IPC_MEM_CTRL_CHL_ID_3, IPC_MEM_PIPE_6, IPC_MEM_PIPE_7,
+	  IPC_MEM_TDS_TRC, IPC_MEM_TDS_TRC, IPC_MEM_MAX_DL_TRC_BUF_SIZE,
+	  WWAN_PORT_UNKNOWN },
+	/* IAT0 */
+	{ IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
+	  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
+	  WWAN_PORT_AT },
+	/* Unknown */
+	{ IPC_MEM_CTRL_CHL_ID_5, IPC_MEM_PIPE_10, IPC_MEM_PIPE_11,
+	  IPC_MEM_MAX_TDS_LOOPBACK, IPC_MEM_MAX_TDS_LOOPBACK,
+	  IPC_MEM_MAX_DL_LOOPBACK_SIZE, WWAN_PORT_UNKNOWN },
+	/* Unknown */
+	{ IPC_MEM_CTRL_CHL_ID_6, IPC_MEM_PIPE_12, IPC_MEM_PIPE_13,
+	  IPC_MEM_MAX_TDS_MBIM, IPC_MEM_MAX_TDS_MBIM,
+	  IPC_MEM_MAX_DL_MBIM_BUF_SIZE, WWAN_PORT_UNKNOWN },
+	/* IAT1 */
+	{ IPC_MEM_CTRL_CHL_ID_7, IPC_MEM_PIPE_14, IPC_MEM_PIPE_15,
+	  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT,
+	  IPC_MEM_MAX_DL_AT_BUF_SIZE, WWAN_PORT_AT },
+};
+
+int ipc_chnl_cfg_get(struct ipc_chnl_cfg *chnl_cfg,
+		     struct pci_dev *dev, int index)
 {
-	if (index >= ARRAY_SIZE(modem_cfg)) {
-		pr_err("index: %d and array size %zu", index,
-		       ARRAY_SIZE(modem_cfg));
+	struct ipc_chnl_cfg *modem_cfg = modem_cfg_7560;
+
+	if (dev->device == INTEL_CP_DEVICE_7360_ID)
+		modem_cfg = modem_cfg_7360;
+
+	if (index >= IPC_MEM_MAX_CHANNELS) {
+		pr_err("index: %d and array size %d", index,
+		       IPC_MEM_MAX_CHANNELS);
 		return -ECHRNG;
 	}
 
diff --git a/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.h b/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.h
index e77084e76718..4703c2baf3e1 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.h
@@ -6,6 +6,8 @@
 #ifndef IOSM_IPC_CHNL_CFG_H
 #define IOSM_IPC_CHNL_CFG_H
 
+#include <linux/pci.h>
+
 #include "iosm_ipc_mux.h"
 
 /* Number of TDs on the trace channel */
@@ -51,10 +53,11 @@ struct ipc_chnl_cfg {
 /**
  * ipc_chnl_cfg_get - Get pipe configuration.
  * @chnl_cfg:		Array of ipc_chnl_cfg struct
+ * @dev:		PCI device struct of the modem
  * @index:		Channel index (upto MAX_CHANNELS)
  *
  * Return: 0 on success and failure value on error
  */
-int ipc_chnl_cfg_get(struct ipc_chnl_cfg *chnl_cfg, int index);
-
+int ipc_chnl_cfg_get(struct ipc_chnl_cfg *chnl_cfg,
+		      struct pci_dev *dev, int index);
 #endif
diff --git a/drivers/net/wwan/iosm/iosm_ipc_devlink.c b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
index 17da85a8f337..e5dcff9845cc 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_devlink.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
@@ -272,7 +272,8 @@ struct iosm_devlink *ipc_devlink_init(struct iosm_imem *ipc_imem)
 		goto region_create_fail;
 	}
 
-	if (ipc_chnl_cfg_get(&chnl_cfg_flash, IPC_MEM_CTRL_CHL_ID_7) < 0)
+	if (ipc_chnl_cfg_get(&chnl_cfg_flash, ipc_imem->pcie->pci,
+			     IPC_MEM_CTRL_CHL_ID_7) < 0)
 		goto chnl_get_fail;
 
 	ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL,
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index 1e6a47976642..98055532e065 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -310,7 +310,9 @@ static void ipc_imem_dl_skb_process(struct iosm_imem *ipc_imem,
 		ipc_pcie_addr_unmap(ipc_imem->pcie, IPC_CB(skb)->len,
 				    IPC_CB(skb)->mapping,
 				    IPC_CB(skb)->direction);
-		if (port_id == IPC_MEM_CTRL_CHL_ID_7)
+		/* 7360: Channel 7 is an AT port, do not fwd to devlink */
+		if (port_id == IPC_MEM_CTRL_CHL_ID_7 &&
+		    ipc_imem->pcie->pci->device != INTEL_CP_DEVICE_7360_ID)
 			ipc_imem_sys_devlink_notify_rx(ipc_imem->ipc_devlink,
 						       skb);
 		else if (ipc_is_trace_channel(ipc_imem, port_id))
@@ -585,13 +587,9 @@ static void ipc_imem_run_state_worker(struct work_struct *instance)
 		ipc_imem->mux->wwan = ipc_imem->wwan;
 
 	while (ctrl_chl_idx < IPC_MEM_MAX_CHANNELS) {
-		if (!ipc_chnl_cfg_get(&chnl_cfg_port, ctrl_chl_idx)) {
+		if (!ipc_chnl_cfg_get(&chnl_cfg_port, ipc_imem->pcie->pci,
+				      ctrl_chl_idx)) {
 			ipc_imem->ipc_port[ctrl_chl_idx] = NULL;
-			if (ipc_imem->pcie->pci->device == INTEL_CP_DEVICE_7360_ID &&
-			    chnl_cfg_port.wwan_port_type == WWAN_PORT_MBIM) {
-				ctrl_chl_idx++;
-				continue;
-			}
 			if (chnl_cfg_port.wwan_port_type != WWAN_PORT_UNKNOWN) {
 				ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL,
 						      chnl_cfg_port,
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c
index 57304a5adf68..6fd7d4a3c9f7 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem_ops.c
@@ -90,7 +90,8 @@ void ipc_imem_wwan_channel_init(struct iosm_imem *ipc_imem,
 		return;
 	}
 
-	ipc_chnl_cfg_get(&chnl_cfg, ipc_imem->nr_of_channels);
+	ipc_chnl_cfg_get(&chnl_cfg, ipc_imem->pcie->pci,
+			 ipc_imem->nr_of_channels);
 	ipc_imem_channel_init(ipc_imem, IPC_CTYPE_WWAN, chnl_cfg,
 			      IRQ_MOD_OFF);
 
diff --git a/drivers/net/wwan/iosm/iosm_ipc_trace.c b/drivers/net/wwan/iosm/iosm_ipc_trace.c
index eeecfa3d10c5..6012f9099d6a 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_trace.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_trace.c
@@ -137,7 +137,8 @@ struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
 	struct ipc_chnl_cfg chnl_cfg = { 0 };
 	struct iosm_trace *ipc_trace;
 
-	ipc_chnl_cfg_get(&chnl_cfg, IPC_MEM_CTRL_CHL_ID_3);
+	ipc_chnl_cfg_get(&chnl_cfg, ipc_imem->pcie->pci,
+			 IPC_MEM_CTRL_CHL_ID_3);
 	ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL, chnl_cfg,
 			      IRQ_MOD_OFF);
 
-- 
2.37.3

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Jakub Kicinski]

On Sun, 25 Sep 2022 21:05:24 -0700 Shane Parslow wrote:
> This patch fixes the control channel mapping for the 7360, which was
> previously the same as the 7560.
> 
> As shown by the reverse engineering efforts of James Wah [1], the layout
> of channels on the 7360 is actually somewhat different from that of the
> 7560.
> 
> A new ipc_chnl_cfg is added specifically for the 7360. The new config
> updates channel 7 to be an AT port and removes the mbim interface, as
> it does not exist on the 7360. The config is otherwise left the same as
> the 7560. ipc_chnl_cfg_get is updated to switch between the two configs.
> In ipc_imem, a special case for the mbim port is removed as it no longer
> exists in the 7360 ipc_chnl_cfg.
> 
> As a result of this, the second userspace AT port now functions whereas
> previously it was routed to the trace channel. Modem crashes ("confused
> phase", "msg timeout", "PORT open refused") resulting from garbage being
> sent to the modem are also fixed.

What's the Fixes: tag for this one?

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Shane Parslow]

On Mon, 26 Sep 2022 13:11:09 -0700 Jakub Kicinski wrote:
> On Sun, 25 Sep 2022 21:05:24 -0700 Shane Parslow wrote:
>> This patch fixes the control channel mapping for the 7360, which was
>> previously the same as the 7560.
>>
>> As shown by the reverse engineering efforts of James Wah [1], the layout
>> of channels on the 7360 is actually somewhat different from that of the
>> 7560.
>>
>> A new ipc_chnl_cfg is added specifically for the 7360. The new config
>> updates channel 7 to be an AT port and removes the mbim interface, as
>> it does not exist on the 7360. The config is otherwise left the same as
>> the 7560. ipc_chnl_cfg_get is updated to switch between the two configs.
>> In ipc_imem, a special case for the mbim port is removed as it no longer
>> exists in the 7360 ipc_chnl_cfg.
>>
>> As a result of this, the second userspace AT port now functions whereas
>> previously it was routed to the trace channel. Modem crashes ("confused
>> phase", "msg timeout", "PORT open refused") resulting from garbage being
>> sent to the modem are also fixed.
>
> What's the Fixes: tag for this one?

There isn't currently an open bug report for this. I can open one if that
is preferred.
The gist is that previously, any writes to the 2nd userspace AT port
would crash the modem.
This is my first patch -- I apologize if I did things out of order.

On Mon, Sep 26, 2022 at 1:11 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 25 Sep 2022 21:05:24 -0700 Shane Parslow wrote:
> > This patch fixes the control channel mapping for the 7360, which was
> > previously the same as the 7560.
> >
> > As shown by the reverse engineering efforts of James Wah [1], the layout
> > of channels on the 7360 is actually somewhat different from that of the
> > 7560.
> >
> > A new ipc_chnl_cfg is added specifically for the 7360. The new config
> > updates channel 7 to be an AT port and removes the mbim interface, as
> > it does not exist on the 7360. The config is otherwise left the same as
> > the 7560. ipc_chnl_cfg_get is updated to switch between the two configs.
> > In ipc_imem, a special case for the mbim port is removed as it no longer
> > exists in the 7360 ipc_chnl_cfg.
> >
> > As a result of this, the second userspace AT port now functions whereas
> > previously it was routed to the trace channel. Modem crashes ("confused
> > phase", "msg timeout", "PORT open refused") resulting from garbage being
> > sent to the modem are also fixed.
>
> What's the Fixes: tag for this one?

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Jakub Kicinski]

On Mon, 26 Sep 2022 13:51:23 -0700 Shane Parslow wrote:
> There isn't currently an open bug report for this. I can open one if that
> is preferred.
> The gist is that previously, any writes to the 2nd userspace AT port
> would crash the modem.
> This is my first patch -- I apologize if I did things out of order.

The Fixes tag just points us to the commit which introduced the bug,
look thru the git history of the kernel for examples. The expected
format is:

        fixes = Fixes: %h (\"%s\")

You need to find the oldest commit where problem exists. This helps
stable tree maintainers to backport the necessary fixes (and netdev
maintainers to make sure that the right tree is targeted).

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Shane Parslow]

Got it, thanks. This fixes: 1f52d7b62285 ("net: wwan: iosm: Enable
M.2 7360 WWAN card support"). Thank you for your patience.

On Mon, Sep 26, 2022 at 2:37 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 26 Sep 2022 13:51:23 -0700 Shane Parslow wrote:
> > There isn't currently an open bug report for this. I can open one if that
> > is preferred.
> > The gist is that previously, any writes to the 2nd userspace AT port
> > would crash the modem.
> > This is my first patch -- I apologize if I did things out of order.
>
> The Fixes tag just points us to the commit which introduced the bug,
> look thru the git history of the kernel for examples. The expected
> format is:
>
>         fixes = Fixes: %h (\"%s\")
>
> You need to find the oldest commit where problem exists. This helps
> stable tree maintainers to backport the necessary fixes (and netdev
> maintainers to make sure that the right tree is targeted).

RE: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Kumar, M Chetan]

> -----Original Message-----
> From: Shane Parslow <shaneparslow808@gmail.com>
> Sent: Monday, September 26, 2022 9:35 AM
> To: shaneparslow808@gmail.com
> Cc: Kumar, M Chetan <m.chetan.kumar@intel.com>; linuxwwan
> <linuxwwan@intel.com>; Loic Poulain <loic.poulain@linaro.org>; Sergey
> Ryazanov <ryazanov.s.a@gmail.com>; Johannes Berg
> <johannes@sipsolutions.net>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel
> mapping
> 
> This patch fixes the control channel mapping for the 7360, which was
> previously the same as the 7560.
> 
> As shown by the reverse engineering efforts of James Wah [1], the layout of
> channels on the 7360 is actually somewhat different from that of the 7560.
> 
> A new ipc_chnl_cfg is added specifically for the 7360. The new config
> updates channel 7 to be an AT port and removes the mbim interface, as it
> does not exist on the 7360. The config is otherwise left the same as the
> 7560. ipc_chnl_cfg_get is updated to switch between the two configs.
> In ipc_imem, a special case for the mbim port is removed as it no longer
> exists in the 7360 ipc_chnl_cfg.
> 
> As a result of this, the second userspace AT port now functions whereas
> previously it was routed to the trace channel. Modem crashes ("confused
> phase", "msg timeout", "PORT open refused") resulting from garbage being
> sent to the modem are also fixed.

Trace channel is mapped to 3rd entry.

/* Trace */
{ IPC_MEM_CTRL_CHL_ID_3, IPC_MEM_PIPE_6, IPC_MEM_PIPE_7,
  IPC_MEM_TDS_TRC, IPC_MEM_TDS_TRC, IPC_MEM_MAX_DL_TRC_BUF_SIZE,
  WWAN_PORT_UNKNOWN },

I cross checked by running AT test on 7360. Both ports are functional as expected.
We should be able to send or receive AT commands with existing below config. 

/* IAT0 */
{ IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
  WWAN_PORT_AT },  -----------> wwan0at0

/* IAT1 */
{ IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
  WWAN_PORT_AT }, ------------> wwan0at1

Does this second AT port (wwan0at1) goes bad at some point or is always not functional/modem
crashes sooner we issue AT command ?

Could you please help to check the modem fw details by running below command.
at+xgendata

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Jakub Kicinski]

On Mon, 26 Sep 2022 15:21:09 -0700 Shane Parslow wrote:
> Got it, thanks. This fixes: 1f52d7b62285 ("net: wwan: iosm: Enable
> M.2 7360 WWAN card support"). Thank you for your patience.

Looks like Kumar has some questions, so let me drop the patch from 
the queue for now. Once the discussion is resolved please repost as
[PATCH net v2] and put the Fixes tag (on a single line) right above
your s-o-b, like this:


[1] https://github.com/xmm7360/reversing

Fixes: 1f52d7b62285 ("net: wwan: iosm: Enable M.2 7360 WWAN card support")
Signed-off-by: Shane Parslow <shaneparslow808@gmail.com>


Thanks!

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Shane Parslow]

On Tue, Sep 27, 2022 at 01:43:22PM +0000, Kumar, M Chetan wrote:
> > -----Original Message-----
> > From: Shane Parslow <shaneparslow808@gmail.com>
> > Sent: Monday, September 26, 2022 9:35 AM
> > To: shaneparslow808@gmail.com
> > Cc: Kumar, M Chetan <m.chetan.kumar@intel.com>; linuxwwan
> > <linuxwwan@intel.com>; Loic Poulain <loic.poulain@linaro.org>; Sergey
> > Ryazanov <ryazanov.s.a@gmail.com>; Johannes Berg
> > <johannes@sipsolutions.net>; David S. Miller <davem@davemloft.net>; Eric
> > Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> > Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel
> > mapping
> > 
> > This patch fixes the control channel mapping for the 7360, which was
> > previously the same as the 7560.
> > 
> > As shown by the reverse engineering efforts of James Wah [1], the layout of
> > channels on the 7360 is actually somewhat different from that of the 7560.
> > 
> > A new ipc_chnl_cfg is added specifically for the 7360. The new config
> > updates channel 7 to be an AT port and removes the mbim interface, as it
> > does not exist on the 7360. The config is otherwise left the same as the
> > 7560. ipc_chnl_cfg_get is updated to switch between the two configs.
> > In ipc_imem, a special case for the mbim port is removed as it no longer
> > exists in the 7360 ipc_chnl_cfg.
> > 
> > As a result of this, the second userspace AT port now functions whereas
> > previously it was routed to the trace channel. Modem crashes ("confused
> > phase", "msg timeout", "PORT open refused") resulting from garbage being
> > sent to the modem are also fixed.
> 
> Trace channel is mapped to 3rd entry.
> 
> /* Trace */
> { IPC_MEM_CTRL_CHL_ID_3, IPC_MEM_PIPE_6, IPC_MEM_PIPE_7,
>   IPC_MEM_TDS_TRC, IPC_MEM_TDS_TRC, IPC_MEM_MAX_DL_TRC_BUF_SIZE,
>   WWAN_PORT_UNKNOWN },
> 
> I cross checked by running AT test on 7360. Both ports are functional as expected.
> We should be able to send or receive AT commands with existing below config. 
> 
> /* IAT0 */
> { IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
>   IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
>   WWAN_PORT_AT },  -----------> wwan0at0
> 
> /* IAT1 */
> { IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
>   IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
>   WWAN_PORT_AT }, ------------> wwan0at1
> 
> Does this second AT port (wwan0at1) goes bad at some point or is always not functional/modem
> crashes sooner we issue AT command ?
> 
> Could you please help to check the modem fw details by running below command.
> at+xgendata

Upon further investigation, it looks like the modem crashes only occur
after S3 sleep, and are likely a different issue that this patch does
not fix. Sorry for the confusion.

I say that the channels are mapped incorrectly because upon opening
wwan0at0, I recieve "+XLCSINIT: UtaLcsInitializeRspCb received result = 0",
and no response to AT commands. The behavior I would expect, and the
behavior I get after applying the patch, is normal responses to AT
commands in the same way as wwan0at1 pre-patch.

To be clear, my patch points wwan0at0 to channel 4, and wwan0at1 to channel
7. I have perhaps been ambiguous with the terms I have been using.

To recap:
-- The modem crashes are likely an unrelated issue.
-- wwan0at0 is currently unresponsive to commands, and outputs
   "+XLCSINIT...", but responds normally post-patch.

AT+XGENDATA returns the following:
+XGENDATA: "XG736ES21S5E20NAMAV2DEFA19223101408
M2_7360_XMM7360_REV_2.1_RPC_NAND 2019-May-29 11:40:45
*XG736ES21S5E20NAMAV2DEFA19223101408__M.2_7360_MR2_01.1920.00*"
"*"
"FAB-CODE:7*SDRAMVendor=0x08 (Winbond), SDRAMRevision=0x0000"

I don't see any firmware updates online.

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Shane Parslow]

Hi Chetan, just making sure you saw my previous email, which I believe
addressed some of your concerns. Do you have any further questions or
concerns about the channel remapping patch?

RE: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Kumar, M Chetan]

> -----Original Message-----
> From: Shane Parslow <shaneparslow808@gmail.com>
> Sent: Wednesday, September 28, 2022 11:37 AM
> To: Kumar, M Chetan <m.chetan.kumar@intel.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control
> channel mapping
> 
> On Tue, Sep 27, 2022 at 01:43:22PM +0000, Kumar, M Chetan wrote:
> > > -----Original Message-----
> > > From: Shane Parslow <shaneparslow808@gmail.com>
> > > Sent: Monday, September 26, 2022 9:35 AM
> > > To: shaneparslow808@gmail.com
> > > Cc: Kumar, M Chetan <m.chetan.kumar@intel.com>; linuxwwan
> > > <linuxwwan@intel.com>; Loic Poulain <loic.poulain@linaro.org>;
> > > Sergey Ryazanov <ryazanov.s.a@gmail.com>; Johannes Berg
> > > <johannes@sipsolutions.net>; David S. Miller <davem@davemloft.net>;
> > > Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> > > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> > > netdev@vger.kernel.org; linux- kernel@vger.kernel.org
> > > Subject: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control
> > > channel mapping
> > >
> > > This patch fixes the control channel mapping for the 7360, which was
> > > previously the same as the 7560.
> > >
> > > As shown by the reverse engineering efforts of James Wah [1], the
> > > layout of channels on the 7360 is actually somewhat different from that
> of the 7560.
> > >
> > > A new ipc_chnl_cfg is added specifically for the 7360. The new
> > > config updates channel 7 to be an AT port and removes the mbim
> > > interface, as it does not exist on the 7360. The config is otherwise
> > > left the same as the 7560. ipc_chnl_cfg_get is updated to switch between
> the two configs.
> > > In ipc_imem, a special case for the mbim port is removed as it no
> > > longer exists in the 7360 ipc_chnl_cfg.
> > >
> > > As a result of this, the second userspace AT port now functions
> > > whereas previously it was routed to the trace channel. Modem crashes
> > > ("confused phase", "msg timeout", "PORT open refused") resulting
> > > from garbage being sent to the modem are also fixed.
> >
> > Trace channel is mapped to 3rd entry.
> >
> > /* Trace */
> > { IPC_MEM_CTRL_CHL_ID_3, IPC_MEM_PIPE_6, IPC_MEM_PIPE_7,
> >   IPC_MEM_TDS_TRC, IPC_MEM_TDS_TRC,
> IPC_MEM_MAX_DL_TRC_BUF_SIZE,
> >   WWAN_PORT_UNKNOWN },
> >
> > I cross checked by running AT test on 7360. Both ports are functional as
> expected.
> > We should be able to send or receive AT commands with existing below
> config.
> >
> > /* IAT0 */
> > { IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
> >   IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT,
> IPC_MEM_MAX_DL_AT_BUF_SIZE,
> >   WWAN_PORT_AT },  -----------> wwan0at0
> >
> > /* IAT1 */
> > { IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
> >   IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT,
> IPC_MEM_MAX_DL_AT_BUF_SIZE,
> >   WWAN_PORT_AT }, ------------> wwan0at1
> >
> > Does this second AT port (wwan0at1) goes bad at some point or is
> > always not functional/modem crashes sooner we issue AT command ?
> >
> > Could you please help to check the modem fw details by running below
> command.
> > at+xgendata
> 
> Upon further investigation, it looks like the modem crashes only occur after
> S3 sleep, and are likely a different issue that this patch does not fix. Sorry for
> the confusion.
> 
> I say that the channels are mapped incorrectly because upon opening
> wwan0at0, I recieve "+XLCSINIT: UtaLcsInitializeRspCb received result = 0",
> and no response to AT commands. The behavior I would expect, and the
> behavior I get after applying the patch, is normal responses to AT commands
> in the same way as wwan0at1 pre-patch.
> 
> To be clear, my patch points wwan0at0 to channel 4, and wwan0at1 to
> channel 7. I have perhaps been ambiguous with the terms I have been using.

I cross checked the channel mapping. It is proper.
In the present implementation 2 AT port are enabled and are mapped to channel 2 & 4.

	/* IAT0 */
	{ IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
	  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
	  WWAN_PORT_AT },

	/* IAT1 */
	{ IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
	  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
	  WWAN_PORT_AT },

The channel 7 you are mapping to wwanat1 is an additional AT channel which we have not mapped to/enabled by default.

I flashed the same version of FW (1920) on my setup and I see both the AT ports are functional as expected. If mapping was not 
proper channel would not have returned response to any AT commands and the reason why you are seeing "+XLCSINIT:" is, 
in that version of modem FW the GNSS module is returning UNSOL command on initialization. This was fixed in later fw.

But one issue I noticed is, modem is crashing in S3 sleep. This issue is also not observed with latest modem fw.

> 
> To recap:
> -- The modem crashes are likely an unrelated issue.
> -- wwan0at0 is currently unresponsive to commands, and outputs
>    "+XLCSINIT...", but responds normally post-patch.
> 
> AT+XGENDATA returns the following:
> +XGENDATA: "XG736ES21S5E20NAMAV2DEFA19223101408
> M2_7360_XMM7360_REV_2.1_RPC_NAND 2019-May-29 11:40:45
> *XG736ES21S5E20NAMAV2DEFA19223101408__M.2_7360_MR2_01.1920.00
> *"
> "*"
> "FAB-CODE:7*SDRAMVendor=0x08 (Winbond), SDRAMRevision=0x0000"
> 
> I don't see any firmware updates online.

Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control channel mapping

[Shane Parslow]

On Fri, Oct 21, 2022 at 02:04:37PM +0000, Kumar, M Chetan wrote:
> > -----Original Message-----
> > From: Shane Parslow <shaneparslow808@gmail.com>
> > Sent: Wednesday, September 28, 2022 11:37 AM
> > To: Kumar, M Chetan <m.chetan.kumar@intel.com>
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control
> > channel mapping
> > 
> > On Tue, Sep 27, 2022 at 01:43:22PM +0000, Kumar, M Chetan wrote:
> > > > -----Original Message-----
> > > > From: Shane Parslow <shaneparslow808@gmail.com>
> > > > Sent: Monday, September 26, 2022 9:35 AM
> > > > To: shaneparslow808@gmail.com
> > > > Cc: Kumar, M Chetan <m.chetan.kumar@intel.com>; linuxwwan
> > > > <linuxwwan@intel.com>; Loic Poulain <loic.poulain@linaro.org>;
> > > > Sergey Ryazanov <ryazanov.s.a@gmail.com>; Johannes Berg
> > > > <johannes@sipsolutions.net>; David S. Miller <davem@davemloft.net>;
> > > > Eric Dumazet <edumazet@google.com>; Jakub Kicinski
> > > > <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> > > > netdev@vger.kernel.org; linux- kernel@vger.kernel.org
> > > > Subject: [PATCH net] net: wwan: iosm: Fix 7360 WWAN card control
> > > > channel mapping
> > > >
> > > > This patch fixes the control channel mapping for the 7360, which was
> > > > previously the same as the 7560.
> > > >
> > > > As shown by the reverse engineering efforts of James Wah [1], the
> > > > layout of channels on the 7360 is actually somewhat different from that
> > of the 7560.
> > > >
> > > > A new ipc_chnl_cfg is added specifically for the 7360. The new
> > > > config updates channel 7 to be an AT port and removes the mbim
> > > > interface, as it does not exist on the 7360. The config is otherwise
> > > > left the same as the 7560. ipc_chnl_cfg_get is updated to switch between
> > the two configs.
> > > > In ipc_imem, a special case for the mbim port is removed as it no
> > > > longer exists in the 7360 ipc_chnl_cfg.
> > > >
> > > > As a result of this, the second userspace AT port now functions
> > > > whereas previously it was routed to the trace channel. Modem crashes
> > > > ("confused phase", "msg timeout", "PORT open refused") resulting
> > > > from garbage being sent to the modem are also fixed.
> > >
> > > Trace channel is mapped to 3rd entry.
> > >
> > > /* Trace */
> > > { IPC_MEM_CTRL_CHL_ID_3, IPC_MEM_PIPE_6, IPC_MEM_PIPE_7,
> > >   IPC_MEM_TDS_TRC, IPC_MEM_TDS_TRC,
> > IPC_MEM_MAX_DL_TRC_BUF_SIZE,
> > >   WWAN_PORT_UNKNOWN },
> > >
> > > I cross checked by running AT test on 7360. Both ports are functional as
> > expected.
> > > We should be able to send or receive AT commands with existing below
> > config.
> > >
> > > /* IAT0 */
> > > { IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
> > >   IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT,
> > IPC_MEM_MAX_DL_AT_BUF_SIZE,
> > >   WWAN_PORT_AT },  -----------> wwan0at0
> > >
> > > /* IAT1 */
> > > { IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
> > >   IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT,
> > IPC_MEM_MAX_DL_AT_BUF_SIZE,
> > >   WWAN_PORT_AT }, ------------> wwan0at1
> > >
> > > Does this second AT port (wwan0at1) goes bad at some point or is
> > > always not functional/modem crashes sooner we issue AT command ?
> > >
> > > Could you please help to check the modem fw details by running below
> > command.
> > > at+xgendata
> > 
> > Upon further investigation, it looks like the modem crashes only occur after
> > S3 sleep, and are likely a different issue that this patch does not fix. Sorry for
> > the confusion.
> > 
> > I say that the channels are mapped incorrectly because upon opening
> > wwan0at0, I recieve "+XLCSINIT: UtaLcsInitializeRspCb received result = 0",
> > and no response to AT commands. The behavior I would expect, and the
> > behavior I get after applying the patch, is normal responses to AT commands
> > in the same way as wwan0at1 pre-patch.
> > 
> > To be clear, my patch points wwan0at0 to channel 4, and wwan0at1 to
> > channel 7. I have perhaps been ambiguous with the terms I have been using.
> 
> I cross checked the channel mapping. It is proper.
> In the present implementation 2 AT port are enabled and are mapped to channel 2 & 4.
> 
> 	/* IAT0 */
> 	{ IPC_MEM_CTRL_CHL_ID_2, IPC_MEM_PIPE_4, IPC_MEM_PIPE_5,
> 	  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
> 	  WWAN_PORT_AT },
> 
> 	/* IAT1 */
> 	{ IPC_MEM_CTRL_CHL_ID_4, IPC_MEM_PIPE_8, IPC_MEM_PIPE_9,
> 	  IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_TDS_AT, IPC_MEM_MAX_DL_AT_BUF_SIZE,
> 	  WWAN_PORT_AT },
> 
> The channel 7 you are mapping to wwanat1 is an additional AT channel which we have not mapped to/enabled by default.
> 
> I flashed the same version of FW (1920) on my setup and I see both the AT ports are functional as expected. If mapping was not 
> proper channel would not have returned response to any AT commands and the reason why you are seeing "+XLCSINIT:" is, 
> in that version of modem FW the GNSS module is returning UNSOL command on initialization. This was fixed in later fw.
> 
> But one issue I noticed is, modem is crashing in S3 sleep. This issue is also not observed with latest modem fw.
> 
> > 
> > To recap:
> > -- The modem crashes are likely an unrelated issue.
> > -- wwan0at0 is currently unresponsive to commands, and outputs
> >    "+XLCSINIT...", but responds normally post-patch.
> > 
> > AT+XGENDATA returns the following:
> > +XGENDATA: "XG736ES21S5E20NAMAV2DEFA19223101408
> > M2_7360_XMM7360_REV_2.1_RPC_NAND 2019-May-29 11:40:45
> > *XG736ES21S5E20NAMAV2DEFA19223101408__M.2_7360_MR2_01.1920.00
> > *"
> > "*"
> > "FAB-CODE:7*SDRAMVendor=0x08 (Winbond), SDRAMRevision=0x0000"
> > 
> > I don't see any firmware updates online.

Looks like my vendor's firmware is a bit behind upstream. Thanks for bearing
with me.

On another note, what would your opinions be on exposing the RPC interface
to userspace? My understanding is that the 7360 does not have an MBIM
interface, so configuration via RPC is required to bring up the modem.

Is an MBIM interface added in a later firmware version? Are there problems
with this approach?

It seems to me that this would be the last step in making the 7360 usable.

I'm imagining something along the lines of adding a WWAN_PORT_RPC to
wwan_core.c and updating iosm_ipc_chnl_cfg.c

Such an interface should be able to work with the existing xmm7360-pci
tool.