All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
@ 2009-02-05  7:30 Bryan Wu
  2009-02-05 15:03 ` Dan Williams
  0 siblings, 1 reply; 7+ messages in thread
From: Bryan Wu @ 2009-02-05  7:30 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, linux-kernel, Cliff Cai, Bryan Wu

From: Cliff Cai <cliff.cai@analog.com>

Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
which only support transferring block with size of power-of-2

[ Bryan Wu <cooloney@kernel.org>:
 - remove some useless coding style cleanup
 - using roundup() function as upstream does
]

Signed-off-by: Cliff Cai <cliff.cai@analog.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/net/wireless/Kconfig            |    6 ++++++
 drivers/net/wireless/libertas/if_sdio.c |   16 ++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
index e4f9f74..a2685fa 100644
--- a/drivers/net/wireless/Kconfig
+++ b/drivers/net/wireless/Kconfig
@@ -151,6 +151,12 @@ config LIBERTAS_SDIO
 	---help---
 	  A driver for Marvell Libertas 8385 and 8686 SDIO devices.
 
+config POWEROF2_BLOCKSIZE_ONLY
+	bool "Support transferring block with size of power-of-2 only"
+	depends on LIBERTAS_SDIO
+	---help---
+	  For SD/SDIO host which only supports transferring block with size of power-of-2.
+
 config LIBERTAS_DEBUG
 	bool "Enable full debugging output in the Libertas module."
 	depends on LIBERTAS
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 4519d73..5efc056 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -272,6 +272,11 @@ static int if_sdio_card_to_host(struct if_sdio_card *card)
 	 */
 	chunk = sdio_align_size(card->func, size);
 
+/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
+#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
+	chunk = (chunk + card->func->cur_blksize - 1) /
+			card->func->cur_blksize * card->func->cur_blksize;
+#endif
 	ret = sdio_readsb(card->func, card->buffer, card->ioport, chunk);
 	if (ret)
 		goto out;
@@ -581,8 +586,14 @@ static int if_sdio_prog_real(struct if_sdio_card *card)
 			lbs_deb_sdio("sending %d bytes (%d bytes) chunk\n",
 				chunk_size, (chunk_size + 31) / 32 * 32);
 */
+/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
+#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
+  			ret = sdio_writesb(card->func, card->ioport,
+ 				chunk_buffer, roundup(chunk_size, 256);
+#else
 			ret = sdio_writesb(card->func, card->ioport,
 				chunk_buffer, roundup(chunk_size, 32));
+#endif
 			if (ret)
 				goto release;
 
@@ -699,6 +710,11 @@ static int if_sdio_host_to_card(struct lbs_private *priv,
 	 */
 	size = sdio_align_size(card->func, nb + 4);
 
+/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
+#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
+	size = (size + card->func->cur_blksize - 1) /
+			card->func->cur_blksize * card->func->cur_blksize;
+#endif
 	packet = kzalloc(sizeof(struct if_sdio_packet) + size,
 			GFP_ATOMIC);
 	if (!packet) {
-- 
1.5.6.3

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

* Re: [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
  2009-02-05  7:30 [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option Bryan Wu
@ 2009-02-05 15:03 ` Dan Williams
  2009-02-06  7:47   ` Bryan Wu
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2009-02-05 15:03 UTC (permalink / raw)
  To: Bryan Wu; +Cc: linville, linux-wireless, linux-kernel, Cliff Cai

On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
> From: Cliff Cai <cliff.cai@analog.com>
> 
> Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
> which only support transferring block with size of power-of-2

Is the point here to avoid copying in the controller code?  As with the
other patches on libertas-dev, I really dislike adding code to *every
SDIO driver* just because the host has certain restrictions.  I'd much
rather that the host/controller code became aware of it's own
restrictions, and exposed those generically to drivers above it.
Without a KConfig option.

Seriously.  The host knows what it needs.  The code to handle that
should go in the host.

How about adding a method like "sdio_align_size" that takes the
controller's constraints into account?  That seems a lot cleaner than
adding #define/KConfig junk to every SDIO driver in the kernel.  One
less codepath to test, makes your life and all our lives easier.

Dan

> [ Bryan Wu <cooloney@kernel.org>:
>  - remove some useless coding style cleanup
>  - using roundup() function as upstream does
> ]
> 
> Signed-off-by: Cliff Cai <cliff.cai@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> ---
>  drivers/net/wireless/Kconfig            |    6 ++++++
>  drivers/net/wireless/libertas/if_sdio.c |   16 ++++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
> index e4f9f74..a2685fa 100644
> --- a/drivers/net/wireless/Kconfig
> +++ b/drivers/net/wireless/Kconfig
> @@ -151,6 +151,12 @@ config LIBERTAS_SDIO
>  	---help---
>  	  A driver for Marvell Libertas 8385 and 8686 SDIO devices.
>  
> +config POWEROF2_BLOCKSIZE_ONLY
> +	bool "Support transferring block with size of power-of-2 only"
> +	depends on LIBERTAS_SDIO
> +	---help---
> +	  For SD/SDIO host which only supports transferring block with size of power-of-2.
> +
>  config LIBERTAS_DEBUG
>  	bool "Enable full debugging output in the Libertas module."
>  	depends on LIBERTAS
> diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
> index 4519d73..5efc056 100644
> --- a/drivers/net/wireless/libertas/if_sdio.c
> +++ b/drivers/net/wireless/libertas/if_sdio.c
> @@ -272,6 +272,11 @@ static int if_sdio_card_to_host(struct if_sdio_card *card)
>  	 */
>  	chunk = sdio_align_size(card->func, size);
>  
> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
> +	chunk = (chunk + card->func->cur_blksize - 1) /
> +			card->func->cur_blksize * card->func->cur_blksize;
> +#endif
>  	ret = sdio_readsb(card->func, card->buffer, card->ioport, chunk);
>  	if (ret)
>  		goto out;
> @@ -581,8 +586,14 @@ static int if_sdio_prog_real(struct if_sdio_card *card)
>  			lbs_deb_sdio("sending %d bytes (%d bytes) chunk\n",
>  				chunk_size, (chunk_size + 31) / 32 * 32);
>  */
> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
> +  			ret = sdio_writesb(card->func, card->ioport,
> + 				chunk_buffer, roundup(chunk_size, 256);
> +#else
>  			ret = sdio_writesb(card->func, card->ioport,
>  				chunk_buffer, roundup(chunk_size, 32));
> +#endif
>  			if (ret)
>  				goto release;
>  
> @@ -699,6 +710,11 @@ static int if_sdio_host_to_card(struct lbs_private *priv,
>  	 */
>  	size = sdio_align_size(card->func, nb + 4);
>  
> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
> +	size = (size + card->func->cur_blksize - 1) /
> +			card->func->cur_blksize * card->func->cur_blksize;
> +#endif
>  	packet = kzalloc(sizeof(struct if_sdio_packet) + size,
>  			GFP_ATOMIC);
>  	if (!packet) {


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

* Re: [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
  2009-02-05 15:03 ` Dan Williams
