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
