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

[Jinjian Song <SongJinJian@hotmail.com>]

Hi Loic,

Thank you very much.

>On Mon, 11 Dec 2023 at 03:06, Jinjian Song <SongJinJian@hotmail.com> wrote:
>
> > > > Mon, Sep 18, 2023 at 08:56:26AM CEST, SongJinJian@hotmail.com wrote:
> > > >Tue, Sep 12, 2023 at 11:48:40AM CEST, songjinjian@hotmail.com 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