Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
MIME-Version: 1.0
References: <ME3P282MB270323F98B97A1A98A50F8F7BBF1A@ME3P282MB2703.AUSP282.PROD.OUTLOOK.COM>
 <ZQF+PHTYDZRX1gql@nanopsycho>
In-Reply-To: <ZQF+PHTYDZRX1gql@nanopsycho>
From:   Loic Poulain <loic.poulain@linaro.org>
Date:   Thu, 21 Sep 2023 11:36:26 +0200
Message-ID: <CAMZdPi-qZ3JjZmEAtEmJETNzKd+k6UcLnLkM0MZoSZ1hKaOXuA@mail.gmail.com>
Subject: Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support
To:     Jiri Pirko <jiri@resnulli.us>
Cc:     Jinjian Song <songjinjian@hotmail.com>, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        corbet@lwn.net, ryazanov.s.a@gmail.com, johannes@sipsolutions.net,
        chandrashekar.devegowda@intel.com, linuxwwan@intel.com,
        chiranjeevi.rapolu@linux.intel.com, haijun.liu@mediatek.com,
        m.chetan.kumar@linux.intel.com, ricardo.martinez@linux.intel.com,
        netdev@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, nmarupaka@google.com,
        vsankar@lenovo.com, danielwinkler@google.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <jiri@resnulli.us> 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.

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