Subject: [PATCH 1/2] ath6kl: Fix endianness with chip register values

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/main.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index 5e807a9..0bcfd46 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -234,6 +234,7 @@ static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
{
int ret;
+ __le32 reg_val = 0;

/* set window register to start read cycle */
ret = ath6kl_set_addrwin_reg(ar, WINDOW_READ_ADDR_ADDRESS, address);
@@ -241,8 +242,10 @@ int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
return ret;

/* read the data */
- ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) value,
- sizeof(*value), HIF_RD_SYNC_BYTE_INC);
+ ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) &reg_val,
+ sizeof(reg_val), HIF_RD_SYNC_BYTE_INC);
+ *value = le32_to_cpu(reg_val);
+
if (ret) {
ath6kl_warn("failed to read32 through diagnose window: %d\n",
ret);
@@ -259,10 +262,13 @@ int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
static int ath6kl_diag_write32(struct ath6kl *ar, u32 address, u32 value)
{
int ret;
+ __le32 reg_val;
+
+ reg_val = cpu_to_le32(value);

/* set write data */
- ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) &value,
- sizeof(value), HIF_WR_SYNC_BYTE_INC);
+ ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) &reg_val,
+ sizeof(reg_val), HIF_WR_SYNC_BYTE_INC);
if (ret) {
ath6kl_err("failed to write 0x%x during diagnose window to 0x%d\n",
address, value);
--
1.7.0.4



Subject: [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read

Need to make sure the chip address for which we need the value
si endian safe.

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/main.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index 0bcfd46..528a347 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -177,8 +177,8 @@ void ath6kl_free_cookie(struct ath6kl *ar, struct ath6kl_cookie *cookie)
static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
{
int status;
- u8 addr_val[4];
s32 i;
+ __le32 addr_val;

/*
* Write bytes 1,2,3 of the register to set the upper address bytes,
@@ -188,16 +188,18 @@ static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
for (i = 1; i <= 3; i++) {
/*
* Fill the buffer with the address byte value we want to
- * hit 4 times.
+ * hit 4 times. No need to worry about endianness as the
+ * same byte is copied to all four bytes of addr_val at
+ * any time.
*/
- memset(addr_val, ((u8 *)&addr)[i], 4);
+ memset((u8 *)&addr_val, ((u8 *)&addr)[i], 4);

/*
* Hit each byte of the register address with a 4-byte
* write operation to the same address, this is a harmless
* operation.
*/
- status = hif_read_write_sync(ar, reg_addr + i, addr_val,
+ status = hif_read_write_sync(ar, reg_addr + i, (u8 *)&addr_val,
4, HIF_WR_SYNC_BYTE_FIX);
if (status)
break;
@@ -215,7 +217,9 @@ static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
* cycle to start, the extra 3 byte write to bytes 1,2,3 has no
* effect since we are writing the same values again
*/
- status = hif_read_write_sync(ar, reg_addr, (u8 *)(&addr),
+ addr_val = cpu_to_le32(addr);
+ status = hif_read_write_sync(ar, reg_addr,
+ (u8 *)&(addr_val),
4, HIF_WR_SYNC_BYTE_INC);

if (status) {
--
1.7.0.4


Subject: Re: [PATCH 1/2] ath6kl: Fix endianness with chip register values

On Wed, Aug 31, 2011 at 03:48:14PM +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
> ---
> drivers/net/wireless/ath/ath6kl/main.c | 14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
> index 5e807a9..0bcfd46 100644
> --- a/drivers/net/wireless/ath/ath6kl/main.c
> +++ b/drivers/net/wireless/ath/ath6kl/main.c
> @@ -234,6 +234,7 @@ static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
> int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
> {
> int ret;
> + __le32 reg_val = 0;
>
> /* set window register to start read cycle */
> ret = ath6kl_set_addrwin_reg(ar, WINDOW_READ_ADDR_ADDRESS, address);
> @@ -241,8 +242,10 @@ int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
> return ret;
>
> /* read the data */
> - ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) value,
> - sizeof(*value), HIF_RD_SYNC_BYTE_INC);
> + ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) &reg_val,
> + sizeof(reg_val), HIF_RD_SYNC_BYTE_INC);
> + *value = le32_to_cpu(reg_val);
> +

This would break your fw_log patch where it is assumed that
the register value read through ath6kl_diag_read32() is LE.
Shall I remove endian conversion from your patch and send it
as a separate one which would be part of this series?.
Having endian conversion in a single place would be simple
and bug free.

Vasanth

2011-09-02 08:56:03

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath6kl: Fix endianness with chip register values

On 09/01/2011 01:08 PM, Vasanthakumar Thiagarajan wrote:
> On Wed, Aug 31, 2011 at 05:19:08PM +0530, Vasanthakumar Thiagarajan wrote:
>> On Wed, Aug 31, 2011 at 03:48:14PM +0530, Vasanthakumar Thiagarajan wrote:
>>> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
>>> ---
>>> drivers/net/wireless/ath/ath6kl/main.c | 14 ++++++++++----
>>> 1 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
>>> index 5e807a9..0bcfd46 100644
>>> --- a/drivers/net/wireless/ath/ath6kl/main.c
>>> +++ b/drivers/net/wireless/ath/ath6kl/main.c
>>> @@ -234,6 +234,7 @@ static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
>>> int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
>>> {
>>> int ret;
>>> + __le32 reg_val = 0;
>>>
>>> /* set window register to start read cycle */
>>> ret = ath6kl_set_addrwin_reg(ar, WINDOW_READ_ADDR_ADDRESS, address);
>>> @@ -241,8 +242,10 @@ int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
>>> return ret;
>>>
>>> /* read the data */
>>> - ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) value,
>>> - sizeof(*value), HIF_RD_SYNC_BYTE_INC);
>>> + ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) &reg_val,
>>> + sizeof(reg_val), HIF_RD_SYNC_BYTE_INC);
>>> + *value = le32_to_cpu(reg_val);
>>> +
>>
>> This would break your fw_log patch where it is assumed that
>> the register value read through ath6kl_diag_read32() is LE.
>> Shall I remove endian conversion from your patch and send it
>> as a separate one which would be part of this series?.
>> Having endian conversion in a single place would be simple
>> and bug free.
>
> I think we should leave endian conversion to the caller during
> register read/write. So this patch can be dropped. But i still think
> my other patch which takes care of endianness for register address
> is needed as the address is again processed on host in
> ath6kl_set_addrwin_reg().

