| Message ID | 20230630012136.1330784-1-ajay.kathat@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: wilc1000: change firmware path from 'atmel' to 'microchip/wilc' | expand |
On Fri, 30 Jun 2023 01:22:31 +0000 Ajay.Kathat@microchip.com wrote: > To inline the linux-firmware path with driver, the firmware path is changed > from 'atmel' to 'microchip/wilc'. This path change will be submitted to > 'linux-firmware' as well. How are you going to make this backward compatible? Users aren't required to update linux-firmware and kernel at the same time.
On Wed, 5 Jul 2023 14:03:38 -0700 Jakub Kicinski wrote: > On Fri, 30 Jun 2023 01:22:31 +0000 Ajay.Kathat@microchip.com wrote: > > To inline the linux-firmware path with driver, the firmware path is changed > > from 'atmel' to 'microchip/wilc'. This path change will be submitted to > > 'linux-firmware' as well. > > How are you going to make this backward compatible? > Users aren't required to update linux-firmware and kernel at the same > time. I guess I should have looked at the next patch on the list ;) Please mention how the backward compatibly is assured in the commit message.
On 7/5/23 14:03, Jakub Kicinski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Fri, 30 Jun 2023 01:22:31 +0000 Ajay.Kathat@microchip.com wrote: >> To inline the linux-firmware path with driver, the firmware path is changed >> from 'atmel' to 'microchip/wilc'. This path change will be submitted to >> 'linux-firmware' as well. > > How are you going to make this backward compatible? > Users aren't required to update linux-firmware and kernel at the same > time. Thanks for pointing it out. I think, the changes are not fully compatible for upgrading the kernel and 'linux-firmware' individually. After the driver changes, the 'linux-firmware' upgrade is necessary. The kernel and 'linux-firmware' can be upgraded independently, so two combinations are possible for the upgrade. updated driver : uses modified path(microchip/wilc) to load FW. updated 'linux-firmware': FW file is at modified path(microchip/wilc) 1. updated driver + old 'linux-firmware' : incompatible - The updated driver will look for FW file in '/microchip/wilc' path which is not present in older 'linux-firmware'. Though the interface format is compatible, the driver will fail to load it because the file is not present. 2. old driver + updated 'linux-firmware': compatible - A link is created from new path to old FW path so the old driver should be able to load from linked path. In order to address scenario#1, a fallback method that loads the FW from the older path(/atmel) can be added in the driver. I think that change will make it compatible for scenario#1. Please suggest, if there is a generic/recommended approach to handle backward compatibility for FW path change. Regards, Ajay
On Thu, 6 Jul 2023 00:12:32 +0000 Ajay.Kathat@microchip.com wrote: > On 7/5/23 14:03, Jakub Kicinski wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On Fri, 30 Jun 2023 01:22:31 +0000 Ajay.Kathat@microchip.com wrote: > >> To inline the linux-firmware path with driver, the firmware path is changed > >> from 'atmel' to 'microchip/wilc'. This path change will be submitted to > >> 'linux-firmware' as well. > > > > How are you going to make this backward compatible? > > Users aren't required to update linux-firmware and kernel at the same > > time. > > Thanks for pointing it out. > > I think, the changes are not fully compatible for upgrading the kernel > and 'linux-firmware' individually. After the driver changes, the > 'linux-firmware' upgrade is necessary. > > The kernel and 'linux-firmware' can be upgraded independently, so two > combinations are possible for the upgrade. > > updated driver : uses modified path(microchip/wilc) to load FW. > updated 'linux-firmware': FW file is at modified path(microchip/wilc) > > 1. updated driver + old 'linux-firmware' : incompatible Ah, totally missed that combination. > - The updated driver will look for FW file in '/microchip/wilc' path > which is not present in older 'linux-firmware'. Though the interface > format is compatible, the driver will fail to load it because the file > is not present. > > > 2. old driver + updated 'linux-firmware': compatible > - A link is created from new path to old FW path so the old driver > should be able to load from linked path. > > In order to address scenario#1, a fallback method that loads the FW from > the older path(/atmel) can be added in the driver. I think that change > will make it compatible for scenario#1. > Please suggest, if there is a generic/recommended approach to handle > backward compatibility for FW path change. I'm afraid you need to request from both new and old patch for some time. Push the change to linux-firmware, but make driver be compatible with both for maybe three full releases? Then the risk of someone still having stale linux-firmware goes down quite a bit. TBH renaming FW paths, much like renaming drivers is usually more risk than reward.
Jakub Kicinski <kuba@kernel.org> writes: >> In order to address scenario#1, a fallback method that loads the FW from >> the older path(/atmel) can be added in the driver. I think that change >> will make it compatible for scenario#1. >> Please suggest, if there is a generic/recommended approach to handle >> backward compatibility for FW path change. > > I'm afraid you need to request from both new and old patch for some > time. Push the change to linux-firmware, but make driver be compatible > with both for maybe three full releases? Then the risk of someone still > having stale linux-firmware goes down quite a bit. I would say at least minimum of two years, preferably more to make it possible to upgrade kernel on LTS distro releases. > TBH renaming FW paths, much like renaming drivers is usually more risk > than reward. I agree, it's just extra work without no actually benefit. Maybe an exception here is iwlwifi, that should be fixed as that clutters the top level firmware directory with dozens of files: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/
On 7/19/23 03:37, Kalle Valo wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Jakub Kicinski <kuba@kernel.org> writes: > >>> In order to address scenario#1, a fallback method that loads the FW from >>> the older path(/atmel) can be added in the driver. I think that change >>> will make it compatible for scenario#1. >>> Please suggest, if there is a generic/recommended approach to handle >>> backward compatibility for FW path change. >> >> I'm afraid you need to request from both new and old patch for some >> time. Push the change to linux-firmware, but make driver be compatible >> with both for maybe three full releases? Then the risk of someone still >> having stale linux-firmware goes down quite a bit. > > I would say at least minimum of two years, preferably more to make it > possible to upgrade kernel on LTS distro releases. > >> TBH renaming FW paths, much like renaming drivers is usually more risk >> than reward. > > I agree, it's just extra work without no actually benefit. Maybe an > exception here is iwlwifi, that should be fixed as that clutters the top > level firmware directory with dozens of files: > Definitely, this change will not have any functionality improvements. It will just help to organize the wilc firmware directory structure. Currently, only wilc1000 firmwares are present in linux-firmware but the work to support wilc3000 and wilc's next-gen device is in progress. The existing wilc driver will be extended and the new firmware files needs to be added to linux-firmware. After this change, the all firmware's can organized under same root directory since adding a new device firmware's under 'atmel' folder may not make sense. Alternatively, the new device firmware(e.g wilc3000) can be added to '/microchip/wilc' without changing wilc1000 firmware path. Is this approach okay. Regards, Ajay
<Ajay.Kathat@microchip.com> writes: > On 7/19/23 03:37, Kalle Valo wrote: > >> Jakub Kicinski <kuba@kernel.org> writes: >> >>>> In order to address scenario#1, a fallback method that loads the FW from >>>> the older path(/atmel) can be added in the driver. I think that change >>>> will make it compatible for scenario#1. >>>> Please suggest, if there is a generic/recommended approach to handle >>>> backward compatibility for FW path change. >>> >>> I'm afraid you need to request from both new and old patch for some >>> time. Push the change to linux-firmware, but make driver be compatible >>> with both for maybe three full releases? Then the risk of someone still >>> having stale linux-firmware goes down quite a bit. >> >> I would say at least minimum of two years, preferably more to make it >> possible to upgrade kernel on LTS distro releases. >> >>> TBH renaming FW paths, much like renaming drivers is usually more risk >>> than reward. >> >> I agree, it's just extra work without no actually benefit. Maybe an >> exception here is iwlwifi, that should be fixed as that clutters the top >> level firmware directory with dozens of files: >> > > Definitely, this change will not have any functionality improvements. It > will just help to organize the wilc firmware directory structure. > > Currently, only wilc1000 firmwares are present in linux-firmware but the > work to support wilc3000 and wilc's next-gen device is in progress. The > existing wilc driver will be extended and the new firmware files needs > to be added to linux-firmware. After this change, the all firmware's can > organized under same root directory since adding a new device firmware's > under 'atmel' folder may not make sense. 'atmel' is only a name, I wouldn't worry about that too much. For example we still have drivers/net/wireless/ath even though Atheros is long gone. > Alternatively, the new device firmware(e.g wilc3000) can be added to > '/microchip/wilc' without changing wilc1000 firmware path. Is this > approach okay. I haven't seen the actual patches but in principle having a new directory 'microchip/wilc/ in linux-firmare sounds like a good idea to me. But better to wait for comments from others for a while before submitting anything.
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c index e9f59de31b0b..4551514b4f3f 100644 --- a/drivers/net/wireless/microchip/wilc1000/netdev.c +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c @@ -19,7 +19,7 @@ /* latest API version supported */ #define WILC1000_API_VER 1 -#define WILC1000_FW_PREFIX "atmel/wilc1000_wifi_firmware-" +#define WILC1000_FW_PREFIX "microchip/wilc/wilc1000_wifi_firmware-" #define __WILC1000_FW(api) WILC1000_FW_PREFIX #api ".bin" #define WILC1000_FW(api) __WILC1000_FW(api)