* [RFC] results of endianness review of qla2xxx
@ 2008-04-16 5:54 Al Viro
2008-04-18 18:17 ` Andrew Vasquez
0 siblings, 1 reply; 8+ messages in thread
From: Al Viro @ 2008-04-16 5:54 UTC (permalink / raw)
To: linux-scsi
Assorted endianness problems and uncertain places:
1) In qla24xx_nvram_config():
nv->nvram_version < __constant_cpu_to_le16(ICB_VERSION)) {
comparisons work better in host-endian...
2) Same function:
icb->login_timeout = cpu_to_le16(nv->login_timeout);
both are little-endian
3) qla24xx_login_fabric():
lg->vp_index = cpu_to_le16(ha->vp_idx);
->vp_index is 8bit, ha->vp_idx is host-endian (and small)
4) qla24xx_fabric_logout():
same story.
5) qla24xx_report_id_acquisition():
if (rptid_entry->entry_status != 0)
return;
if (rptid_entry->entry_status != __constant_cpu_to_le16(CS_COMPLETE))
return;
For one thing, ->entry_status is 8bit. For another, CS_COMPLETE is 0, so this
pair of checks looks bogus anyway.
6) Same function:
vp_idx = LSB(rptid_entry->vp_idx);
->vp_idx is little-endian 16bit; LSB expects host-endian
if (MSB(rptid_entry->vp_idx) == 1)
... and so does MSB.
7) qla2x00_async_event():
handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1]));
...
handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1]));
handles[1] = le32_to_cpu(
((uint32_t)(RD_MAILBOX_REG(ha, reg, 7) << 16)) |
and then those values are passed to qla2x00_process_completed_request() which
expects a host-endian (and uses it as index in array, among other things).
8) qla2x00_fdmi_rpa():
max_frame_size = IS_FWI2_CAPABLE(ha) ?
(uint32_t) icb24->frame_payload_size:
(uint32_t) ha->init_cb->frame_payload_size;
eiter->a.max_frame_size = cpu_to_be32(max_frame_size);
and AFAICS ->frame_payload_size is little-endian 16bit.
9) qla2x00_clear_nvram_protection():
wprot_old = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base));
stat = qla2x00_write_nvram_word_tmo(ha, ha->nvram_base,
__constant_cpu_to_le16(0x1234), 100000);
wprot = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base));
if (stat != QLA_SUCCESS || wprot != 0x1234) {
Odd, since qla2x00_get_nvram_word() returns a host-endian and
qla2x00_write_nvram_word_tmo() expects one (this stuff is bit-banging
16bit values through).
10)
typedef struct vf_id {
uint16_t id : 12;
uint16_t priority : 4;
} vf_id_t;
and
typedef struct vf_hopct {
uint16_t reserved : 8;
uint16_t hopct : 8;
} vf_hopct_t;
Buggered, since (a) vf_id is severely endian-dependent and (b) both will happily
get padding on e.g. arm; since they are embedded into hardware-shared structures,
extra 16 bits tacked onto each is Not Good(tm).
11) The same changeset that had introduced vf_id has a related oddity:
- uint8_t reserved_4[32];
+ uint16_t flags;
+ struct vf_id id;
+ uint16_t reserved_4;
+ struct vf_hopct hopct;
+ uint8_t reserved_5[8];
had been bogus - 2 + 2 + 2 + 2 + 8 != 32; 16 bytes got cut.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFC] results of endianness review of qla2xxx 2008-04-16 5:54 [RFC] results of endianness review of qla2xxx Al Viro @ 2008-04-18 18:17 ` Andrew Vasquez 2008-04-18 19:06 ` Al Viro 2008-04-19 17:52 ` Al Viro 0 siblings, 2 replies; 8+ messages in thread From: Andrew Vasquez @ 2008-04-18 18:17 UTC (permalink / raw) To: Al Viro; +Cc: linux-scsi, Seokmann Ju On Wed, 16 Apr 2008, Al Viro wrote: > Assorted endianness problems and uncertain places: > > 1) In qla24xx_nvram_config(): > nv->nvram_version < __constant_cpu_to_le16(ICB_VERSION)) { > comparisons work better in host-endian... > > 2) Same function: > icb->login_timeout = cpu_to_le16(nv->login_timeout); > both are little-endian > > 3) qla24xx_login_fabric(): > lg->vp_index = cpu_to_le16(ha->vp_idx); > ->vp_index is 8bit, ha->vp_idx is host-endian (and small) > > 4) qla24xx_fabric_logout(): > same story. > > 5) qla24xx_report_id_acquisition(): > if (rptid_entry->entry_status != 0) > return; > if (rptid_entry->entry_status != __constant_cpu_to_le16(CS_COMPLETE)) > return; > For one thing, ->entry_status is 8bit. For another, CS_COMPLETE is 0, so this > pair of checks looks bogus anyway. > > 6) Same function: > vp_idx = LSB(rptid_entry->vp_idx); > ->vp_idx is little-endian 16bit; LSB expects host-endian > if (MSB(rptid_entry->vp_idx) == 1) > ... and so does MSB. Thanks, we'll take a look at these and provide relevant fixes. > 7) qla2x00_async_event(): > handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); > ... > handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); > handles[1] = le32_to_cpu( > ((uint32_t)(RD_MAILBOX_REG(ha, reg, 7) << 16)) | > and then those values are passed to qla2x00_process_completed_request() which > expects a host-endian (and uses it as index in array, among other things). THis is correct (though a bit confuscated), each of the two 16bit mailbox-registers makes-up a 32bit LE value, which is converted to cpu-endian and passed to qla2x00_process_completed_request(). > 8) qla2x00_fdmi_rpa(): > max_frame_size = IS_FWI2_CAPABLE(ha) ? > (uint32_t) icb24->frame_payload_size: > (uint32_t) ha->init_cb->frame_payload_size; > eiter->a.max_frame_size = cpu_to_be32(max_frame_size); > and AFAICS ->frame_payload_size is little-endian 16bit. Yes indeed, le16_to_cpu() need to surround the init_cb/icb24->frame_payload_size references. > 9) qla2x00_clear_nvram_protection(): > wprot_old = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base)); > stat = qla2x00_write_nvram_word_tmo(ha, ha->nvram_base, > __constant_cpu_to_le16(0x1234), 100000); > wprot = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base)); > if (stat != QLA_SUCCESS || wprot != 0x1234) { > Odd, since qla2x00_get_nvram_word() returns a host-endian and > qla2x00_write_nvram_word_tmo() expects one (this stuff is bit-banging > 16bit values through). Odd, yes. I was never happy with this endianess intermixing, but after numerous cycles within our qual-testing, this code works in both big and little-endian systems... > 10) > typedef struct vf_id { > uint16_t id : 12; > uint16_t priority : 4; > } vf_id_t; > > and > > typedef struct vf_hopct { > uint16_t reserved : 8; > uint16_t hopct : 8; > } vf_hopct_t; > > Buggered, since (a) vf_id is severely endian-dependent and (b) both will happily > get padding on e.g. arm; since they are embedded into hardware-shared structures, > extra 16 bits tacked onto each is Not Good(tm). > > 11) The same changeset that had introduced vf_id has a related oddity: > > - uint8_t reserved_4[32]; > + uint16_t flags; > + struct vf_id id; > + uint16_t reserved_4; > + struct vf_hopct hopct; > + uint8_t reserved_5[8]; > > had been bogus - 2 + 2 + 2 + 2 + 8 != 32; 16 bytes got cut. We'll look into these as well... Thanks, AV ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] results of endianness review of qla2xxx 2008-04-18 18:17 ` Andrew Vasquez @ 2008-04-18 19:06 ` Al Viro 2008-04-21 17:26 ` Andrew Vasquez 2008-04-19 17:52 ` Al Viro 1 sibling, 1 reply; 8+ messages in thread From: Al Viro @ 2008-04-18 19:06 UTC (permalink / raw) To: Andrew Vasquez; +Cc: linux-scsi, Seokmann Ju On Fri, Apr 18, 2008 at 11:17:58AM -0700, Andrew Vasquez wrote: > > 7) qla2x00_async_event(): > > handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); > > ... > > handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); > > handles[1] = le32_to_cpu( > > ((uint32_t)(RD_MAILBOX_REG(ha, reg, 7) << 16)) | > > and then those values are passed to qla2x00_process_completed_request() which > > expects a host-endian (and uses it as index in array, among other things). > > THis is correct (though a bit confuscated), each of the two 16bit > mailbox-registers makes-up a 32bit LE value, which is converted to > cpu-endian and passed to qla2x00_process_completed_request(). Um, no. First of all, other branches in that switch clearly assume that mb[] is already host-endian. But leaving that aside, what you are doing here will _not_ work even if mb[1] and mb[2] are l-e in this case. Look what happens: * l-e host: handles[0] = (mb[2] << 16) | mb[1], which is equal to (le16_to_cpu(mb[2]) << 16) | le16_to_cpu(mb[1]) * on b-e host: handles[0] = swab32((mb[2] << 16) | mb[1]), which is equal to (swab16(mb[1]) << 16) | swab16(mb[2]), which is equal to (le16_to_cpu(mb[1]) << 16) | le16_to_cpu(mb[2]) IOW, resulting host-endian value will be different with the same contents in mb[]. Put it another way, suppose you have mb[1] == 0, mb[2] == 0x101. Then on l-e you'll get handles[0] == 0x10100 and on b-e - 0x101. The latter will pass the check for index being too high, the former will fail... BTW, RD_MAILBOX_REG() boils down to readw(), which is definitely host-endian. And we have mb[0] = MBA_SCSI_COMPLETION; mb[1] = MSW(stat); mb[2] = RD_MAILBOX_REG(ha, reg, 2); qla2x00_async_event(ha, mb); so there's at least one path reaching qla2x00_async_event() with mb[0] equal to MBA_SCSI_COMPLETION and mb[2] containing host-endian... Looking at the contents of stat and the things we do to it (switch (stat & 0xff) right outside that code), mb[1] is host-endian as well... AFAICS, the same holds for all codepaths - contents of mb[] is host-endian. > > 9) qla2x00_clear_nvram_protection(): > > wprot_old = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base)); > > stat = qla2x00_write_nvram_word_tmo(ha, ha->nvram_base, > > __constant_cpu_to_le16(0x1234), 100000); > > wprot = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base)); > > if (stat != QLA_SUCCESS || wprot != 0x1234) { > > Odd, since qla2x00_get_nvram_word() returns a host-endian and > > qla2x00_write_nvram_word_tmo() expects one (this stuff is bit-banging > > 16bit values through). > > Odd, yes. I was never happy with this endianess intermixing, but > after numerous cycles within our qual-testing, this code works in both > big and little-endian systems... It looks like check that location is writable; what happens if you replace the first instance with 0x1234 and the second (in wprot == ...) with cpu_to_le16(0x1234)? One "we won't find it there by accident, must've been successful write" value seems to be as good as other, unless something else is going on... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] results of endianness review of qla2xxx 2008-04-18 19:06 ` Al Viro @ 2008-04-21 17:26 ` Andrew Vasquez 0 siblings, 0 replies; 8+ messages in thread From: Andrew Vasquez @ 2008-04-21 17:26 UTC (permalink / raw) To: Al Viro; +Cc: linux-scsi, Seokmann Ju On Fri, 18 Apr 2008, Al Viro wrote: > On Fri, Apr 18, 2008 at 11:17:58AM -0700, Andrew Vasquez wrote: > > > 7) qla2x00_async_event(): > > > handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); > > > ... > > > handles[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); > > > handles[1] = le32_to_cpu( > > > ((uint32_t)(RD_MAILBOX_REG(ha, reg, 7) << 16)) | > > > and then those values are passed to qla2x00_process_completed_request() which > > > expects a host-endian (and uses it as index in array, among other things). > > > > THis is correct (though a bit confuscated), each of the two 16bit > > mailbox-registers makes-up a 32bit LE value, which is converted to > > cpu-endian and passed to qla2x00_process_completed_request(). > > Um, no. First of all, other branches in that switch clearly assume > that mb[] is already host-endian. But leaving that aside, what you > are doing here will _not_ work even if mb[1] and mb[2] are l-e in > this case. > > Look what happens: > * l-e host: > handles[0] = (mb[2] << 16) | mb[1], which is equal to > (le16_to_cpu(mb[2]) << 16) | le16_to_cpu(mb[1]) > * on b-e host: > handles[0] = swab32((mb[2] << 16) | mb[1]), which is equal to > (swab16(mb[1]) << 16) | swab16(mb[2]), which is equal to > (le16_to_cpu(mb[1]) << 16) | le16_to_cpu(mb[2]) > > IOW, resulting host-endian value will be different with the same contents > in mb[]. Put it another way, suppose you have mb[1] == 0, mb[2] == 0x101. > Then on l-e you'll get handles[0] == 0x10100 and on b-e - 0x101. The latter > will pass the check for index being too high, the former will fail... > > BTW, RD_MAILBOX_REG() boils down to readw(), which is definitely host-endian. > And we have > mb[0] = MBA_SCSI_COMPLETION; > mb[1] = MSW(stat); > mb[2] = RD_MAILBOX_REG(ha, reg, 2); > qla2x00_async_event(ha, mb); > so there's at least one path reaching qla2x00_async_event() with mb[0] > equal to MBA_SCSI_COMPLETION and mb[2] containing host-endian... Looking > at the contents of stat and the things we do to it (switch (stat & 0xff) right > outside that code), mb[1] is host-endian as well... AFAICS, the same holds > for all codepaths - contents of mb[] is host-endian. Yes, all mb[] "values" are going to be in host-endian format. But, what's missing here is an understanding of what mb[2] and mb[1] are representing during MBA_SCSI_COMPLETION and MBA_CMPLT_2_32BIT handling. Upon command submission, a 32bit handle (cookie) is associated with each IOCB: ... cmd_pkt = (cmd_entry_t *)ha->request_ring_ptr; cmd_pkt->handle = handle; /* Zero out remaining portion of packet. */ clr_ptr = (uint32_t *)cmd_pkt + 2; memset(clr_ptr, 0, REQUEST_ENTRY_SIZE - 8); cmd_pkt->dseg_count = cpu_to_le16(tot_dsds); /* Set target ID and LUN number*/ SET_TARGET_ID(ha, cmd_pkt->target, sp->fcport->loop_id); cmd_pkt->lun = cpu_to_le16(sp->cmd->device->lun); ... Notice that the 'handle'-value is assigned to the command-IOCB structure in host-endian format and *not* the typical little-endian form used to fill-in the other non-byte-sized members of the structure. So, for example, start with a hypothetical 'handle' value of 0x12345678. On an BE machine, the layout in memory of the value written to the structure will be: mem[0] mem[1] mem[2] mem[3] ------ ------ ----- ------ 12 34 56 78 On an LE machine: mem[0] mem[1] mem[2] mem[3] ------ ------ ----- ------ 78 56 34 12 'handle' sent as a host-endian value is done for a reason, as typically, firmware reports IOCB completion via a structure called the status-IOCB [2]: #define STATUS_TYPE 0x03 /* Status entry. */ typedef struct { uint8_t entry_type; /* Entry type. */ uint8_t entry_count; /* Entry count. */ uint8_t sys_define; /* System defined. */ uint8_t entry_status; /* Entry Status. */ uint32_t handle; /* System handle. */ uint16_t scsi_status; /* SCSI status. */ ... where, during creation of the status-IOCB by firmware, the 'handle'-value (at offset 0x4) is copied from the 'handle' specified from the command-IOCB. Since this 'handle' is a a simple index (in host-endian format) into the outstanding_cmds array of the HBA, there's really no reason to convert the value to little-endian format at submission time, then convert from little-endian to host-endian form during status-IOCB handling within the driver [3]: ... /* Fast path completion. */ if (comp_status == CS_COMPLETE && scsi_status == 0) { qla2x00_process_completed_request(ha, sts->handle); return; } So, getting back to MBA_SCSI_COMPLETION and MBA_CMPLT_2_32BIT completions, the "value" of the mailbox registers returned by firmware are: mb[1]: mem[1] << 8 | mem[0] mb[2]: mem[3] << 8 | mem[2] which on an LE machine result in: mb[1]: 0x5678 mb[2]: 0x1234 handle[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); handle[0] = (0x1234 << 16) | 0x5678) handle[0] = 0x12345678 and on an BE machine the values are: mb[1]: 0x3412 mb[2]: 0x7856 handle[0] = le32_to_cpu((uint32_t)((mb[2] << 16) | mb[1])); handle[0] = swab32((0x7856 << 16) | 0x3412) handle[0] = swab32(0x78563412) handle[0] = 0x12345678 > > > 9) qla2x00_clear_nvram_protection(): > > > wprot_old = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base)); > > > stat = qla2x00_write_nvram_word_tmo(ha, ha->nvram_base, > > > __constant_cpu_to_le16(0x1234), 100000); > > > wprot = cpu_to_le16(qla2x00_get_nvram_word(ha, ha->nvram_base)); > > > if (stat != QLA_SUCCESS || wprot != 0x1234) { > > > Odd, since qla2x00_get_nvram_word() returns a host-endian and > > > qla2x00_write_nvram_word_tmo() expects one (this stuff is bit-banging > > > 16bit values through). > > > > Odd, yes. I was never happy with this endianess intermixing, but > > after numerous cycles within our qual-testing, this code works in both > > big and little-endian systems... > > It looks like check that location is writable; what happens if you replace > the first instance with 0x1234 and the second (in wprot == ...) with > cpu_to_le16(0x1234)? One "we won't find it there by accident, must've been > successful write" value seems to be as good as other, unless something else > is going on... One would hope so, yes... Anyway, hope that helped... -- av [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/scsi/qla2xxx/qla_iocb.c;h=5489d5024673a326b5345f5545151a7dab37a533;hb=HEAD#l346 [2] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/scsi/qla2xxx/qla_def.h;h=094d95f0764c6027cb421e4d3e5da9bf17012dfd;hb=HEAD#l1259 [3] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/scsi/qla2xxx/qla_isr.c;h=285479b62d8f02084ad0daebc12ab249f16348c0;hb=HEAD#l944 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] results of endianness review of qla2xxx 2008-04-18 18:17 ` Andrew Vasquez 2008-04-18 19:06 ` Al Viro @ 2008-04-19 17:52 ` Al Viro 2008-04-19 20:12 ` Al Viro 2008-04-21 17:38 ` Andrew Vasquez 1 sibling, 2 replies; 8+ messages in thread From: Al Viro @ 2008-04-19 17:52 UTC (permalink / raw) To: Andrew Vasquez; +Cc: linux-scsi, Seokmann Ju More of that stuff (added by [SCSI] qla2xxx: Add hardware trace-logging support): qla24xx_write_flash_dword(ha, faddr++, cpu_to_le32(jiffies)); qla24xx_write_flash_dword(ha, faddr++, 0); qla24xx_write_flash_dword(ha, faddr++, *fdata++); qla24xx_write_flash_dword(ha, faddr++, *fdata); in qla2xxx_hw_event_store(). All other callers pass host-endian last argument and function sure as hell looks like it's expecting one... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] results of endianness review of qla2xxx 2008-04-19 17:52 ` Al Viro @ 2008-04-19 20:12 ` Al Viro 2008-04-21 18:19 ` Andrew Vasquez 2008-04-21 17:38 ` Andrew Vasquez 1 sibling, 1 reply; 8+ messages in thread From: Al Viro @ 2008-04-19 20:12 UTC (permalink / raw) To: Andrew Vasquez; +Cc: linux-scsi, Seokmann Ju ... and while we are at it, for (miter = 0, s = optrom, d = dwptr; miter < OPTROM_BURST_DWORDS; miter++, s++, d++) *s = cpu_to_le32(*d); ret = qla2x00_load_ram(ha, optrom_dma, flash_data_to_access_addr(faddr), OPTROM_BURST_DWORDS); in qla24xx_write_flash_data() looks bloody odd. We have dwptr pointing to fixed-endian data, so conversion gets us host-endian and then we pass the area filled with results of conversion for DMA? And yes, dwptr points to fixed-endian - it comes from qla24xx_write_optrom_data(struct scsi_qla_host *ha, uint8_t *buf, uint32_t offset, uint32_t length) { ... rval = qla24xx_write_flash_data(ha, (uint32_t *)buf, offset >> 2, length >> 2); Comments? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] results of endianness review of qla2xxx 2008-04-19 20:12 ` Al Viro @ 2008-04-21 18:19 ` Andrew Vasquez 0 siblings, 0 replies; 8+ messages in thread From: Andrew Vasquez @ 2008-04-21 18:19 UTC (permalink / raw) To: Al Viro; +Cc: linux-scsi, Seokmann Ju On Sat, 19 Apr 2008, Al Viro wrote: > ... and while we are at it, > for (miter = 0, s = optrom, d = dwptr; > miter < OPTROM_BURST_DWORDS; miter++, s++, d++) > *s = cpu_to_le32(*d); > > ret = qla2x00_load_ram(ha, optrom_dma, > flash_data_to_access_addr(faddr), > OPTROM_BURST_DWORDS); > > in qla24xx_write_flash_data() looks bloody odd. We have dwptr pointing > to fixed-endian data, so conversion gets us host-endian and then we pass > the area filled with results of conversion for DMA? > > And yes, dwptr points to fixed-endian - it comes from > qla24xx_write_optrom_data(struct scsi_qla_host *ha, uint8_t *buf, > uint32_t offset, uint32_t length) > { > ... > rval = qla24xx_write_flash_data(ha, (uint32_t *)buf, offset >> 2, > length >> 2); > > Comments? Sure, you're not understanding the format of the various pieces of data as they flow through the driver (often times opaquely)... Look at this code from qla_init.c:qla24xx_load_risc_flash(): ... qla24xx_read_flash_data(ha, dcode, faddr, dlen); for (i = 0; i < dlen; i++) dcode[i] = swab32(dcode[i]); rval = qla2x00_load_ram(ha, ha->request_dma, risc_addr, dlen); ... why the explicit swab32()? Because at production time the data was written in big-endian format. Yet, when the driver used to carry the firmware within the driver, the FW folks built the corresponding .C file of the array of 32-bit words in little-endian format... There's a similar case for the code in qla24xx_load_risc() where the .bin file read via request_firmware() was generated in big-endian format... Why? Well...sometimes "stuff" happens and we accept things... Look, rather than pick apart (the rat hole) each access-region within flash and determine which endian-format is used during read and write, let me just say that we'll continue to be extra-cautious when adding features which interact with the ISPs little-endian RISC and various BE/LE regions of flash... Thanks again for your endianess-scrubbing of qla2xxx, I do appreciate your time and efforts and should have patches submitted shortly for much of the 'badness' reported. Thanks, Andrew Vasquez ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] results of endianness review of qla2xxx 2008-04-19 17:52 ` Al Viro 2008-04-19 20:12 ` Al Viro @ 2008-04-21 17:38 ` Andrew Vasquez 1 sibling, 0 replies; 8+ messages in thread From: Andrew Vasquez @ 2008-04-21 17:38 UTC (permalink / raw) To: Al Viro; +Cc: linux-scsi, Seokmann Ju On Sat, 19 Apr 2008, Al Viro wrote: > More of that stuff (added by [SCSI] qla2xxx: Add hardware > trace-logging support): > qla24xx_write_flash_dword(ha, faddr++, > cpu_to_le32(jiffies)); > qla24xx_write_flash_dword(ha, faddr++, 0); > qla24xx_write_flash_dword(ha, faddr++, *fdata++); > qla24xx_write_flash_dword(ha, faddr++, *fdata); > in qla2xxx_hw_event_store(). The data portions of 'fdata[]' are composites of: #define QMARK(a, b, c, d) \ cpu_to_le32(LSB(a) << 24 | LSB(b) << 16 | LSB(c) << 8 | LSB(d)) ... /* Create marker. */ marker[0] = QMARK('L', ha->fw_major_version, ha->fw_minor_version, ha->fw_subminor_version); marker[1] = QMARK(QLA_DRIVER_MAJOR_VER, QLA_DRIVER_MINOR_VER, QLA_DRIVER_PATCH_VER, QLA_DRIVER_BETA_VER); and: /* Store error. */ fdata[0] = cpu_to_le32(code << 16 | d1); fdata[1] = cpu_to_le32(d2 << 16 | d3); rval = qla2xxx_hw_event_store(ha, fdata); > All other callers pass host-endian last > argument and function sure as hell looks like it's expecting one... I'm not entirely clear on what you are driving at here... The data in question is written to flash in little-endian format... ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-04-21 18:19 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-16 5:54 [RFC] results of endianness review of qla2xxx Al Viro 2008-04-18 18:17 ` Andrew Vasquez 2008-04-18 19:06 ` Al Viro 2008-04-21 17:26 ` Andrew Vasquez 2008-04-19 17:52 ` Al Viro 2008-04-19 20:12 ` Al Viro 2008-04-21 18:19 ` Andrew Vasquez 2008-04-21 17:38 ` Andrew Vasquez
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.