* [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
@ 2012-03-12 6:38 santosh nayak
2012-03-12 6:48 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: santosh nayak @ 2012-03-12 6:38 UTC (permalink / raw)
To: sony.chacko
Cc: rajesh.borundia, netdev, linux-kernel, kernel-janitors, Santosh Nayak
From: Santosh Nayak <santoshprasadnayak@gmail.com>
Fix endian bug.
Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
---
drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++--
drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 10 +++++-----
.../net/ethernet/qlogic/netxen/netxen_nic_main.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
index 2eeac32..b5de8a7 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
@@ -954,7 +954,7 @@ typedef struct nx_mac_list_s {
struct nx_vlan_ip_list {
struct list_head list;
- u32 ip_addr;
+ __be32 ip_addr;
};
/*
@@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct nx_host_sds_ring *sds_ring, int max);
void netxen_p3_free_mac_list(struct netxen_adapter *adapter);
int netxen_config_intr_coalesce(struct netxen_adapter *adapter);
int netxen_config_rss(struct netxen_adapter *adapter, int enable);
-int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd);
+int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd);
int netxen_linkevent_request(struct netxen_adapter *adapter, int enable);
void netxen_advert_link_change(struct netxen_adapter *adapter, int linkup);
void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *);
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
index 6f37470..59d5ee7 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
@@ -909,7 +909,7 @@ int netxen_config_rss(struct netxen_adapter *adapter, int enable)
return rv;
}
-int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd)
+int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd)
{
nx_nic_req_t req;
u64 word;
@@ -922,7 +922,7 @@ int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd)
req.req_hdr = cpu_to_le64(word);
req.words[0] = cpu_to_le64(cmd);
- req.words[1] = cpu_to_le64(ip);
+ memcpy(&req.words[1], &ip, sizeof(u32));
rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1);
if (rv != 0) {
@@ -1050,7 +1050,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac)
if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == -1)
return -1;
- if (*mac == cpu_to_le64(~0ULL)) {
+ if (*(__le64 *)mac == cpu_to_le64(~0ULL)) {
offset = NX_OLD_MAC_ADDR_OFFSET +
(adapter->portnum * sizeof(u64));
@@ -1059,7 +1059,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac)
offset, sizeof(u64), pmac) == -1)
return -1;
- if (*mac == cpu_to_le64(~0ULL))
+ if (*(__le64 *)mac == cpu_to_le64(~0ULL))
return -1;
}
return 0;
@@ -2155,7 +2155,7 @@ static u32 netxen_md_rd_crb(struct netxen_adapter *adapter,
static u32
netxen_md_rdrom(struct netxen_adapter *adapter,
struct netxen_minidump_entry_rdrom
- *romEntry, u32 *data_buff)
+ *romEntry, __le32 *data_buff)
{
int i, count = 0;
u32 size, lck_val;
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 7648995..65a718f 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -805,12 +805,12 @@ netxen_check_options(struct netxen_adapter *adapter)
char brd_name[NETXEN_MAX_SHORT_NAME];
char serial_num[32];
int i, offset, val, err;
- int *ptr32;
+ __le32 *ptr32;
struct pci_dev *pdev = adapter->pdev;
adapter->driver_mismatch = 0;
- ptr32 = (int *)&serial_num;
+ ptr32 = (__le32 *)&serial_num;
offset = NX_FW_SERIAL_NUM_OFFSET;
for (i = 0; i < 8; i++) {
if (netxen_rom_fast_read(adapter, offset, &val) == -1) {
--
1.7.4.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
2012-03-12 6:38 [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug santosh nayak
@ 2012-03-12 6:48 ` David Miller
2012-03-12 6:52 ` santosh prasad nayak
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2012-03-12 6:48 UTC (permalink / raw)
To: santoshprasadnayak
Cc: sony.chacko, rajesh.borundia, netdev, linux-kernel, kernel-janitors
From: santosh nayak <santoshprasadnayak@gmail.com>
Date: Mon, 12 Mar 2012 12:08:23 +0530
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>
> Fix endian bug.
>
> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
This is a very non-trivial patch, therefore the terse commit
message and the lack of any comments in this code is unacceptable.
This commit message is not only terse, it's wrong. You're not
fixing an endian bug, you're fixing several of them.
Explain everything in your commit message.
> req.words[0] = cpu_to_le64(cmd);
> - req.words[1] = cpu_to_le64(ip);
> + memcpy(&req.words[1], &ip, sizeof(u32));
Nobody is going to read that and have any idea why it's correct unless
they happen to be able to discover the long thread you guys had this
past week working out how to do this change correctly.
Putting a comment here for the rest of us mere mortals is therefore
absolutely required.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
2012-03-12 6:48 ` David Miller
@ 2012-03-12 6:52 ` santosh prasad nayak
2012-03-12 9:15 ` santosh prasad nayak
0 siblings, 1 reply; 5+ messages in thread
From: santosh prasad nayak @ 2012-03-12 6:52 UTC (permalink / raw)
To: David Miller
Cc: sony.chacko, rajesh.borundia, netdev, linux-kernel, kernel-janitors
Sure David.
I will resend it with proper comment.
Regards
Santosh
On Mon, Mar 12, 2012 at 12:18 PM, David Miller <davem@davemloft.net> wrote:
> From: santosh nayak <santoshprasadnayak@gmail.com>
> Date: Mon, 12 Mar 2012 12:08:23 +0530
>
>> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>>
>> Fix endian bug.
>>
>> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
>
> This is a very non-trivial patch, therefore the terse commit
> message and the lack of any comments in this code is unacceptable.
>
> This commit message is not only terse, it's wrong. You're not
> fixing an endian bug, you're fixing several of them.
>
> Explain everything in your commit message.
>
>> req.words[0] = cpu_to_le64(cmd);
>> - req.words[1] = cpu_to_le64(ip);
>> + memcpy(&req.words[1], &ip, sizeof(u32));
>
> Nobody is going to read that and have any idea why it's correct unless
> they happen to be able to discover the long thread you guys had this
> past week working out how to do this change correctly.
>
> Putting a comment here for the rest of us mere mortals is therefore
> absolutely required.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
2012-03-12 6:52 ` santosh prasad nayak
@ 2012-03-12 9:15 ` santosh prasad nayak
2012-03-12 9:36 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: santosh prasad nayak @ 2012-03-12 9:15 UTC (permalink / raw)
To: David Miller
Cc: sony.chacko, rajesh.borundia, netdev, linux-kernel, kernel-janitors
Subject and comments are provided below for review.
This is for review not a formal one. Please let me know
if any more changes are required so that I can include it in
a formal patch.
--------------------------------------------------------------------------------------------------------
Subject: [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
Change the datatype of "ip_addr" to __be32 as 'ip' should be in
big endian format.
Adapter needs "ip address" in big endian format stored at lower 32bit
of req.word[1]. netxen_config_ipaddr() now receives 'ip' in big endian
format. To satisfy adapter's need, use memcpy() to copy byte by byte
of 'ip' into lower 32bit of req.word[1].
Mac address and serial number of adapter need to be in little endian format.
Change the data type of the related variables to __le32 / __le64 or cast it
explicitly to __le32 / __le64 depending upon the requirement.
---------------------------------------------------------------------------------------------------------------
Regards
Santosh
On Mon, Mar 12, 2012 at 12:22 PM, santosh prasad nayak
<santoshprasadnayak@gmail.com> wrote:
> Sure David.
> I will resend it with proper comment.
>
> Regards
> Santosh
>
> On Mon, Mar 12, 2012 at 12:18 PM, David Miller <davem@davemloft.net> wrote:
>> From: santosh nayak <santoshprasadnayak@gmail.com>
>> Date: Mon, 12 Mar 2012 12:08:23 +0530
>>
>>> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>>>
>>> Fix endian bug.
>>>
>>> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
>>
>> This is a very non-trivial patch, therefore the terse commit
>> message and the lack of any comments in this code is unacceptable.
>>
>> This commit message is not only terse, it's wrong. You're not
>> fixing an endian bug, you're fixing several of them.
>>
>> Explain everything in your commit message.
>>
>>> req.words[0] = cpu_to_le64(cmd);
>>> - req.words[1] = cpu_to_le64(ip);
>>> + memcpy(&req.words[1], &ip, sizeof(u32));
>>
>> Nobody is going to read that and have any idea why it's correct unless
>> they happen to be able to discover the long thread you guys had this
>> past week working out how to do this change correctly.
>>
>> Putting a comment here for the rest of us mere mortals is therefore
>> absolutely required.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
2012-03-12 9:15 ` santosh prasad nayak
@ 2012-03-12 9:36 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2012-03-12 9:36 UTC (permalink / raw)
To: santoshprasadnayak
Cc: sony.chacko, rajesh.borundia, netdev, linux-kernel, kernel-janitors
From: santosh prasad nayak <santoshprasadnayak@gmail.com>
Date: Mon, 12 Mar 2012 14:45:38 +0530
> Subject: [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug.
>
> Change the datatype of "ip_addr" to __be32 as 'ip' should be in
> big endian format.
>
> Adapter needs "ip address" in big endian format stored at lower 32bit
> of req.word[1]. netxen_config_ipaddr() now receives 'ip' in big endian
> format. To satisfy adapter's need, use memcpy() to copy byte by byte
> of 'ip' into lower 32bit of req.word[1].
>
> Mac address and serial number of adapter need to be in little endian format.
> Change the data type of the related variables to __le32 / __le64 or cast it
> explicitly to __le32 / __le64 depending upon the requirement.
This looks a lot better.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-12 9:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-12 6:38 [PATCH 3/3 v2] netxen: qlogic ethernet : Fix endian bug santosh nayak
2012-03-12 6:48 ` David Miller
2012-03-12 6:52 ` santosh prasad nayak
2012-03-12 9:15 ` santosh prasad nayak
2012-03-12 9:36 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®