Re: [PATCH] net: wwan: To support SAHARA port for Qualcomm WWAN module.

* Re: [PATCH] net: wwan: To support SAHARA port for Qualcomm WWAN module.
       [not found] <MEYP282MB237454C97E14056454FBBC77FD3A9@MEYP282MB2374.AUSP282.PROD.OUTLOOK.COM>
@ 2022-02-21 16:03 ` Loic Poulain
       [not found]   ` <MEYP282MB2374BDAB34B7AE65C1246A44FD3A9@MEYP282MB2374.AUSP282.PROD.OUTLOOK.COM>
  0 siblings, 1 reply; 4+ messages in thread
From: Loic Poulain @ 2022-02-21 16:03 UTC (permalink / raw)
  To: Yonglin Tan
  Cc: ryazanov.s.a, johannes, davem, kuba, netdev, linux-kernel, stable

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
>

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