[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<MEYP282MB2697E81E8FEC49757A385A5FBB0CA@MEYP282MB2697.AUSP282.PROD.OUTLOOK.COM>
Date: Mon, 7 Aug 2023 20:26:53 +0800
From: Jinjian Song <songjinjian@...mail.com>
To: jiri@...nulli.us
Cc: chandrashekar.devegowda@...el.com,
chiranjeevi.rapolu@...ux.intel.com,
danielwinkler@...gle.com,
davem@...emloft.net,
edumazet@...gle.com,
haijun.liu@...iatek.com,
ilpo.jarvinen@...ux.intel.com,
jesse.brandeburg@...el.com,
jinjian.song@...ocom.com,
johannes@...solutions.net,
kuba@...nel.org,
linuxwwan@...el.com,
linuxwwan_5g@...el.com,
loic.poulain@...aro.org,
m.chetan.kumar@...ux.intel.com,
netdev@...r.kernel.org,
pabeni@...hat.com,
ricardo.martinez@...ux.intel.com,
ryazanov.s.a@...il.com,
songjinjian@...mail.com,
soumya.prakash.mishra@...el.com
Subject: Re: [net-next 3/6] net: wwan: t7xx: Implements devlink ops of firmware flashing
Thansk for your review.
Witch command: echo 1 > /sys/bus/pci/devices/${bdf}/remove, then driver will run the
.remove ops, during this steps, driver get the fastboot param then send command to
device let device reset to fastboot download mode.
>
>
>>4.use space command rescan device, driver probe and export flash port.
>
>Again, what's "command rescan device" ?
>
>Could you perhaps rather use command line examples?
Thansk for your review.
With the command: echo 1 > /sys/bus/pci/rescan, then driver will run the .probe options
then driver will follow the fastboot download process to export the download ports.
>
>>5.devlink flash firmware image.
>>
>>if don't suggest use devlink param fastboot driver_reinit, I think set
>>fastboot flag during this action, but Intel colleague Kumar have drop that way in the old
>>v2 patch version.
>>https://patchwork.kernel.org/project/netdevbpf/patch/20230105154300.198873-1-m.chetan.kumar@linux.intel.com/
>>
>>>+ case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
>>>+ return 0;
>>>+ default:
>>>+ /* Unsupported action should not get to this function */
>>>+ return -EOPNOTSUPP;
>>>+ }
>>>+}
>
>>>>+static int t7xx_devlink_info_get_loopback(struct devlink *devlink, struct devlink_info_req *req,
>>>>+ struct netlink_ext_ack *extack)
>>>>+{
>>>>+ struct devlink_flash_component_lookup_ctx *lookup_ctx = req->version_cb_priv;
>>>>+ int ret;
>>>>+
>>>>+ if (!req)
>>>>+ return t7xx_devlink_info_get(devlink, req, extack);
>>>>+
>>>>+ ret = devlink_info_version_running_put_ext(req, lookup_ctx->lookup_name,
>>>
>>>It actually took me a while why you are doing this. You try to overcome
>>>the limitation for drivers to expose version for all components that are
>>>valid for flashing. That is not nice
>>>
>>>Please don't do things like this!
>>>
>>>Expose the versions for all valid components, or don't flash them.
>>
>>For the old modem firmware, it don't support the info_get function, so add the logic here to
>>compatible with old modem firmware update during devlink flash.
>
>No! Don't do this. I don't care about your firmware. We enforce info_get
>and flash component parity, obey it. Either provide the version info for
>all components you want to flash with proper versions, or don't
>implement the flash.
Thanks for your review, I will delete the info_get_loopback function.
>
>
>>
>>>>+ "1.0", DEVLINK_INFO_VERSION_TYPE_COMPONENT);
>>>>+
>>>>+ return ret;
>>>> }
>>>>
Powered by blists - more mailing lists