@ 2009-02-06  7:47   ` Bryan Wu
  2009-02-06  8:00     ` Bryan Wu
  0 siblings, 1 reply; 7+ messages in thread
From: Bryan Wu @ 2009-02-06  7:47 UTC (permalink / raw)
  To: Dan Williams; +Cc: linville, linux-wireless, linux-kernel, Cliff Cai

On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
>> From: Cliff Cai <cliff.cai@analog.com>
>>
>> Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
>> which only support transferring block with size of power-of-2
>
> Is the point here to avoid copying in the controller code?  As with the
> other patches on libertas-dev, I really dislike adding code to *every
> SDIO driver* just because the host has certain restrictions.  I'd much
> rather that the host/controller code became aware of it's own
> restrictions, and exposed those generically to drivers above it.
> Without a KConfig option.
>
> Seriously.  The host knows what it needs.  The code to handle that
> should go in the host.
>

I agree here.

> How about adding a method like "sdio_align_size" that takes the
> controller's constraints into account?  That seems a lot cleaner than
> adding #define/KConfig junk to every SDIO driver in the kernel.  One
> less codepath to test, makes your life and all our lives easier.
>

So we plan to add method ".sdio_align_size" to SDIO stack.
And Blackfin host driver will implement this method while others will
implement this as a dummy function.