Ok, I'm dropping this patch.

Kalle

Subject: Re: [PATCH 1/2] ath6kl: Fix endianness with chip register values

On Wed, Aug 31, 2011 at 05:19:08PM +0530, Vasanthakumar Thiagarajan wrote:
> On Wed, Aug 31, 2011 at 03:48:14PM +0530, Vasanthakumar Thiagarajan wrote:
> > Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
> > ---
> > drivers/net/wireless/ath/ath6kl/main.c | 14 ++++++++++----
> > 1 files changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
> > index 5e807a9..0bcfd46 100644
> > --- a/drivers/net/wireless/ath/ath6kl/main.c
> > +++ b/drivers/net/wireless/ath/ath6kl/main.c
> > @@ -234,6 +234,7 @@ static int ath6kl_set_addrwin_reg(struct ath6kl *ar, u32 reg_addr, u32 addr)
> > int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
> > {
> > int ret;
> > + __le32 reg_val = 0;
> >
> > /* set window register to start read cycle */
> > ret = ath6kl_set_addrwin_reg(ar, WINDOW_READ_ADDR_ADDRESS, address);
> > @@ -241,8 +242,10 @@ int ath6kl_diag_read32(struct ath6kl *ar, u32 address, u32 *value)
> > return ret;
> >
> > /* read the data */
> > - ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) value,
> > - sizeof(*value), HIF_RD_SYNC_BYTE_INC);
> > + ret = hif_read_write_sync(ar, WINDOW_DATA_ADDRESS, (u8 *) &reg_val,
> > + sizeof(reg_val), HIF_RD_SYNC_BYTE_INC);
> > + *value = le32_to_cpu(reg_val);
> > +
>
> This would break your fw_log patch where it is assumed that
> the register value read through ath6kl_diag_read32() is LE.
> Shall I remove endian conversion from your patch and send it
> as a separate one which would be part of this series?.
> Having endian conversion in a single place would be simple
> and bug free.

I think we should leave endian conversion to the caller during
register read/write. So this patch can be dropped. But i still think
my other patch which takes care of endianness for register address
is needed as the address is again processed on host in
ath6kl_set_addrwin_reg().

Vasanth

2011-09-02 09:12:17

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read

On 08/31/2011 01:18 PM, Vasanthakumar Thiagarajan wrote:
> Need to make sure the chip address for which we need the value
> si endian safe.

Thanks, applied.

Kalle