Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 28 Oct 2022 11:09:24 -0700
From:   Shane Parslow <shaneparslow808@gmail.com>
To:     Loic Poulain <loic.poulain@linaro.org>
Cc:     m.chetan.kumar@intel.com, linuxwwan@intel.com,
        ryazanov.s.a@gmail.com, johannes@sipsolutions.net,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] net: wwan: iosm: add rpc interface for xmm
 modems
Message-ID: <Y1wa1JR9URGCUo1P@arch-desk>
References: <20221028003128.514318-1-shaneparslow808@gmail.com>
 <CAMZdPi-tz4_vxum8SYbYVuv71UYhe4QUGO6_w8TPFBcw9oydfQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMZdPi-tz4_vxum8SYbYVuv71UYhe4QUGO6_w8TPFBcw9oydfQ@mail.gmail.com>
Precedence: bulk

On Fri, Oct 28, 2022 at 12:35:01PM +0200, Loic Poulain wrote:
> On Fri, 28 Oct 2022 at 02:37, Shane Parslow <shaneparslow808@gmail.com> wrote:
> >
> > Add a new iosm wwan port that connects to the modem rpc interface. This
> > interface provides a configuration channel, and in the case of the 7360, is
> > the only way to configure the modem (as it does not support mbim).
> 
> Doesn't the AT channel offer that possibility? what is the status of
> 7360 support without this change?

Several initialization functions must be called through the RPC channel
to bring up the 7360. Without this initialization the modem is not
functional beyond responding to simple AT commands. After initialization
through this interface the modem works as expected.

Because of this, the 7360 is currently nonfunctional beyond responding
to a limited set of AT commands. As for the 7560, my understanding is that
it is currently functional, and this interface simply supplements the MBIM
interface.

> 
> > The new interface is compatible with existing software, such as
> > open_xdatachannel.py from the xmm7360-pci project [1].
> >
> > [1] https://github.com/xmm7360/xmm7360-pci
> >
> > Signed-off-by: Shane Parslow <shaneparslow808@gmail.com>
> > ---
> >  drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c | 2 +-
> >  drivers/net/wwan/wwan_core.c              | 4 ++++
> >  include/linux/wwan.h                      | 2 ++
> >  3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c b/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c
> > index 128c999e08bb..91e3e83fc47b 100644
> > --- a/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c
> > +++ b/drivers/net/wwan/iosm/iosm_ipc_chnl_cfg.c
> > @@ -39,7 +39,7 @@ static struct ipc_chnl_cfg modem_cfg[] = {
> >         /* 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 },
> > +         IPC_MEM_MAX_DL_RPC_BUF_SIZE, WWAN_PORT_RPC },
> >         /* 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,
> > diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> > index 62e9f7d6c9fe..cf16a2704914 100644
> > --- a/drivers/net/wwan/wwan_core.c
> > +++ b/drivers/net/wwan/wwan_core.c
> > @@ -319,6 +319,10 @@ static const struct {
> >                 .name = "FIREHOSE",
> >                 .devsuf = "firehose",
> >         },
> > +       [WWAN_PORT_RPC] = {
> > +               .name = "RPC",
> > +               .devsuf = "rpc",
> > +       },
> 
> RPC sounds more like a generic method than an actual XMM-specific
> protocol, isn't there a more precise name for that protocol? if not,
> maybe XMMRPC is more appropriate?

This is the only name I've seen it referred to by. I think XMMRPC/xmmrpc
will have to work.

> 
> >  };
> >
> >  static ssize_t type_show(struct device *dev, struct device_attribute *attr,
> > diff --git a/include/linux/wwan.h b/include/linux/wwan.h
> > index 5ce2acf444fb..3cf2182ad4e9 100644
> > --- a/include/linux/wwan.h
> > +++ b/include/linux/wwan.h
> > @@ -15,6 +15,7 @@
> >   * @WWAN_PORT_QMI: Qcom modem/MSM interface for modem control
> >   * @WWAN_PORT_QCDM: Qcom Modem diagnostic interface
> >   * @WWAN_PORT_FIREHOSE: XML based command protocol
> > + * @WWAN_PORT_RPC: Control protocol for Intel XMM modems
> >   *
> >   * @WWAN_PORT_MAX: Highest supported port types
> >   * @WWAN_PORT_UNKNOWN: Special value to indicate an unknown port type
> > @@ -26,6 +27,7 @@ enum wwan_port_type {
> >         WWAN_PORT_QMI,
> >         WWAN_PORT_QCDM,
> >         WWAN_PORT_FIREHOSE,
> > +       WWAN_PORT_RPC,
> >
> >         /* Add new port types above this line */
> >
> > --
> > 2.38.1
> >
> 
> Reagrds,
> Loic

Thanks for the feedback. Should I go ahead and submit a V2?