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

From: Jiri Pirko
Date: Fri Nov 03 2023 - 06:40:15 EST


Thu, Sep 21, 2023 at 11:36:26AM CEST, loic.poulain@xxxxxxxxxx wrote:
>On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <jiri@xxxxxxxxxxx> wrote:
>>
>> Tue, Sep 12, 2023 at 11:48:40AM CEST, songjinjian@xxxxxxxxxxx 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