Project

General

Profile

Submit #3044

isp: Unify firmware handling with the rest of the system.

Added by sucanjan@fit.cvut.cz 16 days ago. Updated 1 day ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
06/12/2017
Due date:
% Done:

0%


Description

Hello,

these patches convert firmware binary data from C arrays in a header files to uuencode .uu files. Byte order of the firmware image is little-endian. As far as DragonFly BSD supports only little-endian architectures it's ok. But in case of a big-endian architecture the byte order must be changed before firmware is processed by the isp driver. They also move the .uu images to a sys/contrib/dev/isp.

The first part concerns these firmwares: isp_1040, isp_1040_it, isp_1080, isp_1080_it, isp_12160, isp_12160_it, isp_2100, isp_2200, isp_2300 and isp_2322.
The second part concerns these firmwares: isp_2400, isp_2400_multi, isp_2500, isp_2500_multi.

The division is needed because a size of a single patch must be less than 5 MB for the bugtracker to accept.

0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch View (3.47 MB) sucanjan@fit.cvut.cz, 06/12/2017 09:23 AM

0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch View (4.02 MB) sucanjan@fit.cvut.cz, 06/12/2017 09:38 AM

convert.sh View (8.24 KB) sucanjan@fit.cvut.cz, 06/23/2017 03:51 AM

0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch View (3.49 MB) sucanjan@fit.cvut.cz, 06/25/2017 05:42 AM

0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch View (4.02 MB) sucanjan@fit.cvut.cz, 06/25/2017 05:48 AM

History

#1 Updated by sepherosa 9 days ago

Let's wait a bit for others' input. I am ok w/ it.

On Tue, Jun 13, 2017 at 12:39 AM,
<> wrote:
> Issue #3044 has been reported by .
>
> ----------------------------------------
> Submit #3044: isp: Unify firmware handling with the rest of the system.
> http://bugs.dragonflybsd.org/issues/3044
>
> * Author:
> * Status: New
> * Priority: Normal
> * Assignee:
> * Category:
> * Target version:
> ----------------------------------------
> Hello,
>
> these patches convert firmware binary data from C arrays in a header files to uuencode .uu files. Byte order of the firmware image is little-endian. As far as DragonFly BSD supports only little-endian architectures it's ok. But in case of a big-endian architecture the byte order must be changed before firmware is processed by the isp driver. They also move the .uu images to a sys/contrib/dev/isp.
>
> The first part concerns these firmwares: isp_1040, isp_1040_it, isp_1080, isp_1080_it, isp_12160, isp_12160_it, isp_2100, isp_2200, isp_2300 and isp_2322.
> The second part concerns these firmwares: isp_2400, isp_2400_multi, isp_2500, isp_2500_multi.
>
> The division is needed because a size of a single patch must be less than 5 MB for the bugtracker to accept.
>
>
> ---Files--------------------------------
> 0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (3.47 MB)
> 0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (4.02 MB)
>
>
> --
> You have received this notification because you have either subscribed to it, or are involved in it.
> To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account

--
Tomorrow Will Never Die

#2 Updated by swildner 9 days ago

  • Status changed from New to In Progress

Thanks, I've checked the patches and they work on this 1020 card I own (using isp_1040 firmware I suppose).

How exactly did you generate the *.uu files? How would we sync it with FreeBSD's in the future?

#3 Updated by swildner 9 days ago

  • Assignee set to swildner

#4 Updated by sucanjan@fit.cvut.cz 8 days ago

swildner wrote:
> Thanks, I've checked the patches and they work on this 1020 card I own (using isp_1040 firmware I suppose).
>
> How exactly did you generate the *.uu files? How would we sync it with FreeBSD's in the future?

I generated the *.uu files using command-line tools. I

- removed all lines from a C header file keeping only those lines with values of an array for one firmware image (e.g. 0x1234, 0x5678, ...)
- transformed the values (swapped the bytes to be in little-endian order) using sed to an arguments for the bash-builtin printf command (printf "\x34\x12"; printf "\x78\x56"; ...)
- executed generated printf calls as a bash script and redirected stdout to a file
- converted the binary file with uuencode
- concatenated the .uu file with a corresponding license text

