2023-09-12 13:31:30

by Jinjian Song

[permalink] [raw]
Subject: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Adds support for t7xx wwan device firmware flashing & coredump collection
using devlink.

On early detection of wwan device in fastboot mode driver sets up CLDMA0 HW
tx/rx queues for raw data transfer and then registers to devlink framework.
On user space application issuing command for firmware update the driver
sends fastboot flash command & firmware to program NAND.

In flashing procedure the fastboot command & response are exchanged between
driver and device. Once firmware flashing is success, user space application
get modem event by sysfs interface.

The devlink param fastboot is set to true via devlink param command.

$ devlink dev param set pci/0000:bdf name fastboot value 1 cmode driverinit

The wwan device is put into fastboot mode via devlink reload command, by
passing `driver_reinit`.

$ devlink dev reload pci/0000:$bdf action driver_reinit

Note: user space application get the fastboot download event of devcie
from /sys/bus/pci/devices/${bdf}/t7xx_event then do remove(echo 1 >
/sys/bus/pci/devices/${bdf}/remove) and rescan(echo 1 > /sys/bus/pci/rescan)
to let driver goes to firmware flash process.

Below is the devlink command usage for firmware flashing

$ devlink dev flash pci/$BDF file ABC.img component ABC

Note: ABC.img is the firmware to be programmed to "ABC" partition.

In case of coredump collection when wwan device encounters an exception
it reboots & stays in fastboot mode for coredump collection by host driver.
On detecting exception state driver collects the core dump, creates the
devlink region & reports an event to user space application for dump
collection. The user space application invokes devlink region read command
for dump collection.

Below are the devlink commands used for coredump collection.

$ devlink region new pci/$BDF/mr_dump
$ devlink region read pci/$BDF/mr_dump snapshot $ID address $ADD length $LEN
$ devlink region del pci/$BDF/mr_dump snapshot $ID

Upon completion of firmware flashing or coredump collection the wwan device
is reset to normal mode using devlink reload command, by passing `fw_activate`.

$ devlink dev reload pci/0000:$bdf action fw_activate

Note: user space application get the reset event of devcie
from /sys/bus/pci/devices/${bdf}/t7xx_event then do remove(echo 1 >
/sys/bus/pci/devices/${bdf}/remove) and rescan(echo 1 > /sys/bus/pci/rescan)
to let driver goes to normal process.

Jinjian Song (5):
net: wwan: t7xx: Infrastructure for early port configuration
net: wwan: t7xx: Register with devlink and implement firmware flashing
net: wwan: t7xx: Creates region & snapshot for coredump log collection
net: wwan: t7xx: Adds sysfs attribute of modem event
net: wwan: t7xx: Devlink documentation

Documentation/networking/devlink/index.rst | 1 +
Documentation/networking/devlink/t7xx.rst | 232 +++++++
drivers/net/wwan/Kconfig | 1 +
drivers/net/wwan/t7xx/Makefile | 4 +-
drivers/net/wwan/t7xx/t7xx_hif_cldma.c | 47 +-
drivers/net/wwan/t7xx/t7xx_hif_cldma.h | 18 +-
drivers/net/wwan/t7xx/t7xx_modem_ops.c | 5 +-
drivers/net/wwan/t7xx/t7xx_pci.c | 79 ++-
drivers/net/wwan/t7xx/t7xx_pci.h | 19 +
drivers/net/wwan/t7xx/t7xx_port.h | 6 +
drivers/net/wwan/t7xx/t7xx_port_ap_msg.c | 78 +++
drivers/net/wwan/t7xx/t7xx_port_ap_msg.h | 11 +
drivers/net/wwan/t7xx/t7xx_port_flash_dump.c | 695 +++++++++++++++++++
drivers/net/wwan/t7xx/t7xx_port_flash_dump.h | 85 +++
drivers/net/wwan/t7xx/t7xx_port_proxy.c | 118 +++-
drivers/net/wwan/t7xx/t7xx_port_proxy.h | 14 +
drivers/net/wwan/t7xx/t7xx_port_wwan.c | 27 +-
drivers/net/wwan/t7xx/t7xx_reg.h | 28 +-
drivers/net/wwan/t7xx/t7xx_state_monitor.c | 137 +++-
drivers/net/wwan/t7xx/t7xx_state_monitor.h | 1 +
20 files changed, 1528 insertions(+), 78 deletions(-)
create mode 100644 Documentation/networking/devlink/t7xx.rst
create mode 100644 drivers/net/wwan/t7xx/t7xx_port_ap_msg.c
create mode 100644 drivers/net/wwan/t7xx/t7xx_port_ap_msg.h
create mode 100644 drivers/net/wwan/t7xx/t7xx_port_flash_dump.c
create mode 100644 drivers/net/wwan/t7xx/t7xx_port_flash_dump.h