In libertas, we need to call this method to take this constraints into account.

Thanks
-Bryan


>> [ Bryan Wu <cooloney@kernel.org>:
>>  - remove some useless coding style cleanup
>>  - using roundup() function as upstream does
>> ]
>>
>> Signed-off-by: Cliff Cai <cliff.cai@analog.com>
>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>> ---
>>  drivers/net/wireless/Kconfig            |    6 ++++++
>>  drivers/net/wireless/libertas/if_sdio.c |   16 ++++++++++++++++
>>  2 files changed, 22 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
>> index e4f9f74..a2685fa 100644
>> --- a/drivers/net/wireless/Kconfig
>> +++ b/drivers/net/wireless/Kconfig
>> @@ -151,6 +151,12 @@ config LIBERTAS_SDIO
>>       ---help---
>>         A driver for Marvell Libertas 8385 and 8686 SDIO devices.
>>
>> +config POWEROF2_BLOCKSIZE_ONLY
>> +     bool "Support transferring block with size of power-of-2 only"
>> +     depends on LIBERTAS_SDIO
>> +     ---help---
>> +       For SD/SDIO host which only supports transferring block with size of power-of-2.
>> +
>>  config LIBERTAS_DEBUG
>>       bool "Enable full debugging output in the Libertas module."
>>       depends on LIBERTAS
>> diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
>> index 4519d73..5efc056 100644
>> --- a/drivers/net/wireless/libertas/if_sdio.c
>> +++ b/drivers/net/wireless/libertas/if_sdio.c
>> @@ -272,6 +272,11 @@ static int if_sdio_card_to_host(struct if_sdio_card *card)
>>        */
>>       chunk = sdio_align_size(card->func, size);
>>
>> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
>> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
>> +     chunk = (chunk + card->func->cur_blksize - 1) /
>> +                     card->func->cur_blksize * card->func->cur_blksize;
>> +#endif
>>       ret = sdio_readsb(card->func, card->buffer, card->ioport, chunk);
>>       if (ret)
>>               goto out;
>> @@ -581,8 +586,14 @@ static int if_sdio_prog_real(struct if_sdio_card *card)
>>                       lbs_deb_sdio("sending %d bytes (%d bytes) chunk\n",
>>                               chunk_size, (chunk_size + 31) / 32 * 32);
>>  */
>> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
>> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
>> +                     ret = sdio_writesb(card->func, card->ioport,
>> +                             chunk_buffer, roundup(chunk_size, 256);
>> +#else
>>                       ret = sdio_writesb(card->func, card->ioport,
>>                               chunk_buffer, roundup(chunk_size, 32));
>> +#endif
>>                       if (ret)
>>                               goto release;
>>
>> @@ -699,6 +710,11 @@ static int if_sdio_host_to_card(struct lbs_private *priv,
>>        */
>>       size = sdio_align_size(card->func, nb + 4);
>>
>> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
>> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
>> +     size = (size + card->func->cur_blksize - 1) /
>> +                     card->func->cur_blksize * card->func->cur_blksize;
>> +#endif
>>       packet = kzalloc(sizeof(struct if_sdio_packet) + size,
>>                       GFP_ATOMIC);
>>       if (!packet) {
>
>

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

* Re: [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
  2009-02-06  7:47   ` Bryan Wu
@ 2009-02-06  8:00     ` Bryan Wu
  2009-02-10 16:04       ` John W. Linville
  0 siblings, 1 reply; 7+ messages in thread
From: Bryan Wu @ 2009-02-06  8:00 UTC (permalink / raw)
  To: Dan Williams; +Cc: linville, linux-wireless, linux-kernel, Cliff Cai

On Fri, Feb 6, 2009 at 3:47 PM, Bryan Wu <cooloney@kernel.org> wrote:
> On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <dcbw@redhat.com> wrote:
>> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
>>> From: Cliff Cai <cliff.cai@analog.com>
>>>
>>> Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
>>> which only support transferring block with size of power-of-2
>>
>> Is the point here to avoid copying in the controller code?  As with the
>> other patches on libertas-dev, I really dislike adding code to *every
>> SDIO driver* just because the host has certain restrictions.  I'd much
>> rather that the host/controller code became aware of it's own
>> restrictions, and exposed those generically to drivers above it.
>> Without a KConfig option.
>>
>> Seriously.  The host knows what it needs.  The code to handle that
>> should go in the host.
>>
>
> I agree here.
>
>> How about adding a method like "sdio_align_size" that takes the
>> controller's constraints into account?  That seems a lot cleaner than
>> adding #define/KConfig junk to every SDIO driver in the kernel.  One
>> less codepath to test, makes your life and all our lives easier.
>>
>
> So we plan to add method ".sdio_align_size" to SDIO stack.
> And Blackfin host driver will implement this method while others will
> implement this as a dummy function.
>

sdio_align_size is already in SDIO stack, so we just need to add our
constraints to this function.

Thanks
-Bryan

>
>>> [ Bryan Wu <cooloney@kernel.org>:
>>>  - remove some useless coding style cleanup
>>>  - using roundup() function as upstream does
>>> ]
>>>
>>> Signed-off-by: Cliff Cai <cliff.cai@analog.com>
>>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>>> ---
>>>  drivers/net/wireless/Kconfig            |    6 ++++++
>>>  drivers/net/wireless/libertas/if_sdio.c |   16 ++++++++++++++++
>>>  2 files changed, 22 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
>>> index e4f9f74..a2685fa 100644
>>> --- a/drivers/net/wireless/Kconfig
>>> +++ b/drivers/net/wireless/Kconfig
>>> @@ -151,6 +151,12 @@ config LIBERTAS_SDIO
>>>       ---help---
>>>         A driver for Marvell Libertas 8385 and 8686 SDIO devices.
>>>
>>> +config POWEROF2_BLOCKSIZE_ONLY
>>> +     bool "Support transferring block with size of power-of-2 only"
>>> +     depends on LIBERTAS_SDIO
>>> +     ---help---
>>> +       For SD/SDIO host which only supports transferring block with size of power-of-2.
>>> +
>>>  config LIBERTAS_DEBUG
>>>       bool "Enable full debugging output in the Libertas module."
>>>       depends on LIBERTAS
>>> diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
>>> index 4519d73..5efc056 100644
>>> --- a/drivers/net/wireless/libertas/if_sdio.c
>>> +++ b/drivers/net/wireless/libertas/if_sdio.c
>>> @@ -272,6 +272,11 @@ static int if_sdio_card_to_host(struct if_sdio_card *card)
>>>        */
>>>       chunk = sdio_align_size(card->func, size);
>>>
>>> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
>>> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
>>> +     chunk = (chunk + card->func->cur_blksize - 1) /
>>> +                     card->func->cur_blksize * card->func->cur_blksize;
>>> +#endif
>>>       ret = sdio_readsb(card->func, card->buffer, card->ioport, chunk);
>>>       if (ret)
>>>               goto out;
>>> @@ -581,8 +586,14 @@ static int if_sdio_prog_real(struct if_sdio_card *card)
>>>                       lbs_deb_sdio("sending %d bytes (%d bytes) chunk\n",
>>>                               chunk_size, (chunk_size + 31) / 32 * 32);
>>>  */
>>> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
>>> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
>>> +                     ret = sdio_writesb(card->func, card->ioport,
>>> +                             chunk_buffer, roundup(chunk_size, 256);
>>> +#else
>>>                       ret = sdio_writesb(card->func, card->ioport,
>>>                               chunk_buffer, roundup(chunk_size, 32));
>>> +#endif
>>>                       if (ret)
>>>                               goto release;
>>>
>>> @@ -699,6 +710,11 @@ static int if_sdio_host_to_card(struct lbs_private *priv,
>>>        */
>>>       size = sdio_align_size(card->func, nb + 4);
>>>
>>> +/* For SD/SDIO host which only supports transferring block with size of power-of-2 */
>>> +#if defined(CONFIG_POWEROF2_BLOCKSIZE_ONLY)
>>> +     size = (size + card->func->cur_blksize - 1) /
>>> +                     card->func->cur_blksize * card->func->cur_blksize;
>>> +#endif
>>>       packet = kzalloc(sizeof(struct if_sdio_packet) + size,
>>>                       GFP_ATOMIC);
>>>       if (!packet) {
>>
>>
>

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

* Re: [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
  2009-02-06  8:00     ` Bryan Wu
@ 2009-02-10 16:04       ` John W. Linville
  2009-02-10 16:26         ` Dan Williams
  0 siblings, 1 reply; 7+ messages in thread
From: John W. Linville @ 2009-02-10 16:04 UTC (permalink / raw)
  To: Bryan Wu; +Cc: Dan Williams, linux-wireless, linux-kernel, Cliff Cai

On Fri, Feb 06, 2009 at 04:00:28PM +0800, Bryan Wu wrote:
> On Fri, Feb 6, 2009 at 3:47 PM, Bryan Wu <cooloney@kernel.org> wrote:
> > On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <dcbw@redhat.com> wrote:
> >> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
> >>> From: Cliff Cai <cliff.cai@analog.com>
> >>>
> >>> Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
> >>> which only support transferring block with size of power-of-2
> >>
> >> Is the point here to avoid copying in the controller code?  As with the
> >> other patches on libertas-dev, I really dislike adding code to *every
> >> SDIO driver* just because the host has certain restrictions.  I'd much
> >> rather that the host/controller code became aware of it's own
> >> restrictions, and exposed those generically to drivers above it.
> >> Without a KConfig option.
> >>
> >> Seriously.  The host knows what it needs.  The code to handle that
> >> should go in the host.
> >>
> >
> > I agree here.
> >
> >> How about adding a method like "sdio_align_size" that takes the
> >> controller's constraints into account?  That seems a lot cleaner than
> >> adding #define/KConfig junk to every SDIO driver in the kernel.  One
> >> less codepath to test, makes your life and all our lives easier.
> >>
> >
> > So we plan to add method ".sdio_align_size" to SDIO stack.
> > And Blackfin host driver will implement this method while others will
> > implement this as a dummy function.
> >
> 
> sdio_align_size is already in SDIO stack, so we just need to add our
> constraints to this function.

I'm reading this as "this patch is unnecessary or will be replaced
by something better", so I'm dropping it.  If I misread that, feel
free to repost...thanks!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

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

* Re: [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
  2009-02-10 16:04       ` John W. Linville
@ 2009-02-10 16:26         ` Dan Williams
  2009-02-11  3:32           ` Bryan Wu
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2009-02-10 16:26 UTC (permalink / raw)
  To: John W. Linville; +Cc: Bryan Wu, linux-wireless, linux-kernel, Cliff Cai

On Tue, 2009-02-10 at 11:04 -0500, John W. Linville wrote:
> On Fri, Feb 06, 2009 at 04:00:28PM +0800, Bryan Wu wrote:
> > On Fri, Feb 6, 2009 at 3:47 PM, Bryan Wu <cooloney@kernel.org> wrote:
> > > On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <dcbw@redhat.com> wrote:
> > >> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
> > >>> From: Cliff Cai <cliff.cai@analog.com>
> > >>>
> > >>> Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
> > >>> which only support transferring block with size of power-of-2
> > >>
> > >> Is the point here to avoid copying in the controller code?  As with the
> > >> other patches on libertas-dev, I really dislike adding code to *every
> > >> SDIO driver* just because the host has certain restrictions.  I'd much
> > >> rather that the host/controller code became aware of it's own
> > >> restrictions, and exposed those generically to drivers above it.
> > >> Without a KConfig option.
> > >>
> > >> Seriously.  The host knows what it needs.  The code to handle that
> > >> should go in the host.
> > >>
> > >
> > > I agree here.
> > >
> > >> How about adding a method like "sdio_align_size" that takes the
> > >> controller's constraints into account?  That seems a lot cleaner than
> > >> adding #define/KConfig junk to every SDIO driver in the kernel.  One
> > >> less codepath to test, makes your life and all our lives easier.
> > >>
> > >
> > > So we plan to add method ".sdio_align_size" to SDIO stack.
> > > And Blackfin host driver will implement this method while others will
> > > implement this as a dummy function.
> > >
> > 
> > sdio_align_size is already in SDIO stack, so we just need to add our
> > constraints to this function.
> 
> I'm reading this as "this patch is unnecessary or will be replaced
> by something better", so I'm dropping it.  If I misread that, feel
> free to repost...thanks!

Yeah, some patches got posted to the MMC lists that do what this patch
would do, but do it in a somewhat better manner.

Dan



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

* Re: [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option
  2009-02-10 16:26         ` Dan Williams
@ 2009-02-11  3:32           ` Bryan Wu
  0 siblings, 0 replies; 7+ messages in thread
From: Bryan Wu @ 2009-02-11  3:32 UTC (permalink / raw)
  To: Dan Williams; +Cc: John W. Linville, linux-wireless, linux-kernel, Cliff Cai

On Wed, Feb 11, 2009 at 12:26 AM, Dan Williams <dcbw@redhat.com> wrote:
> On Tue, 2009-02-10 at 11:04 -0500, John W. Linville wrote:
>> On Fri, Feb 06, 2009 at 04:00:28PM +0800, Bryan Wu wrote:
>> > On Fri, Feb 6, 2009 at 3:47 PM, Bryan Wu <cooloney@kernel.org> wrote:
>> > > On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <dcbw@redhat.com> wrote:
>> > >> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
>> > >>> From: Cliff Cai <cliff.cai@analog.com>
>> > >>>
>> > >>> Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
>> > >>> which only support transferring block with size of power-of-2
>> > >>
>> > >> Is the point here to avoid copying in the controller code?  As with the
>> > >> other patches on libertas-dev, I really dislike adding code to *every
>> > >> SDIO driver* just because the host has certain restrictions.  I'd much
>> > >> rather that the host/controller code became aware of it's own
>> > >> restrictions, and exposed those generically to drivers above it.
>> > >> Without a KConfig option.
>> > >>
>> > >> Seriously.  The host knows what it needs.  The code to handle that
>> > >> should go in the host.
>> > >>
>> > >
>> > > I agree here.
>> > >
>> > >> How about adding a method like "sdio_align_size" that takes the
>> > >> controller's constraints into account?  That seems a lot cleaner than
>> > >> adding #define/KConfig junk to every SDIO driver in the kernel.  One
>> > >> less codepath to test, makes your life and all our lives easier.
>> > >>
>> > >
>> > > So we plan to add method ".sdio_align_size" to SDIO stack.
>> > > And Blackfin host driver will implement this method while others will
>> > > implement this as a dummy function.
>> > >
>> >
>> > sdio_align_size is already in SDIO stack, so we just need to add our
>> > constraints to this function.
>>
>> I'm reading this as "this patch is unnecessary or will be replaced
>> by something better", so I'm dropping it.  If I misread that, feel
>> free to repost...thanks!
>
> Yeah, some patches got posted to the MMC lists that do what this patch
> would do, but do it in a somewhat better manner.
>

Exactly, Cliff will redesign a new patches for MMC/SD stack to fix this.

Thanks
-Bryan

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

end of thread, other threads:[~2009-02-11  3:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-05  7:30 [PATCH] wireless: introduce POWEROF2_BLOCKSIZE_ONLY option Bryan Wu
2009-02-05 15:03 ` Dan Williams
2009-02-06  7:47   ` Bryan Wu
2009-02-06  8:00     ` Bryan Wu
2009-02-10 16:04       ` John W. Linville
2009-02-10 16:26         ` Dan Williams
2009-02-11  3:32           ` Bryan Wu

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.