About the sync with FreeBSD's: the truth is I didn't think about that.

#5 Updated by sepherosa 8 days ago

It will be great, if this can be automated.

On Tue, Jun 20, 2017 at 4:39 AM,
<> wrote:
> Issue #3044 has been updated by .
>
>
> swildner wrote:
>> Thanks, I've checked the patches and they work on this 1020 card I own (using isp_1040 firmware I suppose).
>>
>> How exactly did you generate the *.uu files? How would we sync it with FreeBSD's in the future?
>
> I generated the *.uu files using command-line tools. I
>
> - removed all lines from a C header file keeping only those lines with values of an array for one firmware image (e.g. 0x1234, 0x5678, ...)
> - transformed the values (swapped the bytes to be in little-endian order) using sed to an arguments for the bash-builtin printf command (printf "\x34\x12"; printf "\x78\x56"; ...)
> - executed generated printf calls as a bash script and redirected stdout to a file
> - converted the binary file with uuencode
> - concatenated the .uu file with a corresponding license text
>
> About the sync with FreeBSD's: the truth is I didn't think about that.
>
> ----------------------------------------
> Submit #3044: isp: Unify firmware handling with the rest of the system.
> http://bugs.dragonflybsd.org/issues/3044#change-13167
>
> * Author:
> * Status: In Progress
> * Priority: Normal
> * Assignee: swildner
> * Category:
> * Target version:
> ----------------------------------------
> Hello,
>
> these patches convert firmware binary data from C arrays in a header files to uuencode .uu files. Byte order of the firmware image is little-endian. As far as DragonFly BSD supports only little-endian architectures it's ok. But in case of a big-endian architecture the byte order must be changed before firmware is processed by the isp driver. They also move the .uu images to a sys/contrib/dev/isp.
>
> The first part concerns these firmwares: isp_1040, isp_1040_it, isp_1080, isp_1080_it, isp_12160, isp_12160_it, isp_2100, isp_2200, isp_2300 and isp_2322.
> The second part concerns these firmwares: isp_2400, isp_2400_multi, isp_2500, isp_2500_multi.
>
> The division is needed because a size of a single patch must be less than 5 MB for the bugtracker to accept.
>
>
> ---Files--------------------------------
> 0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (3.47 MB)
> 0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (4.02 MB)
>
>
> --
> You have received this notification because you have either subscribed to it, or are involved in it.
> To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account

--
Tomorrow Will Never Die

#6 Updated by sucanjan@fit.cvut.cz 8 days ago

I can create a small tool which will take lines of hex numbers on stdin and print corresponding values to stdout. Could it be this way?

sepherosa wrote:
> It will be great, if this can be automated.
>
> On Tue, Jun 20, 2017 at 4:39 AM,
> <> wrote:
> > Issue #3044 has been updated by .
> >
> >
> > swildner wrote:
> >> Thanks, I've checked the patches and they work on this 1020 card I own (using isp_1040 firmware I suppose).
> >>
> >> How exactly did you generate the *.uu files? How would we sync it with FreeBSD's in the future?
> >
> > I generated the *.uu files using command-line tools. I
> >
> > - removed all lines from a C header file keeping only those lines with values of an array for one firmware image (e.g. 0x1234, 0x5678, ...)
> > - transformed the values (swapped the bytes to be in little-endian order) using sed to an arguments for the bash-builtin printf command (printf "\x34\x12"; printf "\x78\x56"; ...)
> > - executed generated printf calls as a bash script and redirected stdout to a file
> > - converted the binary file with uuencode
> > - concatenated the .uu file with a corresponding license text
> >
> > About the sync with FreeBSD's: the truth is I didn't think about that.
> >
> > ----------------------------------------
> > Submit #3044: isp: Unify firmware handling with the rest of the system.
> > http://bugs.dragonflybsd.org/issues/3044#change-13167
> >
> > * Author:
> > * Status: In Progress
> > * Priority: Normal
> > * Assignee: swildner
> > * Category:
> > * Target version:
> > ----------------------------------------
> > Hello,
> >
> > these patches convert firmware binary data from C arrays in a header files to uuencode .uu files. Byte order of the firmware image is little-endian. As far as DragonFly BSD supports only little-endian architectures it's ok. But in case of a big-endian architecture the byte order must be changed before firmware is processed by the isp driver. They also move the .uu images to a sys/contrib/dev/isp.
> >
> > The first part concerns these firmwares: isp_1040, isp_1040_it, isp_1080, isp_1080_it, isp_12160, isp_12160_it, isp_2100, isp_2200, isp_2300 and isp_2322.
> > The second part concerns these firmwares: isp_2400, isp_2400_multi, isp_2500, isp_2500_multi.
> >
> > The division is needed because a size of a single patch must be less than 5 MB for the bugtracker to accept.
> >
> >
> > ---Files--------------------------------
> > 0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (3.47 MB)
> > 0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (4.02 MB)
> >
> >
> > --
> > You have received this notification because you have either subscribed to it, or are involved in it.
> > To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account
>
>
>
> --
> Tomorrow Will Never Die