--
2.34.1


2023-09-13 12:15:28

by Jiri Pirko

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:

pw-bot: changes-requested

2023-09-13 20:22:12

by Jiri Pirko

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>Adds support for t7xx wwan device firmware flashing & coredump collection
>using devlink.

I don't believe that use of devlink is correct here. It seems like a
misfit. IIUC, what you need is to communicate with the modem. Basically
a communication channel to modem. The other wwan drivers implement these
channels in _ctrl.c files, using multiple protocols. Why can't you do
something similar and let devlink out of this please?

Until you put in arguments why you really need devlink and why is it a
good fit, I'm against this. Please don't send any other versions of this
patchset that use devlink.

NACK.

2023-09-18 11:47:37

by Jinjian Song

[permalink] [raw]
Subject: RE: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>>Adds support for t7xx wwan device firmware flashing & coredump
>>collection using devlink.

>I don't believe that use of devlink is correct here. It seems like a misfit. IIUC, what you need is to communicate with the modem. Basically a communication channel to modem. The other wwan drivers implement these channels in _ctrl.c files, using multiple protocols. Why can't you do something similar and let devlink out of this please?

>Until you put in arguments why you really need devlink and why is it a good fit, I'm against this. Please don't send any other versions of this patchset that use devlink.

Yes, t7xx driver need communicate with modem with a communication channel to modem.
I took a look at the _ctrl.c files under wwan directory, it seemed the implementation can be well integrated with QualCommon's modem, if we do like this, I think we need modem firmware change, maybe not be suitable for current MTK modem directly.
Except for Qualcomm modem driver, there is also an Intel wwan driver 'iosm' and it use devlink to implement firmware flash(https://www.kernel.org/doc/html/latest/networking/devlink/iosm.html), Intel and MTK design and use devlink to do this work on
'mtk_t7xx' driver and I continue to do this work.

I think devlink framework can support this scene and if we use devlink we don't need to develop other flash tools or other user space applications, use upstream devlink commands directly.

Thanks.
>NACK.

2023-09-21 17:00:14

by Loic Poulain

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <[email protected]> wrote:
>
> Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
> >Adds support for t7xx wwan device firmware flashing & coredump collection
> >using devlink.
>
> I don't believe that use of devlink is correct here. It seems like a
> misfit. IIUC, what you need is to communicate with the modem. Basically
> a communication channel to modem. The other wwan drivers implement these
> channels in _ctrl.c files, using multiple protocols. Why can't you do
> something similar and let devlink out of this please?
>
> Until you put in arguments why you really need devlink and why is it a
> good fit, I'm against this. Please don't send any other versions of this
> patchset that use devlink.

The t7xx driver already has regular wwan data and control interfaces
registered with the wwan framework, making it functional. Here the
exposed low level resources are not really wwan/class specific as it
is for firmware upgrade and coredump, so I think that is why Jinjian
chose the 'feature agnostic' devlink framework. IMHO I think it makes
sense to rely on such a framework, or maybe on the devcoredump class.

That said, I see the protocol for flashing and doing the coreboot is
fastboot, which is already supported on the user side with the
fastboot tool, so I'm not sure abstracting it here makes sense. If the
protocol is really fasboot compliant, Wouldn't it be simpler to
directly expose it as a new device/channel? and rely on a userspace
tool for regular fastboot operations (flash, boot, dump). This may
require slightly modifying the fastboot tool to detect and support
that new transport (in addition to the existing usb and ethernet
support).

Regards,
Loic

2023-10-08 03:20:03

by Jinjian Song

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

>On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <[email protected]> wrote:
>>
>> Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>> >Adds support for t7xx wwan device firmware flashing & coredump
>> >collection using devlink.
>>
>> I don't believe that use of devlink is correct here. It seems like a
>> misfit. IIUC, what you need is to communicate with the modem.
>> Basically a communication channel to modem. The other wwan drivers
>> implement these channels in _ctrl.c files, using multiple protocols.
>> Why can't you do something similar and let devlink out of this please?
>>
>> Until you put in arguments why you really need devlink and why is it a
>> good fit, I'm against this. Please don't send any other versions of
>> this patchset that use devlink.

>The t7xx driver already has regular wwan data and control interfaces registered with the wwan framework, making it functional. Here the exposed low level resources are not really wwan/class specific as it is for firmware upgrade and coredump, so I think that is why Jinjian chose the 'feature agnostic' devlink framework. IMHO I
>think it makes sense to rely on such a framework, or maybe on the devcoredump class.

>That said, I see the protocol for flashing and doing the coreboot is fastboot, which is already supported on the user side with the fastboot tool, so I'm not sure abstracting it here makes sense. If the protocol is really fasboot compliant, Wouldn't it be simpler to directly expose it as a new device/channel? and rely on a userspace
>tool for regular fastboot operations (flash, boot, dump). This may require slightly modifying the fastboot tool to detect and support that new transport (in addition to the existing usb and ethernet support).

As far as I know, these patch set created by Intel colleague. later, it was handed over to me for further follow-up. I remember Intel colleague mentioned that using devlink framework is an open-source suggestion, but I unable to verify temporarily.
After I read the devlink framework, I found that the components of devlink like flash and region can be used to implement the features of firmware flashing and coredump collection without other user space application and wwan iosm driver also use devlink framework.

Yes, it seemed mtk_t7xx driver can use fastboot protocol for flashing with fastboot tool but I think this will result in some other tasks completing these features:
1.Driver export a port to user space for fastboot tool and this port must be scan by fastboot tool for firmware flashing feature.
2.A new tool will be needed for coredump feature and driver will adapter for this tool for modem device use the same channel for fastboot and coredump.

IF we use devlink framework, it seemed more complete and unified. So I hope it can be allowed and I would like to confirm if the current driver can continue using devlink?
Hope to get your help to make sure whether t7xx can use devlink, if can't , we may need to consider other solutions.

Regards,
JinJian

2023-10-22 14:54:01

by Jinjian Song

[permalink] [raw]
Subject: RE: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

>On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <[email protected]> wrote:
>>
>> Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>> >Adds support for t7xx wwan device firmware flashing & coredump
>> >collection using devlink.
>>
>> I don't believe that use of devlink is correct here. It seems like a
>> misfit. IIUC, what you need is to communicate with the modem.
>> Basically a communication channel to modem. The other wwan drivers
>> implement these channels in _ctrl.c files, using multiple protocols.
>> Why can't you do something similar and let devlink out of this please?
>>
>> Until you put in arguments why you really need devlink and why is it a
>> good fit, I'm against this. Please don't send any other versions of
>> this patchset that use devlink.

>The t7xx driver already has regular wwan data and control interfaces registered with the wwan framework, making it functional. Here the exposed low level resources are not really wwan/class specific as it is for firmware upgrade >and coredump, so I think that is why Jinjian chose the 'feature agnostic' devlink framework. IMHO I think it makes sense to rely on such a framework, or maybe on the devcoredump class.

>That said, I see the protocol for flashing and doing the coreboot is fastboot, which is already supported on the user side with the fastboot tool, so I'm not sure abstracting it here makes sense. If the protocol is really fasboot compliant, Wouldn't it be simpler to directly expose it as a new device/channel? and rely on a userspace tool for regular fastboot operations (flash, boot, dump). This may require slightly modifying the fastboot tool to detect and support that new transport (in addition to the existing usb and ethernet support).

I think this is the advantages of using devlink to implement flash and dump collect:
1.Devlink framework provide the interface of flash and dump, and no need to develop corresponding tools anymore. From another perspective, using devlnik can directly reduce the complexity of PC manufacturer's customer production lines, helping to reduce their costs(time/production).
2.Currently, the platform architecture of each WWAN module manufacturer is not compatible, Qualcomm implement communicate channels in host driver and using fastboot tool to flash, using their coredump tool to collect dump. Intel implement their host driver in devlink framework, using devlink tool to flash and collect dump. MTK design to use devlink doing flash and dump collection. Devlink can ignore difference in platform architecture, abstracting these commonly used interfaces, I understand that Intel and MTK are preparing to use this plan in the future.
3.Fastboot tool relies on manual installation by user and it does not have the advantages of the above two features(currently, seemed can't support collect core dump directly). It seems that devlink does not need to be installed separately, _ctrl.c files used for QualComm, but Intel and MTK don't use the same design in module, so cannot directly reference.
4.It seemed that Intel WWAN product using iosm driver withing devlink framework has be allowned in upstream, it provides some guidance and direction.

So hope to get your help about the suggestions for the next steps, thanks.

Regards,
Jinjian

2023-11-03 06:33:14

by Jinjian Song

[permalink] [raw]
Subject: RE: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

>>On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <[email protected]> wrote:
>>>
>>> Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>>> >Adds support for t7xx wwan device firmware flashing & coredump
>>> >collection using devlink.
>>>
>>> I don't believe that use of devlink is correct here. It seems like a
>>> misfit. IIUC, what you need is to communicate with the modem.
>>> Basically a communication channel to modem. The other wwan drivers
>>> implement these channels in _ctrl.c files, using multiple protocols.
>>> Why can't you do something similar and let devlink out of this please?
>>>
>>> Until you put in arguments why you really need devlink and why is it
>>> a good fit, I'm against this. Please don't send any other versions of
>>> this patchset that use devlink.

>The t7xx driver already has regular wwan data and control interfaces registered with the wwan framework, making it functional. Here the exposed low level resources are not really wwan/class specific as it is for firmware upgrade >and coredump, so I think that is why Jinjian chose the 'feature agnostic' devlink framework. IMHO I think it makes sense to rely on such a framework, or maybe on the devcoredump class.

>That said, I see the protocol for flashing and doing the coreboot is fastboot, which is already supported on the user side with the fastboot tool, so I'm not sure abstracting it here makes sense. If the protocol is really fasboot compliant, Wouldn't it be simpler to directly expose it as a new device/channel? and rely on a userspace tool for regular fastboot operations (flash, boot, dump). This may require slightly modifying the fastboot tool to detect and support that new transport (in addition to the existing usb and ethernet support).

>I think this is the advantages of using devlink to implement flash and dump collect:
>1.Devlink framework provide the interface of flash and dump, and no need to develop corresponding tools anymore. From another perspective, using devlnik can directly reduce the complexity of PC manufacturer's customer production lines, helping to reduce their costs(time/production).
>2.Currently, the platform architecture of each WWAN module manufacturer is not compatible, Qualcomm implement communicate channels in host driver and using fastboot tool to flash, using their coredump tool to collect dump. Intel implement their host driver in devlink framework, using devlink tool to flash and collect dump. MTK design to use devlink doing flash and dump collection. Devlink can ignore difference in platform architecture, abstracting these commonly used interfaces, I understand that Intel and MTK are preparing to use this plan in the future.
>3.Fastboot tool relies on manual installation by user and it does not have the advantages of the above two features(currently, seemed can't support collect core dump directly). It seems that devlink does not need to be installed separately, _ctrl.c files used for QualComm, but Intel and MTK don't use the same design in module, so cannot directly reference.
>4.It seemed that Intel WWAN product using iosm driver withing devlink framework has be allowned in upstream, it provides some guidance and direction.

>So hope to get your help about the suggestions for the next steps, thanks.


Hope to get your help about the suggestion, if devlink framework here is not suitable, I hope to discuss other alternative solutions.


Thanks.
Regards,
Jinjian

2023-11-03 10:41:02

by Jiri Pirko

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Thu, Sep 21, 2023 at 11:36:26AM CEST, [email protected] wrote:
>On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <[email protected]> wrote:
>>
>> Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>> >Adds support for t7xx wwan device firmware flashing & coredump collection
>> >using devlink.
>>
>> I don't believe that use of devlink is correct here. It seems like a
>> misfit. IIUC, what you need is to communicate with the modem. Basically
>> a communication channel to modem. The other wwan drivers implement these
>> channels in _ctrl.c files, using multiple protocols. Why can't you do
>> something similar and let devlink out of this please?
>>
>> Until you put in arguments why you really need devlink and why is it a
>> good fit, I'm against this. Please don't send any other versions of this
>> patchset that use devlink.
>
>The t7xx driver already has regular wwan data and control interfaces
>registered with the wwan framework, making it functional. Here the
>exposed low level resources are not really wwan/class specific as it
>is for firmware upgrade and coredump, so I think that is why Jinjian
>chose the 'feature agnostic' devlink framework. IMHO I think it makes
>sense to rely on such a framework, or maybe on the devcoredump class.
>
>That said, I see the protocol for flashing and doing the coreboot is
>fastboot, which is already supported on the user side with the
>fastboot tool, so I'm not sure abstracting it here makes sense. If the
>protocol is really fasboot compliant, Wouldn't it be simpler to
>directly expose it as a new device/channel? and rely on a userspace
>tool for regular fastboot operations (flash, boot, dump). This may
>require slightly modifying the fastboot tool to detect and support
>that new transport (in addition to the existing usb and ethernet
>support).

Sounds sane. Please let devlink out of this.

>
>Regards,
>Loic

2023-11-03 10:42:00

by Jiri Pirko

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Mon, Sep 18, 2023 at 08:56:26AM CEST, [email protected] wrote:
>Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
>>>Adds support for t7xx wwan device firmware flashing & coredump
>>>collection using devlink.
>
>>I don't believe that use of devlink is correct here. It seems like a misfit. IIUC, what you need is to communicate with the modem. Basically a communication channel to modem. The other wwan drivers implement these channels in _ctrl.c files, using multiple protocols. Why can't you do something similar and let devlink out of this please?
>
>>Until you put in arguments why you really need devlink and why is it a good fit, I'm against this. Please don't send any other versions of this patchset that use devlink.
>
> Yes, t7xx driver need communicate with modem with a communication channel to modem.
> I took a look at the _ctrl.c files under wwan directory, it seemed the implementation can be well integrated with QualCommon's modem, if we do like this, I think we need modem firmware change, maybe not be suitable for current MTK modem directly.
> Except for Qualcomm modem driver, there is also an Intel wwan driver 'iosm' and it use devlink to implement firmware flash(https://www.kernel.org/doc/html/latest/networking/devlink/iosm.html), Intel and MTK design and use devlink to do this work on

If that exists, I made a mistake as a gatekeeper. That usage looks
wrong.


> 'mtk_t7xx' driver and I continue to do this work.
>
> I think devlink framework can support this scene and if we use devlink we don't need to develop other flash tools or other user space applications, use upstream devlink commands directly.

Please don't.


>
> Thanks.
>>NACK.

2023-12-11 02:06:27

by Jinjian Song

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Hi Loic,

Thank you very much.

> On Fri, 3 Nov 2023 at 11:41, Jiri Pirko <[email protected]> wrote:
> > >
> > > Mon, Sep 18, 2023 at 08:56:26AM CEST, [email protected] wrote:
> > >Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
> > >>>Adds support for t7xx wwan device firmware flashing & coredump
> > >>>collection using devlink.
> > >
> > >>I don't believe that use of devlink is correct here. It seems like a misfit. IIUC, what you need is to communicate with the modem. Basically a communication channel to modem. The other wwan drivers implement these channels in _ctrl.c files, using multiple protocols. Why can't you do something similar and let devlink out of this please?
> > >
> > >>Until you put in arguments why you really need devlink and why is it a good fit, I'm against this. Please don't send any other versions of this patchset that use devlink.
> > >
> > > Yes, t7xx driver need communicate with modem with a communication channel to modem.
> > > I took a look at the _ctrl.c files under wwan directory, it seemed the implementation can be well integrated with QualCommon's modem, if we do like this, I think we need modem firmware change, maybe not be suitable for current MTK modem directly.
> > > Except for Qualcomm modem driver, there is also an Intel wwan
> > > driver 'iosm' and it use devlink to implement firmware
> > > flash(https://www.kernel.org/doc/html/latest/networking/devlink/io
> > > sm.html), Intel and MTK design and use devlink to do this work on
> >
> > If that exists, I made a mistake as a gatekeeper. That usage looks
> > wrong.
> >
> > > 'mtk_t7xx' driver and I continue to do this work.
> > >
> > > I think devlink framework can support this scene and if we use devlink we don't need to develop other flash tools or other user space applications, use upstream devlink commands directly.
> >
> > Please don't.

> So this is clear that devlink should not be used for this wwan
firmware upgrade, if you still want to abstract the fastboot protocol part, maybe the easier would be to move on the generic firmware framework, and especially the firmware upload API which seems to be a good fit here? https://docs.kernel.org/driver-api/firmware/fw_upload.html#firmware-upload-api

1.This API seemed fit here, but I haven't find the refer to use the API, codes in /lib/test_firmware.c shown some intruduce, I think if I'm consider how to implement ops.prepare(what to verify, it seemed modem will do that) and ops.poll_complete? And it seemed request_firmware API also can recieve the data from use space, is it a way to use sysfs to trigger request firmware to kernel?

In addition to this, I may have to create sysfs interface to pass the firmware partition parameter.And find a nother way to export the coredump port to user space.

2.How about we add a new WWAN port type, abstract fastboot and dump channel, like WWAN_PORT_XXX, then use this port with WWAN framework to handle firmware ops and dump ops.

Hope to get your advice, thanks very much.

I want to implement it use the way of title 2, create a new WWAN port type used for the channel with modem.

Regards,
Jinjian

2023-12-12 21:45:47

by Loic Poulain

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Hi Jinjian,

On Mon, 11 Dec 2023 at 03:06, Jinjian Song <[email protected]> wrote:
>
> > > > Mon, Sep 18, 2023 at 08:56:26AM CEST, [email protected] wrote:
> > > >Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
> > > >>>Adds support for t7xx wwan device firmware flashing & coredump
> > > >>>collection using devlink.
> > > >
> > > >>I don't believe that use of devlink is correct here. It seems like a misfit. IIUC, what you need is to communicate with the modem. Basically a communication channel to modem. The other wwan drivers implement these channels in _ctrl.c files, using multiple protocols. Why can't you do something similar and let devlink out of this please?
> > > >
> > > >>Until you put in arguments why you really need devlink and why is it a good fit, I'm against this. Please don't send any other versions of this patchset that use devlink.
> > > >
> > > > Yes, t7xx driver need communicate with modem with a communication channel to modem.
> > > > I took a look at the _ctrl.c files under wwan directory, it seemed the implementation can be well integrated with QualCommon's modem, if we do like this, I think we need modem firmware change, maybe not be suitable for current MTK modem directly.
> > > > Except for Qualcomm modem driver, there is also an Intel wwan
> > > > driver 'iosm' and it use devlink to implement firmware
> > > > flash(https://www.kernel.org/doc/html/latest/networking/devlink/io
> > > > sm.html), Intel and MTK design and use devlink to do this work on
> > >
> > > If that exists, I made a mistake as a gatekeeper. That usage looks
> > > wrong.
> > >
> > > > 'mtk_t7xx' driver and I continue to do this work.
> > > >
> > > > I think devlink framework can support this scene and if we use devlink we don't need to develop other flash tools or other user space applications, use upstream devlink commands directly.
> > >
> > > Please don't.
>
> > So this is clear that devlink should not be used for this wwan
> firmware upgrade, if you still want to abstract the fastboot protocol part, maybe the easier would be to move on the generic firmware framework, and especially the firmware upload API which seems to be a good fit here? https://docs.kernel.org/driver-api/firmware/fw_upload.html#firmware-upload-api
>
> 1.This API seemed fit here, but I haven't find the refer to use the API, codes in /lib/test_firmware.c shown some intruduce, I think if I'm consider how to implement ops.prepare(what to verify, it seemed modem will do that) and ops.poll_complete? And it seemed request_firmware API also can recieve the data from use space, is it a way to use sysfs to trigger request firmware to kernel?
>
> In addition to this, I may have to create sysfs interface to pass the firmware partition parameter.And find a nother way to export the coredump port to user space.
>
> 2.How about we add a new WWAN port type, abstract fastboot and dump channel, like WWAN_PORT_XXX, then use this port with WWAN framework to handle firmware ops and dump ops.
>
> Hope to get your advice, thanks very much.
>
> I want to implement it use the way of title 2, create a new WWAN port type used for the channel with modem.

I understand that the firmware update may not be as simple as
submitting a single blob, and so firmware-upload-api may not be easy
to use as is. But can you elaborate a bit on 'abstracting fastboot',
not sure why it is necessary to add another abstraction level when
fastboot is already supported by open source tools/libraries.

Regards,
Loic

2023-12-13 14:09:16

by Jinjian Song

[permalink] [raw]
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support

Hi Loic,

Thank you very much.

>On Mon, 11 Dec 2023 at 03:06, Jinjian Song <[email protected]> wrote:
>
> > > > Mon, Sep 18, 2023 at 08:56:26AM CEST, [email protected] wrote:
> > > >Tue, Sep 12, 2023 at 11:48:40AM CEST, [email protected] wrote:
> > > >>>Adds support for t7xx wwan device firmware flashing & coredump
> > > >>>collection using devlink.
> > > >
> > > >>I don't believe that use of devlink is correct here. It seems like a misfit. IIUC, what you need is to communicate with the modem. Basically a communication channel to modem. The other wwan drivers implement these channels in _ctrl.c files, using multiple protocols. Why can't you do something similar and let devlink out of this please?
> > > >
> > > >>Until you put in arguments why you really need devlink and why is it a good fit, I'm against this. Please don't send any other versions of this patchset that use devlink.
> > > >
> > > > Yes, t7xx driver need communicate with modem with a communication channel to modem.
> > > > I took a look at the _ctrl.c files under wwan directory, it seemed the implementation can be well integrated with QualCommon's modem, if we do like this, I think we need modem firmware change, maybe not be suitable for current MTK modem directly.
> > > > Except for Qualcomm modem driver, there is also an Intel wwan
> > > > driver 'iosm' and it use devlink to implement firmware
> > > > flash(https://www.kernel.org/doc/html/latest/networking/devlink/
> > > > io sm.html), Intel and MTK design and use devlink to do this
> > > > work on
> > >
> > > If that exists, I made a mistake as a gatekeeper. That usage looks
> > > wrong.
> > >
> > > > 'mtk_t7xx' driver and I continue to do this work.
> > > >
> > > > I think devlink framework can support this scene and if we use devlink we don't need to develop other flash tools or other user space applications, use upstream devlink commands directly.
> > >
> > > Please don't.
>
> > So this is clear that devlink should not be used for this wwan
> firmware upgrade, if you still want to abstract the fastboot protocol
> part, maybe the easier would be to move on the generic firmware
> framework, and especially the firmware upload API which seems to be a
> good fit here?
> https://docs.kernel.org/driver-api/firmware/fw_upload.html#firmware-up
> load-api
>
> 1.This API seemed fit here, but I haven't find the refer to use the API, codes in /lib/test_firmware.c shown some intruduce, I think if I'm consider how to implement ops.prepare(what to verify, it seemed modem will do that) and ops.poll_complete? And it seemed request_firmware API also can recieve the data from use space, is it a way to use sysfs to trigger request firmware to kernel?
>
> In addition to this, I may have to create sysfs interface to pass the firmware partition parameter.And find a nother way to export the coredump port to user space.
>
> 2.How about we add a new WWAN port type, abstract fastboot and dump channel, like WWAN_PORT_XXX, then use this port with WWAN framework to handle firmware ops and dump ops.
>
> Hope to get your advice, thanks very much.
>
> I want to implement it use the way of title 2, create a new WWAN port type used for the channel with modem.

>I understand that the firmware update may not be as simple as submitting a single blob, and so firmware-upload-api may not be easy to use as is. But can you elaborate a bit on 'abstracting fastboot', not sure why it is necessary to add another abstraction level when fastboot is already supported by open source tools/libraries.

It maybe not necessary to add another abstraction level for fastboot. Previously, I thought that the open source tools/libraries worked with USB devices directly, but at that time, I thought it might be more troublesome to adapt to PCIe devices, so abstraction may be an easy way.
Currently, I think create the channel is better than using firmware upload API, I want add a new WWAN port type for this channel, and using fastboot protocol command to write data through this port.
e.g.
t7xx driver to create the channel /dev/wwan0fastboot0 when needed, I want to check whether fastboot open source tools can works with this channel directly, if not, consider using commands directly.
1.create this channel /dev/wwan0fastboot0.
2.download:size > /dev/wwan0fastboot0
3."firmwire data" > /dev/wwan0fastboot0
4.flash:partition > /dev/wwan0fastboot0

Regards,
Jinjian