Message ID | 20210920161136.2398632-11-Jerome.Pouiller@silabs.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Kalle Valo |
Headers | show |
Series | wfx: get out from the staging area | expand |
Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> [...] > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > + const struct firmware **fw, int *file_offset) > +{ > + int keyset_file; > + char filename[256]; > + const char *data; > + int ret; > + > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > + wdev->pdata.file_fw, keyset_chip); > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > + if (ret) { > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > + filename, wdev->pdata.file_fw); > + snprintf(filename, sizeof(filename), "%s.sec", > + wdev->pdata.file_fw); > + ret = request_firmware(fw, filename, wdev->dev); > + if (ret) { > + dev_err(wdev->dev, "can't load %s\n", filename); > + *fw = NULL; > + return ret; > + } > + } How is this firmware file loading supposed to work? If I'm reading the code right, the driver tries to load file "wfm_wf200_??.sec" but in linux-firmware the file is silabs/wfm_wf200_C0.sec: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs That can't work automatically, unless I'm missing something of course. Also I would prefer to use directory name as the driver name wfx, but I guess silabs is also doable. Also I'm not seeing the PDS files in linux-firmware. The idea is that when user installs an upstream kernel and the linux-firmware everything will work automatically, without any manual file installations.
On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > > > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> > > [...] > > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > > + const struct firmware **fw, int *file_offset) > > +{ > > + int keyset_file; > > + char filename[256]; > > + const char *data; > > + int ret; > > + > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > > + wdev->pdata.file_fw, keyset_chip); > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > > + if (ret) { > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > > + filename, wdev->pdata.file_fw); > > + snprintf(filename, sizeof(filename), "%s.sec", > > + wdev->pdata.file_fw); > > + ret = request_firmware(fw, filename, wdev->dev); > > + if (ret) { > > + dev_err(wdev->dev, "can't load %s\n", filename); > > + *fw = NULL; > > + return ret; > > + } > > + } > > How is this firmware file loading supposed to work? If I'm reading the > code right, the driver tries to load file "wfm_wf200_??.sec" but in > linux-firmware the file is silabs/wfm_wf200_C0.sec: > > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs > > That can't work automatically, unless I'm missing something of course. The firmware are signed. "C0" is the key used to sign this firmware. This key must match with the key burned into the chip. Fortunately, the driver is able to read the key accepted by the chip and automatically choose the right firmware. We could imagine to add a attribute in the DT to choose the firmware to load. However, it would be a pity to have to specify it manually whereas the driver is able to detect it automatically. Currently, the only possible key is C0. However, it exists some internal parts with other keys. In addition, it is theoretically possible to ask to Silabs to burn parts with a specific key in order to improve security of a product. Obviously, for now, this feature mainly exists for the Silabs firmware developers who have to work with other keys. > Also I would prefer to use directory name as the driver name wfx, but I > guess silabs is also doable. I have no opinion. > Also I'm not seeing the PDS files in linux-firmware. The idea is that > when user installs an upstream kernel and the linux-firmware everything > will work automatically, without any manual file installations. WF200 is just a chip. Someone has to design an antenna before to be able to use. However, we have evaluation boards that have antennas and corresponding PDS files[1]. Maybe linux-firmware should include the PDS for these boards and the DT should contains the name of the design. eg: compatible = "silabs,brd4001a", "silabs,wf200"; So the driver will know which PDS it should use. In fact, I am sure I had this idea in mind when I have started to write the wfx driver. But with the time I have forgotten it. If you agree with that idea, I can work on it next week. [1]: https://github.com/SiliconLabs/wfx-pds
On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote: > On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: > > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > > > > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > > > > > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> > > > > [...] > > > > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > > > + const struct firmware **fw, int *file_offset) > > > +{ > > > + int keyset_file; > > > + char filename[256]; > > > + const char *data; > > > + int ret; > > > + > > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > > > + wdev->pdata.file_fw, keyset_chip); > > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > > > + if (ret) { > > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > > > + filename, wdev->pdata.file_fw); > > > + snprintf(filename, sizeof(filename), "%s.sec", > > > + wdev->pdata.file_fw); > > > + ret = request_firmware(fw, filename, wdev->dev); > > > + if (ret) { > > > + dev_err(wdev->dev, "can't load %s\n", filename); > > > + *fw = NULL; > > > + return ret; > > > + } > > > + } > > > > How is this firmware file loading supposed to work? If I'm reading the > > code right, the driver tries to load file "wfm_wf200_??.sec" but in > > linux-firmware the file is silabs/wfm_wf200_C0.sec: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs > > > > That can't work automatically, unless I'm missing something of course. > > The firmware are signed. "C0" is the key used to sign this firmware. This > key must match with the key burned into the chip. Fortunately, the driver > is able to read the key accepted by the chip and automatically choose the > right firmware. > > We could imagine to add a attribute in the DT to choose the firmware to > load. However, it would be a pity to have to specify it manually whereas > the driver is able to detect it automatically. > > Currently, the only possible key is C0. However, it exists some internal > parts with other keys. In addition, it is theoretically possible to ask > to Silabs to burn parts with a specific key in order to improve security > of a product. > > Obviously, for now, this feature mainly exists for the Silabs firmware > developers who have to work with other keys. > > > Also I would prefer to use directory name as the driver name wfx, but I > > guess silabs is also doable. > > I have no opinion. > > > > Also I'm not seeing the PDS files in linux-firmware. The idea is that > > when user installs an upstream kernel and the linux-firmware everything > > will work automatically, without any manual file installations. > > WF200 is just a chip. Someone has to design an antenna before to be able > to use. > > However, we have evaluation boards that have antennas and corresponding > PDS files[1]. Maybe linux-firmware should include the PDS for these boards So chip vendor provides firmware and card vendor provides PDS files. In my opinion all files should go into linux-firmware repository. If Silabs has PDS files for its devel boards (which are basically cards) then I think these files should go also into linux-firmware repository. And based on some parameter, driver should load correct PDS file. Seems like DT can be a place where to put something which indicates which PDS file should be used. But should be in DT directly name of PDS file? Or should be in DT just additional compatible string with card vendor name and then in driver itself should be mapping table from compatible string to filename? I do not know what is better. > and the DT should contains the name of the design. eg: > > compatible = "silabs,brd4001a", "silabs,wf200"; > > So the driver will know which PDS it should use. > > In fact, I am sure I had this idea in mind when I have started to write > the wfx driver. But with the time I have forgotten it. > > If you agree with that idea, I can work on it next week. > > > [1]: https://github.com/SiliconLabs/wfx-pds > > -- > Jérôme Pouiller > >
On Friday 1 October 2021 18:08:32 CEST Pali Rohár wrote: > On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote: > > On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: > > > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > > > > > > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > > > > > > > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> > > > > > > [...] > > > > > > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > > > > + const struct firmware **fw, int *file_offset) > > > > +{ > > > > + int keyset_file; > > > > + char filename[256]; > > > > + const char *data; > > > > + int ret; > > > > + > > > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > > > > + wdev->pdata.file_fw, keyset_chip); > > > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > > > > + if (ret) { > > > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > > > > + filename, wdev->pdata.file_fw); > > > > + snprintf(filename, sizeof(filename), "%s.sec", > > > > + wdev->pdata.file_fw); > > > > + ret = request_firmware(fw, filename, wdev->dev); > > > > + if (ret) { > > > > + dev_err(wdev->dev, "can't load %s\n", filename); > > > > + *fw = NULL; > > > > + return ret; > > > > + } > > > > + } > > > > > > How is this firmware file loading supposed to work? If I'm reading the > > > code right, the driver tries to load file "wfm_wf200_??.sec" but in > > > linux-firmware the file is silabs/wfm_wf200_C0.sec: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs > > > > > > That can't work automatically, unless I'm missing something of course. > > > > The firmware are signed. "C0" is the key used to sign this firmware. This > > key must match with the key burned into the chip. Fortunately, the driver > > is able to read the key accepted by the chip and automatically choose the > > right firmware. > > > > We could imagine to add a attribute in the DT to choose the firmware to > > load. However, it would be a pity to have to specify it manually whereas > > the driver is able to detect it automatically. > > > > Currently, the only possible key is C0. However, it exists some internal > > parts with other keys. In addition, it is theoretically possible to ask > > to Silabs to burn parts with a specific key in order to improve security > > of a product. > > > > Obviously, for now, this feature mainly exists for the Silabs firmware > > developers who have to work with other keys. > > > > > Also I would prefer to use directory name as the driver name wfx, but I > > > guess silabs is also doable. > > > > I have no opinion. > > > > > > > Also I'm not seeing the PDS files in linux-firmware. The idea is that > > > when user installs an upstream kernel and the linux-firmware everything > > > will work automatically, without any manual file installations. > > > > WF200 is just a chip. Someone has to design an antenna before to be able > > to use. > > > > However, we have evaluation boards that have antennas and corresponding > > PDS files[1]. Maybe linux-firmware should include the PDS for these boards > > So chip vendor provides firmware and card vendor provides PDS files. Exactly. > In > my opinion all files should go into linux-firmware repository. If Silabs > has PDS files for its devel boards (which are basically cards) then I > think these files should go also into linux-firmware repository. > > And based on some parameter, driver should load correct PDS file. Seems > like DT can be a place where to put something which indicates which PDS > file should be used. > > But should be in DT directly name of PDS file? Or should be in DT just > additional compatible string with card vendor name and then in driver > itself should be mapping table from compatible string to filename? I do > not know what is better. The DT already accepts the attribute silabs,antenna-config-file (see patch #2). I think that linux-firmware repository will reject the pds files if no driver in the kernel directly point to it. Else how to detect orphans? So, I think it is slightly better to use a mapping table. > > and the DT should contains the name of the design. eg: > > > > compatible = "silabs,brd4001a", "silabs,wf200"; > > > > So the driver will know which PDS it should use. > > > > In fact, I am sure I had this idea in mind when I have started to write > > the wfx driver. But with the time I have forgotten it. > > > > If you agree with that idea, I can work on it next week. > > > > > > [1]: https://github.com/SiliconLabs/wfx-pds
Jérôme Pouiller <jerome.pouiller@silabs.com> writes: > On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: >> Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: >> >> > From: Jérôme Pouiller <jerome.pouiller@silabs.com> >> > >> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> >> >> [...] >> >> > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, >> > + const struct firmware **fw, int *file_offset) >> > +{ >> > + int keyset_file; >> > + char filename[256]; >> > + const char *data; >> > + int ret; >> > + >> > + snprintf(filename, sizeof(filename), "%s_%02X.sec", >> > + wdev->pdata.file_fw, keyset_chip); >> > + ret = firmware_request_nowarn(fw, filename, wdev->dev); >> > + if (ret) { >> > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", >> > + filename, wdev->pdata.file_fw); >> > + snprintf(filename, sizeof(filename), "%s.sec", >> > + wdev->pdata.file_fw); >> > + ret = request_firmware(fw, filename, wdev->dev); >> > + if (ret) { >> > + dev_err(wdev->dev, "can't load %s\n", filename); >> > + *fw = NULL; >> > + return ret; >> > + } >> > + } >> >> How is this firmware file loading supposed to work? If I'm reading the >> code right, the driver tries to load file "wfm_wf200_??.sec" but in >> linux-firmware the file is silabs/wfm_wf200_C0.sec: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs >> >> That can't work automatically, unless I'm missing something of course. > > The firmware are signed. "C0" is the key used to sign this firmware. This > key must match with the key burned into the chip. Fortunately, the driver > is able to read the key accepted by the chip and automatically choose the > right firmware. > > We could imagine to add a attribute in the DT to choose the firmware to > load. However, it would be a pity to have to specify it manually whereas > the driver is able to detect it automatically. > > Currently, the only possible key is C0. However, it exists some internal > parts with other keys. In addition, it is theoretically possible to ask > to Silabs to burn parts with a specific key in order to improve security > of a product. > > Obviously, for now, this feature mainly exists for the Silabs firmware > developers who have to work with other keys. My point above was about the directory "silabs". If I read the code correctly, wfx driver tries to load "foo.bin" but in the linux-firmware file is "silabs/foo.bin". So the should also include directory name in the request and use "silabs/foo.bin". >> Also I would prefer to use directory name as the driver name wfx, but I >> guess silabs is also doable. > > I have no opinion. > > >> Also I'm not seeing the PDS files in linux-firmware. The idea is that >> when user installs an upstream kernel and the linux-firmware everything >> will work automatically, without any manual file installations. > > WF200 is just a chip. Someone has to design an antenna before to be able > to use. Doesn't that apply to all wireless chips? :) Some store that information to the EEPROM inside the chip, others somewhere outside of the chip. > However, we have evaluation boards that have antennas and corresponding > PDS files[1]. Maybe linux-firmware should include the PDS for these boards > and the DT should contains the name of the design. eg: > > compatible = "silabs,brd4001a", "silabs,wf200"; > > So the driver will know which PDS it should use. > > In fact, I am sure I had this idea in mind when I have started to write > the wfx driver. But with the time I have forgotten it. > > If you agree with that idea, I can work on it next week. This sounds very similar what we have in ath10k, only that in ath10k we call them board files. The way ath10k works is that we have board-2.bin which is a container file containg multiple board files and then during firmware initialisation ath10k automatically chooses the correct board file based on various parameters like PCI subsystem ids, an id stored in the eeprom, Device Tree etc. And then ath10k pushes the chosed board file to the firmware.
Pali Rohár <pali@kernel.org> writes: > On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote: >> On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: >> > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: >> > >> > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> >> > > >> > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> >> > >> > [...] >> > >> > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, >> > > + const struct firmware **fw, int *file_offset) >> > > +{ >> > > + int keyset_file; >> > > + char filename[256]; >> > > + const char *data; >> > > + int ret; >> > > + >> > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", >> > > + wdev->pdata.file_fw, keyset_chip); >> > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); >> > > + if (ret) { >> > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", >> > > + filename, wdev->pdata.file_fw); >> > > + snprintf(filename, sizeof(filename), "%s.sec", >> > > + wdev->pdata.file_fw); >> > > + ret = request_firmware(fw, filename, wdev->dev); >> > > + if (ret) { >> > > + dev_err(wdev->dev, "can't load %s\n", filename); >> > > + *fw = NULL; >> > > + return ret; >> > > + } >> > > + } >> > >> > How is this firmware file loading supposed to work? If I'm reading the >> > code right, the driver tries to load file "wfm_wf200_??.sec" but in >> > linux-firmware the file is silabs/wfm_wf200_C0.sec: >> > >> > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs >> > >> > That can't work automatically, unless I'm missing something of course. >> >> The firmware are signed. "C0" is the key used to sign this firmware. This >> key must match with the key burned into the chip. Fortunately, the driver >> is able to read the key accepted by the chip and automatically choose the >> right firmware. >> >> We could imagine to add a attribute in the DT to choose the firmware to >> load. However, it would be a pity to have to specify it manually whereas >> the driver is able to detect it automatically. >> >> Currently, the only possible key is C0. However, it exists some internal >> parts with other keys. In addition, it is theoretically possible to ask >> to Silabs to burn parts with a specific key in order to improve security >> of a product. >> >> Obviously, for now, this feature mainly exists for the Silabs firmware >> developers who have to work with other keys. >> >> > Also I would prefer to use directory name as the driver name wfx, but I >> > guess silabs is also doable. >> >> I have no opinion. >> >> >> > Also I'm not seeing the PDS files in linux-firmware. The idea is that >> > when user installs an upstream kernel and the linux-firmware everything >> > will work automatically, without any manual file installations. >> >> WF200 is just a chip. Someone has to design an antenna before to be able >> to use. >> >> However, we have evaluation boards that have antennas and corresponding >> PDS files[1]. Maybe linux-firmware should include the PDS for these boards > > So chip vendor provides firmware and card vendor provides PDS files. In > my opinion all files should go into linux-firmware repository. If Silabs > has PDS files for its devel boards (which are basically cards) then I > think these files should go also into linux-firmware repository. I agree, all files required for normal functionality should be in linux-firmware. The upstream philosophy is that a user can just install the latest kernel and latest linux-firmware, and everything should work out of box (without any manual work). > And based on some parameter, driver should load correct PDS file. Seems > like DT can be a place where to put something which indicates which PDS > file should be used. Again I agree. > But should be in DT directly name of PDS file? Or should be in DT just > additional compatible string with card vendor name and then in driver > itself should be mapping table from compatible string to filename? I do > not know what is better. This is also what I was wondering, to me it sounds wrong to have a filename in DT. I was more thinking about calling it "antenna name" (and not the actually filename), but using compatible strings sounds good to me as well. But of course DT maintainers know this better.
Jérôme Pouiller <jerome.pouiller@silabs.com> writes: > On Friday 1 October 2021 18:08:32 CEST Pali Rohár wrote: >> On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote: >> > On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: >> > > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: >> > > >> > > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> >> > > > >> > > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> >> > > >> > > [...] >> > > >> > > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, >> > > > + const struct firmware **fw, int *file_offset) >> > > > +{ >> > > > + int keyset_file; >> > > > + char filename[256]; >> > > > + const char *data; >> > > > + int ret; >> > > > + >> > > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", >> > > > + wdev->pdata.file_fw, keyset_chip); >> > > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); >> > > > + if (ret) { >> > > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", >> > > > + filename, wdev->pdata.file_fw); >> > > > + snprintf(filename, sizeof(filename), "%s.sec", >> > > > + wdev->pdata.file_fw); >> > > > + ret = request_firmware(fw, filename, wdev->dev); >> > > > + if (ret) { >> > > > + dev_err(wdev->dev, "can't load %s\n", filename); >> > > > + *fw = NULL; >> > > > + return ret; >> > > > + } >> > > > + } >> > > >> > > How is this firmware file loading supposed to work? If I'm reading the >> > > code right, the driver tries to load file "wfm_wf200_??.sec" but in >> > > linux-firmware the file is silabs/wfm_wf200_C0.sec: >> > > >> > > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs >> > > >> > > That can't work automatically, unless I'm missing something of course. >> > >> > The firmware are signed. "C0" is the key used to sign this firmware. This >> > key must match with the key burned into the chip. Fortunately, the driver >> > is able to read the key accepted by the chip and automatically choose the >> > right firmware. >> > >> > We could imagine to add a attribute in the DT to choose the firmware to >> > load. However, it would be a pity to have to specify it manually whereas >> > the driver is able to detect it automatically. >> > >> > Currently, the only possible key is C0. However, it exists some internal >> > parts with other keys. In addition, it is theoretically possible to ask >> > to Silabs to burn parts with a specific key in order to improve security >> > of a product. >> > >> > Obviously, for now, this feature mainly exists for the Silabs firmware >> > developers who have to work with other keys. >> > >> > > Also I would prefer to use directory name as the driver name wfx, but I >> > > guess silabs is also doable. >> > >> > I have no opinion. >> > >> > >> > > Also I'm not seeing the PDS files in linux-firmware. The idea is that >> > > when user installs an upstream kernel and the linux-firmware everything >> > > will work automatically, without any manual file installations. >> > >> > WF200 is just a chip. Someone has to design an antenna before to be able >> > to use. >> > >> > However, we have evaluation boards that have antennas and corresponding >> > PDS files[1]. Maybe linux-firmware should include the PDS for these boards >> >> So chip vendor provides firmware and card vendor provides PDS files. > > Exactly. > >> In >> my opinion all files should go into linux-firmware repository. If Silabs >> has PDS files for its devel boards (which are basically cards) then I >> think these files should go also into linux-firmware repository. >> >> And based on some parameter, driver should load correct PDS file. Seems >> like DT can be a place where to put something which indicates which PDS >> file should be used. >> >> But should be in DT directly name of PDS file? Or should be in DT just >> additional compatible string with card vendor name and then in driver >> itself should be mapping table from compatible string to filename? I do >> not know what is better. > > The DT already accepts the attribute silabs,antenna-config-file (see > patch #2). > > I think that linux-firmware repository will reject the pds files if > no driver in the kernel directly point to it. Else how to detect > orphans? This (linux-firmware rejecting files) is news to me, do you have any pointers? > So, I think it is slightly better to use a mapping table. Not following you here.
On Thursday 7 October 2021 10:08:53 CEST Kalle Valo wrote: > Jérôme Pouiller <jerome.pouiller@silabs.com> writes: > > On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: > >> Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > >> > >> > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > >> > > >> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> > >> > >> [...] > >> > >> > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > >> > + const struct firmware **fw, int *file_offset) > >> > +{ > >> > + int keyset_file; > >> > + char filename[256]; > >> > + const char *data; > >> > + int ret; > >> > + > >> > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > >> > + wdev->pdata.file_fw, keyset_chip); > >> > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > >> > + if (ret) { > >> > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > >> > + filename, wdev->pdata.file_fw); > >> > + snprintf(filename, sizeof(filename), "%s.sec", > >> > + wdev->pdata.file_fw); > >> > + ret = request_firmware(fw, filename, wdev->dev); > >> > + if (ret) { > >> > + dev_err(wdev->dev, "can't load %s\n", filename); > >> > + *fw = NULL; > >> > + return ret; > >> > + } > >> > + } > >> > >> How is this firmware file loading supposed to work? If I'm reading the > >> code right, the driver tries to load file "wfm_wf200_??.sec" but in > >> linux-firmware the file is silabs/wfm_wf200_C0.sec: > >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs > >> > >> That can't work automatically, unless I'm missing something of course. > > > > The firmware are signed. "C0" is the key used to sign this firmware. This > > key must match with the key burned into the chip. Fortunately, the driver > > is able to read the key accepted by the chip and automatically choose the > > right firmware. > > > > We could imagine to add a attribute in the DT to choose the firmware to > > load. However, it would be a pity to have to specify it manually whereas > > the driver is able to detect it automatically. > > > > Currently, the only possible key is C0. However, it exists some internal > > parts with other keys. In addition, it is theoretically possible to ask > > to Silabs to burn parts with a specific key in order to improve security > > of a product. > > > > Obviously, for now, this feature mainly exists for the Silabs firmware > > developers who have to work with other keys. > > My point above was about the directory "silabs". If I read the code > correctly, wfx driver tries to load "foo.bin" but in the linux-firmware > file is "silabs/foo.bin". So the should also include directory name in > the request and use "silabs/foo.bin". Oh! Absolutely. I had never noticed my firmware was not in silabs/ on my test setup. [...]
On Thursday 07 October 2021 11:19:10 Kalle Valo wrote: > Jérôme Pouiller <jerome.pouiller@silabs.com> writes: > > > On Friday 1 October 2021 18:08:32 CEST Pali Rohár wrote: > >> On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote: > >> > On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: > >> > > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > >> > > > >> > > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > >> > > > > >> > > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> > >> > > > >> > > [...] > >> > > > >> > > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > >> > > > + const struct firmware **fw, int *file_offset) > >> > > > +{ > >> > > > + int keyset_file; > >> > > > + char filename[256]; > >> > > > + const char *data; > >> > > > + int ret; > >> > > > + > >> > > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > >> > > > + wdev->pdata.file_fw, keyset_chip); > >> > > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > >> > > > + if (ret) { > >> > > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > >> > > > + filename, wdev->pdata.file_fw); > >> > > > + snprintf(filename, sizeof(filename), "%s.sec", > >> > > > + wdev->pdata.file_fw); > >> > > > + ret = request_firmware(fw, filename, wdev->dev); > >> > > > + if (ret) { > >> > > > + dev_err(wdev->dev, "can't load %s\n", filename); > >> > > > + *fw = NULL; > >> > > > + return ret; > >> > > > + } > >> > > > + } > >> > > > >> > > How is this firmware file loading supposed to work? If I'm reading the > >> > > code right, the driver tries to load file "wfm_wf200_??.sec" but in > >> > > linux-firmware the file is silabs/wfm_wf200_C0.sec: > >> > > > >> > > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs > >> > > > >> > > That can't work automatically, unless I'm missing something of course. > >> > > >> > The firmware are signed. "C0" is the key used to sign this firmware. This > >> > key must match with the key burned into the chip. Fortunately, the driver > >> > is able to read the key accepted by the chip and automatically choose the > >> > right firmware. > >> > > >> > We could imagine to add a attribute in the DT to choose the firmware to > >> > load. However, it would be a pity to have to specify it manually whereas > >> > the driver is able to detect it automatically. > >> > > >> > Currently, the only possible key is C0. However, it exists some internal > >> > parts with other keys. In addition, it is theoretically possible to ask > >> > to Silabs to burn parts with a specific key in order to improve security > >> > of a product. > >> > > >> > Obviously, for now, this feature mainly exists for the Silabs firmware > >> > developers who have to work with other keys. > >> > > >> > > Also I would prefer to use directory name as the driver name wfx, but I > >> > > guess silabs is also doable. > >> > > >> > I have no opinion. > >> > > >> > > >> > > Also I'm not seeing the PDS files in linux-firmware. The idea is that > >> > > when user installs an upstream kernel and the linux-firmware everything > >> > > will work automatically, without any manual file installations. > >> > > >> > WF200 is just a chip. Someone has to design an antenna before to be able > >> > to use. > >> > > >> > However, we have evaluation boards that have antennas and corresponding > >> > PDS files[1]. Maybe linux-firmware should include the PDS for these boards > >> > >> So chip vendor provides firmware and card vendor provides PDS files. > > > > Exactly. > > > >> In > >> my opinion all files should go into linux-firmware repository. If Silabs > >> has PDS files for its devel boards (which are basically cards) then I > >> think these files should go also into linux-firmware repository. > >> > >> And based on some parameter, driver should load correct PDS file. Seems > >> like DT can be a place where to put something which indicates which PDS > >> file should be used. > >> > >> But should be in DT directly name of PDS file? Or should be in DT just > >> additional compatible string with card vendor name and then in driver > >> itself should be mapping table from compatible string to filename? I do > >> not know what is better. > > > > The DT already accepts the attribute silabs,antenna-config-file (see > > patch #2). > > > > I think that linux-firmware repository will reject the pds files if > > no driver in the kernel directly point to it. Else how to detect > > orphans? > > This (linux-firmware rejecting files) is news to me, do you have any > pointers? I understand this as, linux-firmware rejects files which are not used by any driver yet. But you can send both pull request for linux-firmware and pull request for your kernel driver to mailing lists. And once driver changes are merged into -net tree then pull request for linux-firmware can be merged too. > > So, I think it is slightly better to use a mapping table. > > Not following you here. I understand this part to have mapping table between DTS compatible string and pds firmware name in driver code. > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hello Rob! Could you look at issue below to represent antenna (pds) firmware file requirement for this driver in DTS file? On Thursday 07 October 2021 11:16:29 Kalle Valo wrote: > Pali Rohár <pali@kernel.org> writes: > > > On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote: > >> On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote: > >> > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes: > >> > > >> > > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > >> > > > >> > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> > >> > > >> > [...] > >> > > >> > > +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, > >> > > + const struct firmware **fw, int *file_offset) > >> > > +{ > >> > > + int keyset_file; > >> > > + char filename[256]; > >> > > + const char *data; > >> > > + int ret; > >> > > + > >> > > + snprintf(filename, sizeof(filename), "%s_%02X.sec", > >> > > + wdev->pdata.file_fw, keyset_chip); > >> > > + ret = firmware_request_nowarn(fw, filename, wdev->dev); > >> > > + if (ret) { > >> > > + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", > >> > > + filename, wdev->pdata.file_fw); > >> > > + snprintf(filename, sizeof(filename), "%s.sec", > >> > > + wdev->pdata.file_fw); > >> > > + ret = request_firmware(fw, filename, wdev->dev); > >> > > + if (ret) { > >> > > + dev_err(wdev->dev, "can't load %s\n", filename); > >> > > + *fw = NULL; > >> > > + return ret; > >> > > + } > >> > > + } > >> > > >> > How is this firmware file loading supposed to work? If I'm reading the > >> > code right, the driver tries to load file "wfm_wf200_??.sec" but in > >> > linux-firmware the file is silabs/wfm_wf200_C0.sec: > >> > > >> > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs > >> > > >> > That can't work automatically, unless I'm missing something of course. > >> > >> The firmware are signed. "C0" is the key used to sign this firmware. This > >> key must match with the key burned into the chip. Fortunately, the driver > >> is able to read the key accepted by the chip and automatically choose the > >> right firmware. > >> > >> We could imagine to add a attribute in the DT to choose the firmware to > >> load. However, it would be a pity to have to specify it manually whereas > >> the driver is able to detect it automatically. > >> > >> Currently, the only possible key is C0. However, it exists some internal > >> parts with other keys. In addition, it is theoretically possible to ask > >> to Silabs to burn parts with a specific key in order to improve security > >> of a product. > >> > >> Obviously, for now, this feature mainly exists for the Silabs firmware > >> developers who have to work with other keys. > >> > >> > Also I would prefer to use directory name as the driver name wfx, but I > >> > guess silabs is also doable. > >> > >> I have no opinion. > >> > >> > >> > Also I'm not seeing the PDS files in linux-firmware. The idea is that > >> > when user installs an upstream kernel and the linux-firmware everything > >> > will work automatically, without any manual file installations. > >> > >> WF200 is just a chip. Someone has to design an antenna before to be able > >> to use. > >> > >> However, we have evaluation boards that have antennas and corresponding > >> PDS files[1]. Maybe linux-firmware should include the PDS for these boards > > > > So chip vendor provides firmware and card vendor provides PDS files. In > > my opinion all files should go into linux-firmware repository. If Silabs > > has PDS files for its devel boards (which are basically cards) then I > > think these files should go also into linux-firmware repository. > > I agree, all files required for normal functionality should be in > linux-firmware. The upstream philosophy is that a user can just install > the latest kernel and latest linux-firmware, and everything should work > out of box (without any manual work). > > > And based on some parameter, driver should load correct PDS file. Seems > > like DT can be a place where to put something which indicates which PDS > > file should be used. > > Again I agree. > > > But should be in DT directly name of PDS file? Or should be in DT just > > additional compatible string with card vendor name and then in driver > > itself should be mapping table from compatible string to filename? I do > > not know what is better. > > This is also what I was wondering, to me it sounds wrong to have a > filename in DT. I was more thinking about calling it "antenna name" (and > not the actually filename), but using compatible strings sounds good to > me as well. But of course DT maintainers know this better. > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/net/wireless/silabs/wfx/fwio.c b/drivers/net/wireless/silabs/wfx/fwio.c new file mode 100644 index 000000000000..98a9391b2bee --- /dev/null +++ b/drivers/net/wireless/silabs/wfx/fwio.c @@ -0,0 +1,405 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Firmware loading. + * + * Copyright (c) 2017-2020, Silicon Laboratories, Inc. + * Copyright (c) 2010, ST-Ericsson + */ +#include <linux/firmware.h> +#include <linux/slab.h> +#include <linux/mm.h> +#include <linux/bitfield.h> + +#include "fwio.h" +#include "wfx.h" +#include "hwio.h" + +/* Addresses below are in SRAM area */ +#define WFX_DNLD_FIFO 0x09004000 +#define DNLD_BLOCK_SIZE 0x0400 +#define DNLD_FIFO_SIZE 0x8000 /* (32 * DNLD_BLOCK_SIZE) */ +/* Download Control Area (DCA) */ +#define WFX_DCA_IMAGE_SIZE 0x0900C000 +#define WFX_DCA_PUT 0x0900C004 +#define WFX_DCA_GET 0x0900C008 +#define WFX_DCA_HOST_STATUS 0x0900C00C +#define HOST_READY 0x87654321 +#define HOST_INFO_READ 0xA753BD99 +#define HOST_UPLOAD_PENDING 0xABCDDCBA +#define HOST_UPLOAD_COMPLETE 0xD4C64A99 +#define HOST_OK_TO_JUMP 0x174FC882 +#define WFX_DCA_NCP_STATUS 0x0900C010 +#define NCP_NOT_READY 0x12345678 +#define NCP_READY 0x87654321 +#define NCP_INFO_READY 0xBD53EF99 +#define NCP_DOWNLOAD_PENDING 0xABCDDCBA +#define NCP_DOWNLOAD_COMPLETE 0xCAFEFECA +#define NCP_AUTH_OK 0xD4C64A99 +#define NCP_AUTH_FAIL 0x174FC882 +#define NCP_PUB_KEY_RDY 0x7AB41D19 +#define WFX_DCA_FW_SIGNATURE 0x0900C014 +#define FW_SIGNATURE_SIZE 0x40 +#define WFX_DCA_FW_HASH 0x0900C054 +#define FW_HASH_SIZE 0x08 +#define WFX_DCA_FW_VERSION 0x0900C05C +#define FW_VERSION_SIZE 0x04 +#define WFX_DCA_RESERVED 0x0900C060 +#define DCA_RESERVED_SIZE 0x20 +#define WFX_STATUS_INFO 0x0900C080 +#define WFX_BOOTLOADER_LABEL 0x0900C084 +#define BOOTLOADER_LABEL_SIZE 0x3C +#define WFX_PTE_INFO 0x0900C0C0 +#define PTE_INFO_KEYSET_IDX 0x0D +#define PTE_INFO_SIZE 0x10 +#define WFX_ERR_INFO 0x0900C0D0 +#define ERR_INVALID_SEC_TYPE 0x05 +#define ERR_SIG_VERIF_FAILED 0x0F +#define ERR_AES_CTRL_KEY 0x10 +#define ERR_ECC_PUB_KEY 0x11 +#define ERR_MAC_KEY 0x18 + +#define DCA_TIMEOUT 50 /* milliseconds */ +#define WAKEUP_TIMEOUT 200 /* milliseconds */ + +static const char * const fwio_errors[] = { + [ERR_INVALID_SEC_TYPE] = "Invalid section type or wrong encryption", + [ERR_SIG_VERIF_FAILED] = "Signature verification failed", + [ERR_AES_CTRL_KEY] = "AES control key not initialized", + [ERR_ECC_PUB_KEY] = "ECC public key not initialized", + [ERR_MAC_KEY] = "MAC key not initialized", +}; + +/* request_firmware() allocate data using vmalloc(). It is not compatible with + * underlying hardware that use DMA. Function below detect this case and + * allocate a bounce buffer if necessary. + * + * Notice that, in doubt, you can enable CONFIG_DEBUG_SG to ask kernel to + * detect this problem at runtime (else, kernel silently fail). + * + * NOTE: it may also be possible to use 'pages' from struct firmware and avoid + * bounce buffer + */ +static int sram_write_dma_safe(struct wfx_dev *wdev, u32 addr, const u8 *buf, + size_t len) +{ + int ret; + const u8 *tmp; + + if (!virt_addr_valid(buf)) { + tmp = kmemdup(buf, len, GFP_KERNEL); + if (!tmp) + return -ENOMEM; + } else { + tmp = buf; + } + ret = sram_buf_write(wdev, addr, tmp, len); + if (tmp != buf) + kfree(tmp); + return ret; +} + +static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip, + const struct firmware **fw, int *file_offset) +{ + int keyset_file; + char filename[256]; + const char *data; + int ret; + + snprintf(filename, sizeof(filename), "%s_%02X.sec", + wdev->pdata.file_fw, keyset_chip); + ret = firmware_request_nowarn(fw, filename, wdev->dev); + if (ret) { + dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n", + filename, wdev->pdata.file_fw); + snprintf(filename, sizeof(filename), "%s.sec", + wdev->pdata.file_fw); + ret = request_firmware(fw, filename, wdev->dev); + if (ret) { + dev_err(wdev->dev, "can't load %s\n", filename); + *fw = NULL; + return ret; + } + } + + data = (*fw)->data; + if (memcmp(data, "KEYSET", 6) != 0) { + /* Legacy firmware format */ + *file_offset = 0; + keyset_file = 0x90; + } else { + *file_offset = 8; + keyset_file = (hex_to_bin(data[6]) * 16) | hex_to_bin(data[7]); + if (keyset_file < 0) { + dev_err(wdev->dev, "%s corrupted\n", filename); + release_firmware(*fw); + *fw = NULL; + return -EINVAL; + } + } + if (keyset_file != keyset_chip) { + dev_err(wdev->dev, "firmware keyset is incompatible with chip (file: 0x%02X, chip: 0x%02X)\n", + keyset_file, keyset_chip); + release_firmware(*fw); + *fw = NULL; + return -ENODEV; + } + wdev->keyset = keyset_file; + return 0; +} + +static int wait_ncp_status(struct wfx_dev *wdev, u32 status) +{ + ktime_t now, start; + u32 reg; + int ret; + + start = ktime_get(); + for (;;) { + ret = sram_reg_read(wdev, WFX_DCA_NCP_STATUS, ®); + if (ret < 0) + return -EIO; + now = ktime_get(); + if (reg == status) + break; + if (ktime_after(now, ktime_add_ms(start, DCA_TIMEOUT))) + return -ETIMEDOUT; + } + if (ktime_compare(now, start)) + dev_dbg(wdev->dev, "chip answer after %lldus\n", + ktime_us_delta(now, start)); + else + dev_dbg(wdev->dev, "chip answer immediately\n"); + return 0; +} + +static int upload_firmware(struct wfx_dev *wdev, const u8 *data, size_t len) +{ + int ret; + u32 offs, bytes_done = 0; + ktime_t now, start; + + if (len % DNLD_BLOCK_SIZE) { + dev_err(wdev->dev, "firmware size is not aligned. Buffer overrun will occur\n"); + return -EIO; + } + offs = 0; + while (offs < len) { + start = ktime_get(); + for (;;) { + now = ktime_get(); + if (offs + DNLD_BLOCK_SIZE - bytes_done < DNLD_FIFO_SIZE) + break; + if (ktime_after(now, ktime_add_ms(start, DCA_TIMEOUT))) + return -ETIMEDOUT; + ret = sram_reg_read(wdev, WFX_DCA_GET, &bytes_done); + if (ret < 0) + return ret; + } + if (ktime_compare(now, start)) + dev_dbg(wdev->dev, "answer after %lldus\n", + ktime_us_delta(now, start)); + + ret = sram_write_dma_safe(wdev, WFX_DNLD_FIFO + + (offs % DNLD_FIFO_SIZE), + data + offs, DNLD_BLOCK_SIZE); + if (ret < 0) + return ret; + + /* The device seems to not support writing 0 in this register + * during first loop + */ + offs += DNLD_BLOCK_SIZE; + ret = sram_reg_write(wdev, WFX_DCA_PUT, offs); + if (ret < 0) + return ret; + } + return 0; +} + +static void print_boot_status(struct wfx_dev *wdev) +{ + u32 reg; + + sram_reg_read(wdev, WFX_STATUS_INFO, ®); + if (reg == 0x12345678) + return; + sram_reg_read(wdev, WFX_ERR_INFO, ®); + if (reg < ARRAY_SIZE(fwio_errors) && fwio_errors[reg]) + dev_info(wdev->dev, "secure boot: %s\n", fwio_errors[reg]); + else + dev_info(wdev->dev, "secure boot: Error %#02x\n", reg); +} + +static int load_firmware_secure(struct wfx_dev *wdev) +{ + const struct firmware *fw = NULL; + int header_size; + int fw_offset; + ktime_t start; + u8 *buf; + int ret; + + BUILD_BUG_ON(PTE_INFO_SIZE > BOOTLOADER_LABEL_SIZE); + buf = kmalloc(BOOTLOADER_LABEL_SIZE + 1, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + sram_reg_write(wdev, WFX_DCA_HOST_STATUS, HOST_READY); + ret = wait_ncp_status(wdev, NCP_INFO_READY); + if (ret) + goto error; + + sram_buf_read(wdev, WFX_BOOTLOADER_LABEL, buf, BOOTLOADER_LABEL_SIZE); + buf[BOOTLOADER_LABEL_SIZE] = 0; + dev_dbg(wdev->dev, "bootloader: \"%s\"\n", buf); + + sram_buf_read(wdev, WFX_PTE_INFO, buf, PTE_INFO_SIZE); + ret = get_firmware(wdev, buf[PTE_INFO_KEYSET_IDX], &fw, &fw_offset); + if (ret) + goto error; + header_size = fw_offset + FW_SIGNATURE_SIZE + FW_HASH_SIZE; + + sram_reg_write(wdev, WFX_DCA_HOST_STATUS, HOST_INFO_READ); + ret = wait_ncp_status(wdev, NCP_READY); + if (ret) + goto error; + + sram_reg_write(wdev, WFX_DNLD_FIFO, 0xFFFFFFFF); /* Fifo init */ + sram_write_dma_safe(wdev, WFX_DCA_FW_VERSION, "\x01\x00\x00\x00", + FW_VERSION_SIZE); + sram_write_dma_safe(wdev, WFX_DCA_FW_SIGNATURE, fw->data + fw_offset, + FW_SIGNATURE_SIZE); + sram_write_dma_safe(wdev, WFX_DCA_FW_HASH, + fw->data + fw_offset + FW_SIGNATURE_SIZE, + FW_HASH_SIZE); + sram_reg_write(wdev, WFX_DCA_IMAGE_SIZE, fw->size - header_size); + sram_reg_write(wdev, WFX_DCA_HOST_STATUS, HOST_UPLOAD_PENDING); + ret = wait_ncp_status(wdev, NCP_DOWNLOAD_PENDING); + if (ret) + goto error; + + start = ktime_get(); + ret = upload_firmware(wdev, fw->data + header_size, + fw->size - header_size); + if (ret) + goto error; + dev_dbg(wdev->dev, "firmware load after %lldus\n", + ktime_us_delta(ktime_get(), start)); + + sram_reg_write(wdev, WFX_DCA_HOST_STATUS, HOST_UPLOAD_COMPLETE); + ret = wait_ncp_status(wdev, NCP_AUTH_OK); + /* Legacy ROM support */ + if (ret < 0) + ret = wait_ncp_status(wdev, NCP_PUB_KEY_RDY); + if (ret < 0) + goto error; + sram_reg_write(wdev, WFX_DCA_HOST_STATUS, HOST_OK_TO_JUMP); + +error: + kfree(buf); + if (fw) + release_firmware(fw); + if (ret) + print_boot_status(wdev); + return ret; +} + +static int init_gpr(struct wfx_dev *wdev) +{ + int ret, i; + static const struct { + int index; + u32 value; + } gpr_init[] = { + { 0x07, 0x208775 }, + { 0x08, 0x2EC020 }, + { 0x09, 0x3C3C3C }, + { 0x0B, 0x322C44 }, + { 0x0C, 0xA06497 }, + }; + + for (i = 0; i < ARRAY_SIZE(gpr_init); i++) { + ret = igpr_reg_write(wdev, gpr_init[i].index, + gpr_init[i].value); + if (ret < 0) + return ret; + dev_dbg(wdev->dev, " index %02x: %08x\n", + gpr_init[i].index, gpr_init[i].value); + } + return 0; +} + +int wfx_init_device(struct wfx_dev *wdev) +{ + int ret; + int hw_revision, hw_type; + int wakeup_timeout = 50; /* ms */ + ktime_t now, start; + u32 reg; + + reg = CFG_DIRECT_ACCESS_MODE | CFG_CPU_RESET | CFG_BYTE_ORDER_ABCD; + if (wdev->pdata.use_rising_clk) + reg |= CFG_CLK_RISE_EDGE; + ret = config_reg_write(wdev, reg); + if (ret < 0) { + dev_err(wdev->dev, "bus returned an error during first write access. Host configuration error?\n"); + return -EIO; + } + + ret = config_reg_read(wdev, ®); + if (ret < 0) { + dev_err(wdev->dev, "bus returned an error during first read access. Bus configuration error?\n"); + return -EIO; + } + if (reg == 0 || reg == ~0) { + dev_err(wdev->dev, "chip mute. Bus configuration error or chip wasn't reset?\n"); + return -EIO; + } + dev_dbg(wdev->dev, "initial config register value: %08x\n", reg); + + hw_revision = FIELD_GET(CFG_DEVICE_ID_MAJOR, reg); + if (hw_revision == 0) { + dev_err(wdev->dev, "bad hardware revision number: %d\n", + hw_revision); + return -ENODEV; + } + hw_type = FIELD_GET(CFG_DEVICE_ID_TYPE, reg); + if (hw_type == 1) { + dev_notice(wdev->dev, "development hardware detected\n"); + wakeup_timeout = 2000; + } + + ret = init_gpr(wdev); + if (ret < 0) + return ret; + + ret = control_reg_write(wdev, CTRL_WLAN_WAKEUP); + if (ret < 0) + return -EIO; + start = ktime_get(); + for (;;) { + ret = control_reg_read(wdev, ®); + now = ktime_get(); + if (reg & CTRL_WLAN_READY) + break; + if (ktime_after(now, ktime_add_ms(start, wakeup_timeout))) { + dev_err(wdev->dev, "chip didn't wake up. Chip wasn't reset?\n"); + return -ETIMEDOUT; + } + } + dev_dbg(wdev->dev, "chip wake up after %lldus\n", + ktime_us_delta(now, start)); + + ret = config_reg_write_bits(wdev, CFG_CPU_RESET, 0); + if (ret < 0) + return ret; + ret = load_firmware_secure(wdev); + if (ret < 0) + return ret; + return config_reg_write_bits(wdev, + CFG_DIRECT_ACCESS_MODE | + CFG_IRQ_ENABLE_DATA | + CFG_IRQ_ENABLE_WRDY, + CFG_IRQ_ENABLE_DATA); +} diff --git a/drivers/net/wireless/silabs/wfx/fwio.h b/drivers/net/wireless/silabs/wfx/fwio.h new file mode 100644 index 000000000000..eeea61210eca --- /dev/null +++ b/drivers/net/wireless/silabs/wfx/fwio.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Firmware loading. + * + * Copyright (c) 2017-2019, Silicon Laboratories, Inc. + * Copyright (c) 2010, ST-Ericsson + */ +#ifndef WFX_FWIO_H +#define WFX_FWIO_H + +struct wfx_dev; + +int wfx_init_device(struct wfx_dev *wdev); + +#endif