#7 Updated by sepherosa 8 days ago

Hmm, I wanted to mean, the tool could be isp specific. Take an isp
firmware header file, and
convert it to uu file. The tool does not need to be generic; it is
only used by us to sync other BSD's isp firmware.

On Tue, Jun 20, 2017 at 4:07 PM,
<> wrote:
> Issue #3044 has been updated by .
>
>
> I can create a small tool which will take lines of hex numbers on stdin and print corresponding values to stdout. Could it be this way?
>
> sepherosa wrote:
>> It will be great, if this can be automated.
>>
>> On Tue, Jun 20, 2017 at 4:39 AM,
>> <> wrote:
>> > Issue #3044 has been updated by .
>> >
>> >
>> > swildner wrote:
>> >> Thanks, I've checked the patches and they work on this 1020 card I own (using isp_1040 firmware I suppose).
>> >>
>> >> How exactly did you generate the *.uu files? How would we sync it with FreeBSD's in the future?
>> >
>> > I generated the *.uu files using command-line tools. I
>> >
>> > - removed all lines from a C header file keeping only those lines with values of an array for one firmware image (e.g. 0x1234, 0x5678, ...)
>> > - transformed the values (swapped the bytes to be in little-endian order) using sed to an arguments for the bash-builtin printf command (printf "\x34\x12"; printf "\x78\x56"; ...)
>> > - executed generated printf calls as a bash script and redirected stdout to a file
>> > - converted the binary file with uuencode
>> > - concatenated the .uu file with a corresponding license text
>> >
>> > About the sync with FreeBSD's: the truth is I didn't think about that.
>> >
>> > ----------------------------------------
>> > Submit #3044: isp: Unify firmware handling with the rest of the system.
>> > http://bugs.dragonflybsd.org/issues/3044#change-13167
>> >
>> > * Author:
>> > * Status: In Progress
>> > * Priority: Normal
>> > * Assignee: swildner
>> > * Category:
>> > * Target version:
>> > ----------------------------------------
>> > Hello,
>> >
>> > these patches convert firmware binary data from C arrays in a header files to uuencode .uu files. Byte order of the firmware image is little-endian. As far as DragonFly BSD supports only little-endian architectures it's ok. But in case of a big-endian architecture the byte order must be changed before firmware is processed by the isp driver. They also move the .uu images to a sys/contrib/dev/isp.
>> >
>> > The first part concerns these firmwares: isp_1040, isp_1040_it, isp_1080, isp_1080_it, isp_12160, isp_12160_it, isp_2100, isp_2200, isp_2300 and isp_2322.
>> > The second part concerns these firmwares: isp_2400, isp_2400_multi, isp_2500, isp_2500_multi.
>> >
>> > The division is needed because a size of a single patch must be less than 5 MB for the bugtracker to accept.
>> >
>> >
>> > ---Files--------------------------------
>> > 0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (3.47 MB)
>> > 0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (4.02 MB)
>> >
>> >
>> > --
>> > You have received this notification because you have either subscribed to it, or are involved in it.
>> > To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account
>>
>>
>>
>> --
>> Tomorrow Will Never Die
>
>
>
> ----------------------------------------
> Submit #3044: isp: Unify firmware handling with the rest of the system.
> http://bugs.dragonflybsd.org/issues/3044#change-13169
>
> * Author:
> * Status: In Progress
> * Priority: Normal
> * Assignee: swildner
> * Category:
> * Target version:
> ----------------------------------------
> Hello,
>
> these patches convert firmware binary data from C arrays in a header files to uuencode .uu files. Byte order of the firmware image is little-endian. As far as DragonFly BSD supports only little-endian architectures it's ok. But in case of a big-endian architecture the byte order must be changed before firmware is processed by the isp driver. They also move the .uu images to a sys/contrib/dev/isp.
>
> The first part concerns these firmwares: isp_1040, isp_1040_it, isp_1080, isp_1080_it, isp_12160, isp_12160_it, isp_2100, isp_2200, isp_2300 and isp_2322.
> The second part concerns these firmwares: isp_2400, isp_2400_multi, isp_2500, isp_2500_multi.
>
> The division is needed because a size of a single patch must be less than 5 MB for the bugtracker to accept.
>
>
> ---Files--------------------------------
> 0001-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (3.47 MB)
> 0002-isp-Unify-firmware-handling-with-the-rest-of-the-sys.patch (4.02 MB)
>
>
> --
> You have received this notification because you have either subscribed to it, or are involved in it.
> To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account

