All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ath6kl: Fix endianness with chip register values
@ 2011-08-31 10:18 Vasanthakumar Thiagarajan
  2011-08-31 10:18 ` [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read Vasanthakumar Thiagarajan
  2011-08-31 11:49 ` [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
  0 siblings, 2 replies; 6+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-08-31 10:18 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, kvalo

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read
  2011-08-31 10:18 [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
@ 2011-08-31 10:18 ` Vasanthakumar Thiagarajan
  2011-09-02  9:12   ` Kalle Valo
  2011-08-31 11:49 ` [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
  1 sibling, 1 reply; 6+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-08-31 10:18 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, kvalo

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

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ath6kl: Fix endianness with chip register values
  2011-08-31 10:18 [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
  2011-08-31 10:18 ` [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read Vasanthakumar Thiagarajan
@ 2011-08-31 11:49 ` Vasanthakumar Thiagarajan
  2011-09-01 10:08   ` Vasanthakumar Thiagarajan
  1 sibling, 1 reply; 6+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-08-31 11:49 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, kvalo

On Wed, Aug 31, 2011 at 03:48:14PM +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ath6kl: Fix endianness with chip register values
  2011-08-31 11:49 ` [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
@ 2011-09-01 10:08   ` Vasanthakumar Thiagarajan
  2011-09-02  8:55     ` Kalle Valo
  0 siblings, 1 reply; 6+ messages in thread
From: Vasanthakumar Thiagarajan @ 2011-09-01 10:08 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, kvalo

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 <vthiagar@qca.qualcomm.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ath6kl: Fix endianness with chip register values
  2011-09-01 10:08   ` Vasanthakumar Thiagarajan
@ 2011-09-02  8:55     ` Kalle Valo
  0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2011-09-02  8:55 UTC (permalink / raw)
  To: Vasanthakumar Thiagarajan; +Cc: linux-wireless

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 <vthiagar@qca.qualcomm.com>
>>> ---
>>>  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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read
  2011-08-31 10:18 ` [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read Vasanthakumar Thiagarajan
@ 2011-09-02  9:12   ` Kalle Valo
  0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2011-09-02  9:12 UTC (permalink / raw)
  To: Vasanthakumar Thiagarajan; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-09-02  9:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-31 10:18 [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
2011-08-31 10:18 ` [PATCH 2/2] ath6kl: Fix endianness in requesting chip register read Vasanthakumar Thiagarajan
2011-09-02  9:12   ` Kalle Valo
2011-08-31 11:49 ` [PATCH 1/2] ath6kl: Fix endianness with chip register values Vasanthakumar Thiagarajan
2011-09-01 10:08   ` Vasanthakumar Thiagarajan
2011-09-02  8:55     ` Kalle Valo

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.