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

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

[Loic Poulain]

Hi Yonglin,

On Mon, 21 Feb 2022 at 09:42, 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.

So this is SAHARA debug mode? Can you share an example of usage and
tool to communicate via this channel, AFAIU SAHARA is already partly
supported in MHI stack to load the firehose programmer automatically,
so can you elaborate a bit on how it works?

Regards,
Loic


>
> Cc: stable@vger.kernel.org
> Fixes: fa588eba632d ("net: Add Qcom WWAN control driver")
> Signed-off-by: Yonglin Tan <yonglin.tan@outlook.com>
> ---
>  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
>

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

[Loic Poulain]

On Mon, 21 Feb 2022 at 10:27, 谭 永林 <yonglin.tan@outlook.com> wrote:
>
> Dear Loic,
>
> The Qualcomm x24&x55&x6x platform are all using SAHARA port to collect memory dump log. User can use QLOG tool to collect the whole memory dump of device while the device entered into Dump mode. The firehose protocol is used to download FW from the host, it's different from SAHARA. MHI CH 2&3 are used as SAHARA CH but firehose use 34&35 CH.

Ah right, firehose programmer is loaded when entering EDL mode, not SBL.

>
> The bus driver use " MHI_CHANNEL_CONFIG_UL&DL_SBL " to initiate SAHARA CH configs.
>
> The code is Quectel EM1XX channel config.
> MHI_CHANNEL_CONFIG_DL_SBL(3, "SAHARA", 32, 0),
>
> For example,
> - Once the device crashed due to some reasons.
> - The module entered into DUMP mode(SBL Stage), and changed the EE to SBL.
> - Then the host should detect the change and re-enumerate the SAHARA port.
> - Then users can use QLOG tool to collect the Device Memory LOG by SAHARA Protocol via CH 2&3.

OK, you should add that to the commit message.

Also, instead of exposing the raw sahara protocol, can't this be
integrated with the devcoredump framework
(https://lwn.net/Articles/610887/)?

Regards,
Loic

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

[Loic Poulain]

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
>

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

[Sergey Ryazanov]

Hello Yonglin,

On Mon, Feb 21, 2022 at 7:21 PM 谭 永林 <yonglin.tan@outlook.com> wrote:
> SAHARA protocol is used not only to dump the memory but also to enable FW downloading.
> The protocol is designed primarily for transferring software images from a host to a target and provides a simple mechanism for requesting data to be transferred over any physical link.
>
> To conclude, the SAHARA port provide a mechanism to transfer messages between host and device during SBL.
>
> -----邮件原件-----
> 发件人: Loic Poulain <loic.poulain@linaro.org>
> 发送时间: 2022年2月22日 0:03
> 收件人: Yonglin Tan <yonglin.tan@outlook.com>
> 抄送: 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
> 主题: Re: [PATCH] net: wwan: To support SAHARA port for Qualcomm WWAN module.
>
> 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.

Here I agree with Loic. An interface that is exported through the WWAN
subsystem should be common enough for WWAN devices. For a firmware
crash dump collection, there are better solutions like devcoredump as
already pointed out by Loic. The firmware update function can be
implemented using the devlink firmware flashing API. You could see the
use of both APIs in the IOSM driver.

If none of these interfaces can be used to utilize the SAHARA
functionality, then, as Loic has already pointed out, such an
interface should probably go to the debugfs. But I suggest to
implement the firmware management functionality using the generic
kernel API. This will save time for end users, distro maintainers and
probably for modem vendor support too.

-- 
Sergey