Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <yonglin.tan@outlook.com> <MEYP282MB237454C97E14056454FBBC77FD3A9@MEYP282MB2374.AUSP282.PROD.OUTLOOK.COM>
In-Reply-To: <MEYP282MB237454C97E14056454FBBC77FD3A9@MEYP282MB2374.AUSP282.PROD.OUTLOOK.COM>
From:   Loic Poulain <loic.poulain@linaro.org>
Date:   Mon, 21 Feb 2022 17:03:12 +0100
Message-ID: <CAMZdPi_7KGx69s5tFumkswVXiQSdxXZjDXT5f9njRnBNz1k-VA@mail.gmail.com>
Subject: Re: [PATCH] net: wwan: To support SAHARA port for Qualcomm WWAN module.
To:     Yonglin Tan <yonglin.tan@outlook.com>
Cc:     ryazanov.s.a@gmail.com, johannes@sipsolutions.net,
        davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Yonglin,

On Mon, 21 Feb 2022 at 13:21, Yonglin Tan <yonglin.tan@outlook.com> wrote:
>
> The SAHARA port for Qualcomm WWAN module is used to capture
> memory dump. But now this feature has not been supported by
> linux kernel code. Such that no SAHARA driver matched while
> the device entered to DUMP mode. Once the device crashed due
> to some reasons, device will enter into DUMP mode and running
> in SBL stage. After that, the device change EE to SBL and the
> host will detect the EE change event and re-enumerate SAHARA
> port.
>
> Cc: stable@vger.kernel.org
> Fixes: fa588eba632d ("net: Add Qcom WWAN control driver")
> Signed-off-by: Yonglin Tan <yonglin.tan@outlook.com>
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Sorry, but I've not yet offered that tag :-)

The WWAN framework is a generic way to expose a WWAN device and its
related control/data protocols, such as AT, QMI, MBIM, QCDM, etc...
All the exposed protocols are supported by open-source user
tools/daemons such as ModemManager, ofono, fwupd... SAHARA does not
seem to be WWAN specific and is not something needed for controlling a
modem, right?

I know it would be easier to just add this channel to the WWAN ports,
but we don't want to rawly expose something that could fit into an
existing framework/subsystem, that's why I referred to the devcoredump
framework, which 'seems' a better place for its integration. But I
could be wrong, I don't know much about devcoredump and maybe SAHARA
is doing much more than a firmware coredump...

As a last resort, I think this kind of debug interface should go to debugfs.

Regards,
Loic


> ---
>  drivers/net/wwan/mhi_wwan_ctrl.c | 1 +
>  drivers/net/wwan/wwan_core.c     | 4 ++++
>  include/linux/wwan.h             | 1 +
>  3 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> index e4d0f69..4cf420e 100644
> --- a/drivers/net/wwan/mhi_wwan_ctrl.c
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -262,6 +262,7 @@ static const struct mhi_device_id mhi_wwan_ctrl_match_table[] = {
>         { .chan = "QMI", .driver_data = WWAN_PORT_QMI },
>         { .chan = "DIAG", .driver_data = WWAN_PORT_QCDM },
>         { .chan = "FIREHOSE", .driver_data = WWAN_PORT_FIREHOSE },
> +       { .chan = "SAHARA", .driver_data = WWAN_PORT_SAHARA },
>         {},
>  };
>  MODULE_DEVICE_TABLE(mhi, mhi_wwan_ctrl_match_table);
> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> index b8c7843..2630677 100644
> --- a/drivers/net/wwan/wwan_core.c
> +++ b/drivers/net/wwan/wwan_core.c
> @@ -318,6 +318,10 @@ static const struct {
>                 .name = "FIREHOSE",
>                 .devsuf = "firehose",
>         },
> +       [WWAN_PORT_SAHARA] = {
> +               .name = "SAHARA",
> +               .devsuf = "sahara",
> +       },
>  };
>
>  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 5ce2acf..fc8ecaf 100644
> --- a/include/linux/wwan.h
> +++ b/include/linux/wwan.h
> @@ -26,6 +26,7 @@ enum wwan_port_type {
>         WWAN_PORT_QMI,
>         WWAN_PORT_QCDM,
>         WWAN_PORT_FIREHOSE,
> +       WWAN_PORT_SAHARA,
>
>         /* Add new port types above this line */
>
> --
> 2.7.4
>