--
Tomorrow Will Never Die

#8 Updated by sucanjan@fit.cvut.cz 7 days ago

I have the tool finished.

During the work I discovered a mistake I made. In the asm_2100.h file there are two variants of firmware image for the same device. When I was processing the file I concatenated both variants to one image. Which variant is compiled depends on if USE_SMALLER_2100_FIRMWARE is defined or not.
The solution I would like to implement is to generate two images (one for each variant) and place the conditional selection to the sys/dev/disk/isp_pci.c to the isp_pci_attach() function where the decision which image to get by firmware_get() is made.

Could it be done in this way?

sepherosa wrote:
> Hmm, I wanted to mean, the tool could be isp specific. Take an isp
> firmware header file, and
> convert it to uu file. The tool does not need to be generic; it is
> only used by us to sync other BSD's isp firmware.

#9 Updated by swildner 7 days ago

I was wondering, is the firmware perhaps more or less easily downloadble from Qlogic and we should take it from them directly.

#10 Updated by sucanjan@fit.cvut.cz 5 days ago

I have searched the Qlogic web site and haven't got any useful results. It seems that for the older cards (and also for some newer) there are no files for download, and the mapping between device identification and firmwares is not very clear. I think that the "more or less easily downloadable" requirement is not satisfied and it will be easier to use other BSD systems as a source of the firmware.

swildner wrote:
> I was wondering, is the firmware perhaps more or less easily downloadble from Qlogic and we should take it from them directly.

#11 Updated by sucanjan@fit.cvut.cz 5 days ago

This is the conversion script for the firmware header files for a review.

#12 Updated by sucanjan@fit.cvut.cz 3 days ago

This is the second version of the patches. I added the conversion script as sys/tools/fw_convert_ispfw.sh, replaced firmware .uu files by those, generated by the script, added smaller variant of the isp_2100 firmware and modified isp_pci.c file for conditional usage of the smaller variant. The condition was originally placed in the sys/dev/disk/ispfw/asm_2100.h file which the patch deletes.

#13 Updated by swildner 1 day ago

Looks fine, thanks. Also for the script.

I've tested it and the firmware is loaded here with ispfw in the kernel config as well as with isp_1040_load=yes in /boot/loader.conf.

One question though: Should/can we preserve the ispfw.ko module (one module with all firmwares) that we had before? If not, I'll add removing ispfw.ko to 'make upgrade'.

#14 Updated by sucanjan@fit.cvut.cz 1 day ago

I think it is good to remove ispfw.ko. Looking at the sys/dev/disk/ispfw/Makefile it seems, that ispfw.ko is built alongside of all the other modules which contain individual firmwares so user will end up with two copies of the same firmware images laying on a filesystem (one in the ispfw.ko and one in the individual module).

But manual page should be updated accordingly. I will take some inspiration from iwn(4) manual page.

Should I upload a new set of the patches with the manual page modified, or it will be sufficient to create a third patch for the modification?

#15 Updated by swildner 1 day ago

A third patch should be fine.

Also available in: Atom PDF