gmane.linux.kernel.wireless.general

Johannes Berg | 6 Oct 18:46

[PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

This makes the code surrounding the last few things using
IWL_SET_BITS16 actually readable and removes the macros
so nobody will ever consider using them again.

Signed-off-by: Johannes Berg <johannes@...>
---
Tested on 5000 hw.

 drivers/net/wireless/iwlwifi/iwl-4965-hw.h |   82 ++++++++---------------
 drivers/net/wireless/iwlwifi/iwl-4965.c    |   15 ++--
 drivers/net/wireless/iwlwifi/iwl-5000-hw.h |   55 +++++++--------
 drivers/net/wireless/iwlwifi/iwl-5000.c    |   41 ++++++-----
 drivers/net/wireless/iwlwifi/iwl-helpers.h |  102 -----------------------------
 drivers/net/wireless/iwlwifi/iwl-tx.c      |    6 -
 6 files changed, 90 insertions(+), 211 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
18:01:57.207234025 +0200
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
18:27:22.318362726 +0200
@@ -857,18 +857,24 @@ struct iwl_tfd_addr_desc {
  * A maximum of 255 (not 256!) TFDs may be on a queue waiting for Tx.
  */
 struct iwl_tfd_frame {
-	__le32 val0;
-	/* __le32 rsvd1:24; */
-	/* __le32 num_tbs:5; */
-#define IWL_num_tbs_POS 24
-#define IWL_num_tbs_LEN 5
-#define IWL_num_tbs_SYM val0

(Continue reading)

Johannes Berg | 6 Oct 19:45

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap


> --- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-5000-hw.h	2008-10-06
18:09:21.375233932 +0200
> +++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-5000-hw.h	2008-10-06
18:32:50.180236365 +0200
> @@ -84,46 +84,43 @@
>  #define IWL50_NUM_AMPDU_QUEUES		  10
>  #define IWL50_FIRST_AMPDU_QUEUE		  10
>  
> -#define IWL_sta_id_POS 12
> -#define IWL_sta_id_LEN 4
> -#define IWL_sta_id_SYM val
> -
>  /* Fixed (non-configurable) rx data from phy */
>  
>  /* Base physical address of iwl5000_shared is provided to SCD_DRAM_BASE_ADDR
>   * and &iwl5000_shared.val0 is provided to FH_RSCSR_CHNL0_STTS_WPTR_REG */
>  struct iwl5000_sched_queue_byte_cnt_tbl {
> -	struct iwl4965_queue_byte_cnt_entry tfd_offset[IWL50_QUEUE_SIZE +
> -						       IWL50_MAX_WIN_SIZE];
> +	/* highest 4 bits of each entry are sta ID */
> +	__le16 _tfd_offset[IWL50_QUEUE_SIZE + IWL50_MAX_WIN_SIZE];
>  } __attribute__ ((packed));
>  
> +static inline void
> +iwl5000_queue_byte_cnt_set(struct iwl5000_sched_queue_byte_cnt_tbl *tbl,
> +			   u32 idx, u16 cnt)
> +{
> +	BUG_ON(idx >= IWL50_QUEUE_SIZE + IWL50_MAX_WIN_SIZE);

(Continue reading)

Johannes Berg | 6 Oct 19:49

[PATCH v2] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

This makes the code surrounding the last few things using
IWL_SET_BITS16 actually readable and removes the macros
so nobody will ever consider using them again.

Signed-off-by: Johannes Berg <johannes@...>
---
Tested on 5000 hw.

v2: don't BUG_ON an invalid index because it actually happens!

 drivers/net/wireless/iwlwifi/iwl-4965-hw.h |   85 +++++++++---------------
 drivers/net/wireless/iwlwifi/iwl-4965.c    |   15 ++--
 drivers/net/wireless/iwlwifi/iwl-5000-hw.h |   61 +++++++++--------
 drivers/net/wireless/iwlwifi/iwl-5000.c    |   41 ++++++-----
 drivers/net/wireless/iwlwifi/iwl-helpers.h |  102 -----------------------------
 drivers/net/wireless/iwlwifi/iwl-tx.c      |    6 -
 6 files changed, 99 insertions(+), 211 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
18:01:57.207234025 +0200
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
19:35:09.488701911 +0200
@@ -857,18 +857,24 @@ struct iwl_tfd_addr_desc {
  * A maximum of 255 (not 256!) TFDs may be on a queue waiting for Tx.
  */
 struct iwl_tfd_frame {
-	__le32 val0;
-	/* __le32 rsvd1:24; */
-	/* __le32 num_tbs:5; */
-#define IWL_num_tbs_POS 24

(Continue reading)

Marcel Holtmann | 6 Oct 20:18

Re: [PATCH v2] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

Hi Johannes,

> This makes the code surrounding the last few things using
> IWL_SET_BITS16 actually readable and removes the macros
> so nobody will ever consider using them again.
> 
> Signed-off-by: Johannes Berg <johannes@...>
> ---
> Tested on 5000 hw.
> 
> v2: don't BUG_ON an invalid index because it actually happens!
> 
>  drivers/net/wireless/iwlwifi/iwl-4965-hw.h |   85 +++++++++---------------
>  drivers/net/wireless/iwlwifi/iwl-4965.c    |   15 ++--
>  drivers/net/wireless/iwlwifi/iwl-5000-hw.h |   61 +++++++++--------
>  drivers/net/wireless/iwlwifi/iwl-5000.c    |   41 ++++++-----
>  drivers/net/wireless/iwlwifi/iwl-helpers.h |  102 -----------------------------
>  drivers/net/wireless/iwlwifi/iwl-tx.c      |    6 -
>  6 files changed, 99 insertions(+), 211 deletions(-)
> 
> --- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
18:01:57.207234025 +0200
> +++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
19:35:09.488701911 +0200
> @@ -857,18 +857,24 @@ struct iwl_tfd_addr_desc {
>   * A maximum of 255 (not 256!) TFDs may be on a queue waiting for Tx.
>   */
>  struct iwl_tfd_frame {
> -	__le32 val0;
> -	/* __le32 rsvd1:24; */

(Continue reading)

Johannes Berg | 6 Oct 20:24

Re: [PATCH v2] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Mon, 2008-10-06 at 20:18 +0200, Marcel Holtmann wrote:

> > +static inline u8 iwl_get_num_tbs(struct iwl_tfd_frame *frame)
> > +{
> > +	return frame->num_tbs & 0x1f;
> > +}
> > +
> > +static inline void iwl_set_num_tbs(struct iwl_tfd_frame *frame, u8 num_tbs)
> > +{
> > +	BUG_ON(num_tbs >= 20);
> > +
> > +	frame->num_tbs &= ~0x1f;
> > +	frame->num_tbs |= num_tbs;
> > +}
> 
> what is this magic doing and what is wrong with
> 
> 	frame->num_tbs = num_tbs & 0x1f
> 
> Or do you expect to do something with the upper 3 bits?

I don't know. I was just keeping the original semantics. Maybe future hw
will use those bits? Maybe they are already used and need to be
preserved? This hw programming stuff in the driver is all black magic to
me.

> Personally I think it is important that we always zero any data
> structure and especially the reserved bits of it. Otherwise stuff just
> magically seems to work, but it is pure luck if the right bits are set.

(Continue reading)

Tomas Winkler | 6 Oct 20:35

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Mon, Oct 6, 2008 at 6:50 PM, Johannes Berg <johannes@...> wrote:
> This makes the code surrounding the last few things using
> IWL_SET_BITS16 actually readable and removes the macros
> so nobody will ever consider using them again.
>
> Signed-off-by: Johannes Berg <johannes@...>

I don't want these macros eithers but I NACK any patches with cursing
in the commit logs.
Thanks
Tomas

> ---
> Tested on 5000 hw.
>
>  drivers/net/wireless/iwlwifi/iwl-4965-hw.h |   82 ++++++++---------------
>  drivers/net/wireless/iwlwifi/iwl-4965.c    |   15 ++--
>  drivers/net/wireless/iwlwifi/iwl-5000-hw.h |   55 +++++++--------
>  drivers/net/wireless/iwlwifi/iwl-5000.c    |   41 ++++++-----
>  drivers/net/wireless/iwlwifi/iwl-helpers.h |  102 -----------------------------
>  drivers/net/wireless/iwlwifi/iwl-tx.c      |    6 -
>  6 files changed, 90 insertions(+), 211 deletions(-)
>
> --- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-4965-hw.h    2008-10-06
18:01:57.207234025 +0200
> +++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-4965-hw.h 2008-10-06
18:27:22.318362726 +0200
> @@ -857,18 +857,24 @@ struct iwl_tfd_addr_desc {
>  * A maximum of 255 (not 256!) TFDs may be on a queue waiting for Tx.
>  */

(Continue reading)

Marcel Holtmann | 6 Oct 21:12

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

Hi Tomas,

> > This makes the code surrounding the last few things using
> > IWL_SET_BITS16 actually readable and removes the macros
> > so nobody will ever consider using them again.
> >
> > Signed-off-by: Johannes Berg <johannes@...>
> 
> I don't want these macros eithers but I NACK any patches with cursing
> in the commit logs.

did I miss an internal joke here? I fail to see the cursing ;)

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

drago01 | 6 Oct 22:24

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Mon, Oct 6, 2008 at 9:12 PM, Marcel Holtmann
<holtmann@...> wrote:
> Hi Tomas,
>
>> > This makes the code surrounding the last few things using
>> > IWL_SET_BITS16 actually readable and removes the macros
>> > so nobody will ever consider using them again.
>> >
>> > Signed-off-by: Johannes Berg <johannes@...>
>>
>> I don't want these macros eithers but I NACK any patches with cursing
>> in the commit logs.
>
> did I miss an internal joke here? I fail to see the cursing ;)

he is talking about the "crap" in the subject ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christoph Hellwig | 6 Oct 23:32

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Mon, Oct 06, 2008 at 10:24:07PM +0200, drago01 wrote:
> > did I miss an internal joke here? I fail to see the cursing ;)
> 
> he is talking about the "crap" in the subject ;)

In that case I'd recommend he gets a little thicker skin.  We're not a
bunch of cinderallas.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Holger Schurig | 7 Oct 08:21

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

> I don't want these macros eithers but I NACK any patches with
> cursing in the commit logs.

I fully agree. There's no need to speak about "crap" when 
changing other people's code. In some cultures thats quite 
insulting.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christoph Hellwig | 7 Oct 09:15

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 07, 2008 at 08:21:56AM +0200, Holger Schurig wrote:
> > I don't want these macros eithers but I NACK any patches with
> > cursing in the commit logs.
> 
> I fully agree. There's no need to speak about "crap" when 
> changing other people's code. In some cultures thats quite 
> insulting.

Well, it is crap.  Looks at it objectively.  It's a perfectly fine
description and most of us use it perfectly happily for our own code,
too.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

John W. Linville | 7 Oct 15:16

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 07, 2008 at 03:15:29AM -0400, Christoph Hellwig wrote:
> On Tue, Oct 07, 2008 at 08:21:56AM +0200, Holger Schurig wrote:
> > > I don't want these macros eithers but I NACK any patches with
> > > cursing in the commit logs.
> > 
> > I fully agree. There's no need to speak about "crap" when 
> > changing other people's code. In some cultures thats quite 
> > insulting.
> 
> Well, it is crap.  Looks at it objectively.  It's a perfectly fine
> description and most of us use it perfectly happily for our own code,
> too.

I agree that this is at worst a little bit rude.  The patch should be
rejected or merged on its own basis, although I do reserve the right
to edit a changelog from time to time... 

Now, can we discuss the patch rather than Johannes' intemperate
word choice?

John
--

-- 
John W. Linville		Linux should be at the core
linville@...			of your literate lifestyle.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Tomas Winkler | 7 Oct 16:03

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 7, 2008 at 3:16 PM, John W. Linville <linville@...> wrote:
> On Tue, Oct 07, 2008 at 03:15:29AM -0400, Christoph Hellwig wrote:
>> On Tue, Oct 07, 2008 at 08:21:56AM +0200, Holger Schurig wrote:
>> > > I don't want these macros eithers but I NACK any patches with
>> > > cursing in the commit logs.
>> >
>> > I fully agree. There's no need to speak about "crap" when
>> > changing other people's code. In some cultures thats quite
>> > insulting.
>>
>> Well, it is crap.  Looks at it objectively.  It's a perfectly fine
>> description and most of us use it perfectly happily for our own code,
>> too.
>
> I agree that this is at worst a little bit rude.  The patch should be
> rejected or merged on its own basis, although I do reserve the right
> to edit a changelog from time to time... 

Appreciated.

> Now, can we discuss the patch rather than Johannes' intemperate
> word choice?

With Johannes permissions I will split the patch into chunks first so
I can take smaller steps second
it will better fit into patches I have in pipe.

Thanks
Tomas
--

(Continue reading)

Johannes Berg | 7 Oct 18:58

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, 2008-10-07 at 16:03 +0200, Tomas Winkler wrote:

> > Now, can we discuss the patch rather than Johannes' intemperate
> > word choice?
> 
> With Johannes permissions I will split the patch into chunks first so
> I can take smaller steps second
> it will better fit into patches I have in pipe.

Fine with me, can you adopt the first one too that cleans up the DMA
code? And please keep the 10 pairs -> 20 addresses, it's a lot easier to
understand :)

johannes

Tomas Winkler | 7 Oct 21:28

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 7, 2008 at 6:58 PM, Johannes Berg <johannes@...> wrote:
> On Tue, 2008-10-07 at 16:03 +0200, Tomas Winkler wrote:
>
>> > Now, can we discuss the patch rather than Johannes' intemperate
>> > word choice?
>>
>> With Johannes permissions I will split the patch into chunks first so
>> I can take smaller steps second
>> it will better fit into patches I have in pipe.
>
> Fine with me, can you adopt the first one too that cleans up the DMA
> code?
Yes I handling that too.
 And please keep the 10 pairs -> 20 addresses, it's a lot easier to
> understand :)
I don't like on this solution two things one is that it 32 bit
variables sits on unaligned address and second
it doesn't mach HW description which defines this as  descriptor couple

Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg | 7 Oct 21:34

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, 2008-10-07 at 21:28 +0200, Tomas Winkler wrote:

> > And please keep the 10 pairs -> 20 addresses, it's a lot easier to
> > understand :)

> I don't like on this solution two things one is that it 32 bit
> variables sits on unaligned address

That doesn't matter at all since it's packed, and the bitwise accesses
you did previously are way less efficient.

>  and second
> it doesn't mach HW description which defines this as  descriptor couple

Then tell the HW engineers to fix the HW docs. It makes no sense at all
to say this is 10 pairs rather than 20 single ones if there's nothing
that uses a pair, the whole thing entirely uses single entries in the
numbering etc.

johannes

Johannes Berg | 7 Oct 21:39

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, 2008-10-07 at 21:34 +0200, Johannes Berg wrote:

> Then tell the HW engineers to fix the HW docs. It makes no sense at all
> to say this is 10 pairs rather than 20 single ones if there's nothing
> that uses a pair, the whole thing entirely uses single entries in the
> numbering etc.

If you need to visualise this, let me draw it:

bytes (with nibbles): | . | . | . | . | . | . || . | . | . | . | . | . |
                      | addr 1          |len1 || addr 2          |len2 |
your forced layout:   |               |                |               |
my layout:            |               |       ||               |       |

Note how this actually matches the border between the border between the
two descriptors, the || border.

_Please_, just fix it, this is _way_ easier than the forced layout
you're doing.

johannes

Tomas Winkler | 7 Oct 22:06

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 7, 2008 at 9:39 PM, Johannes Berg <johannes@...> wrote:
> On Tue, 2008-10-07 at 21:34 +0200, Johannes Berg wrote:
>
>> Then tell the HW engineers to fix the HW docs. It makes no sense at all
>> to say this is 10 pairs rather than 20 single ones if there's nothing
>> that uses a pair, the whole thing entirely uses single entries in the
>> numbering etc.
>
> If you need to visualise this, let me draw it:
>
> bytes (with nibbles): | . | . | . | . | . | . || . | . | . | . | . | . |
>                      | addr 1          |len1 || addr 2          |len2 |
> your forced layout:   |               |                |               |
> my layout:            |               |       ||               |       |
>
> Note how this actually matches the border between the border between the
> two descriptors, the || border.

Your layout put addr2 (which is 32 bit) sits on 16 bit boundary
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg | 7 Oct 22:10

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, 2008-10-07 at 22:06 +0200, Tomas Winkler wrote:

> > bytes (with nibbles): | . | . | . | . | . | . || . | . | . | . | . | . |
> >                      | addr 1          |len1 || addr 2          |len2 |
> > your forced layout:   |               |                |               |
> > my layout:            |               |       ||               |       |
> >
> > Note how this actually matches the border between the border between the
> > two descriptors, the || border.
> 
> Your layout put addr2 (which is 32 bit) sits on 16 bit boundary

So? Yours makes it need two words, which is even less efficient to
access. And since the struct is packed, on those architectures that
don't do unaligned accesses efficiently won't need any put_unaligned
either.

Also, keep in mind that address 2 is _never_ used at all anyway because
we don't and cannot do fragmented frames due to the lack of IP
checksumming in hardware, as I and davem have tried to explain to you
multiple times already.

johannes

Tomas Winkler | 7 Oct 22:46

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 7, 2008 at 10:10 PM, Johannes Berg
<johannes@...> wrote:
> On Tue, 2008-10-07 at 22:06 +0200, Tomas Winkler wrote:
>
>> > bytes (with nibbles): | . | . | . | . | . | . || . | . | . | . | . | . |
>> >                      | addr 1          |len1 || addr 2          |len2 |
>> > your forced layout:   |               |                |               |
>> > my layout:            |               |       ||               |       |
>> >
>> > Note how this actually matches the border between the border between the
>> > two descriptors, the || border.
>>
>> Your layout put addr2 (which is 32 bit) sits on 16 bit boundary
>
> So? Yours makes it need two words, which is even less efficient to
> access. And since the struct is packed, on those architectures that
> don't do unaligned accesses efficiently won't need any put_unaligned
> either.
>
> Also, keep in mind that address 2 is _never_ used at all anyway
This is not correct we always use first 2 pointers. First for tx command
second for the actual packet.

because
> we don't and cannot do fragmented frames due to the lack of IP
> checksumming in hardware, as I and davem have tried to explain to you
> multiple times already.

This is bad side of current Linux implementation
Tomas

(Continue reading)

Johannes Berg | 8 Oct 10:02

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, 2008-10-07 at 22:46 +0200, Tomas Winkler wrote:
> >> > bytes (with nibbles): | . | . | . | . | . | . || . | . | . | . | . | . |
> >> >                      | addr 1          |len1 || addr 2          |len2 |
> >> > your forced layout:   |               |                |               |
> >> > my layout:            |               |       ||               |       |
> >> >
> >> > Note how this actually matches the border between the border between the
> >> > two descriptors, the || border.
> >>
> >> Your layout put addr2 (which is 32 bit) sits on 16 bit boundary
> >
> > So? Yours makes it need two words, which is even less efficient to
> > access. And since the struct is packed, on those architectures that
> > don't do unaligned accesses efficiently won't need any put_unaligned
> > either.
> >
> > Also, keep in mind that address 2 is _never_ used at all anyway

> This is not correct we always use first 2 pointers. First for tx command
> second for the actual packet.

Ok so you use two. Writing three words in total. I strongly suggest that
you have WAY more trouble in iwlwifi than an imagined performance issue
coming from a corrected and understandable struct layout.

> > because
> > we don't and cannot do fragmented frames due to the lack of IP
> > checksumming in hardware, as I and davem have tried to explain to you
> > multiple times already.
>

(Continue reading)

Tomas Winkler | 8 Oct 14:38

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Wed, Oct 8, 2008 at 10:02 AM, Johannes Berg
<johannes@...> wrote:
> On Tue, 2008-10-07 at 22:46 +0200, Tomas Winkler wrote:
>> >> > bytes (with nibbles): | . | . | . | . | . | . || . | . | . | . | . | . |
>> >> >                      | addr 1          |len1 || addr 2          |len2 |
>> >> > your forced layout:   |               |                |               |
>> >> > my layout:            |               |       ||               |       |
>> >> >
>> >> > Note how this actually matches the border between the border between the
>> >> > two descriptors, the || border.
>> >>
>> >> Your layout put addr2 (which is 32 bit) sits on 16 bit boundary
>> >
>> > So? Yours makes it need two words, which is even less efficient to
>> > access. And since the struct is packed, on those architectures that
>> > don't do unaligned accesses efficiently won't need any put_unaligned
>> > either.
>> >
>> > Also, keep in mind that address 2 is _never_ used at all anyway
>
>> This is not correct we always use first 2 pointers. First for tx command
>> second for the actual packet.
>
> Ok so you use two.

My only point is just to make sure that you understand that __never__
is not correct

 Writing three words in total. I strongly suggest that
> you have WAY more trouble in iwlwifi than an imagined performance issue

(Continue reading)

Johannes Berg | 9 Oct 11:52

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Wed, 2008-10-08 at 14:38 +0200, Tomas Winkler wrote:

> >> > Also, keep in mind that address 2 is _never_ used at all anyway
> >
> >> This is not correct we always use first 2 pointers. First for tx command
> >> second for the actual packet.
> >
> > Ok so you use two.
> 
> My only point is just to make sure that you understand that __never__
> is not correct

Right, sorry, I really should have checked.

> > Writing three words in total. I strongly suggest that
> > you have WAY more trouble in iwlwifi than an imagined performance issue
> > coming from a corrected and understandable struct layout.
> 
> You are rally trying breaking into open doors Currently I'm more
> concern with correctness then performance so I wanted to rise
> hopefully all issues.
> I'm testing your layout it's work so far in my home setup. I'm on
> holidays till EOW so I will be able to give it some more stress in Lab
> only next week.

Sure, that's fine, I can't really see what can go wrong but testing is
always good. It just seemed to me that for some reason you preferred
keeping the pair-layout and complicating the code?

johannes

(Continue reading)

Tomas Winkler | 9 Oct 18:35

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Thu, Oct 9, 2008 at 11:52 AM, Johannes Berg
<johannes@...> wrote:
> On Wed, 2008-10-08 at 14:38 +0200, Tomas Winkler wrote:
>
>> >> > Also, keep in mind that address 2 is _never_ used at all anyway
>> >
>> >> This is not correct we always use first 2 pointers. First for tx command
>> >> second for the actual packet.
>> >
>> > Ok so you use two.
>>
>> My only point is just to make sure that you understand that __never__
>> is not correct
>
> Right, sorry, I really should have checked.
>
>> > Writing three words in total. I strongly suggest that
>> > you have WAY more trouble in iwlwifi than an imagined performance issue
>> > coming from a corrected and understandable struct layout.
>>
>> You are rally trying breaking into open doors Currently I'm more
>> concern with correctness then performance so I wanted to rise
>> hopefully all issues.
>> I'm testing your layout it's work so far in my home setup. I'm on
>> holidays till EOW so I will be able to give it some more stress in Lab
>> only next week.
>
> Sure, that's fine, I can't really see what can go wrong but testing is
> always good.

(Continue reading)

Tomas Winkler | 7 Oct 22:01

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 7, 2008 at 9:34 PM, Johannes Berg <johannes@...> wrote:
> On Tue, 2008-10-07 at 21:28 +0200, Tomas Winkler wrote:
>
>> > And please keep the 10 pairs -> 20 addresses, it's a lot easier to
>> > understand :)
>
>> I don't like on this solution two things one is that it 32 bit
>> variables sits on unaligned address
>
> That doesn't matter at all since it's packed, and the bitwise accesses
> you did previously are way less efficient.

This is not packet this is shared memory.

Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Johannes Berg | 7 Oct 22:06

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, 2008-10-07 at 22:01 +0200, Tomas Winkler wrote:
> On Tue, Oct 7, 2008 at 9:34 PM, Johannes Berg <johannes@...> wrote:
> > On Tue, 2008-10-07 at 21:28 +0200, Tomas Winkler wrote:
> >
> >> > And please keep the 10 pairs -> 20 addresses, it's a lot easier to
> >> > understand :)
> >
> >> I don't like on this solution two things one is that it 32 bit
> >> variables sits on unaligned address
> >
> > That doesn't matter at all since it's packed, and the bitwise accesses
> > you did previously are way less efficient.
> 
> This is not packet this is shared memory.

??
The struct is _packed_, nothing to do with packets.

johannes

Tomas Winkler | 7 Oct 22:08

Re: [PATCH] iwlwifi: get rid of IWL_{GET,SET}_BITS crap

On Tue, Oct 7, 2008 at 10:06 PM, Johannes Berg
<johannes@...> wrote:
> On Tue, 2008-10-07 at 22:01 +0200, Tomas Winkler wrote:
>> On Tue, Oct 7, 2008 at 9:34 PM, Johannes Berg <johannes@...> wrote:
>> > On Tue, 2008-10-07 at 21:28 +0200, Tomas Winkler wrote:
>> >
>> >> > And please keep the 10 pairs -> 20 addresses, it's a lot easier to
>> >> > understand :)
>> >
>> >> I don't like on this solution two things one is that it 32 bit
>> >> variables sits on unaligned address
>> >
>> > That doesn't matter at all since it's packed, and the bitwise accesses
>> > you did previously are way less efficient.
>>
>> This is not packet this is shared memory.
>
> ??
> The struct is _packed_, nothing to do with packets.
I'm tired .. :)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Return

Return to gmane.linux.kernel.wireless.general.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane