From: Cliff Cai <[email protected]>
Introduce POWEROF2_BLOCKSIZE_ONLY option for those SD/SDIO host
which only support transferring block with size of power-of-2
[ Bryan Wu <[email protected]>:
- remove some useless coding style cleanup
- using roundup() function as upstream does
]
Signed-off-by: Cliff Cai <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
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
On Fri, Feb 6, 2009 at 3:47 PM, Bryan Wu <[email protected]> wrote:
> On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <[email protected]> wrote:
>> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
>>> From: Cliff Cai <[email protected]>
>>>
>>> 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 <[email protected]>:
>>> - remove some useless coding style cleanup
>>> - using roundup() function as upstream does
>>> ]
>>>
>>> Signed-off-by: Cliff Cai <[email protected]>
>>> Signed-off-by: Bryan Wu <[email protected]>
>>> ---
>>> 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) {
>>
>>
>
On Fri, Feb 06, 2009 at 04:00:28PM +0800, Bryan Wu wrote:
> On Fri, Feb 6, 2009 at 3:47 PM, Bryan Wu <[email protected]> wrote:
> > On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <[email protected]> wrote:
> >> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
> >>> From: Cliff Cai <[email protected]>
> >>>
> >>> 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
[email protected] might be all we have. Be ready.
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 <[email protected]> wrote:
> > > On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <[email protected]> wrote:
> > >> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
> > >>> From: Cliff Cai <[email protected]>
> > >>>
> > >>> 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
On Wed, Feb 11, 2009 at 12:26 AM, Dan Williams <[email protected]> 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 <[email protected]> wrote:
>> > > On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <[email protected]> wrote:
>> > >> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
>> > >>> From: Cliff Cai <[email protected]>
>> > >>>
>> > >>> 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
On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
> From: Cliff Cai <[email protected]>
>
> 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 <[email protected]>:
> - remove some useless coding style cleanup
> - using roundup() function as upstream does
> ]
>
> Signed-off-by: Cliff Cai <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> 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) {
On Thu, Feb 5, 2009 at 11:03 PM, Dan Williams <[email protected]> wrote:
> On Thu, 2009-02-05 at 15:30 +0800, Bryan Wu wrote:
>> From: Cliff Cai <[email protected]>
>>
>> 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 <[email protected]>:
>> - remove some useless coding style cleanup
>> - using roundup() function as upstream does
>> ]
>>
>> Signed-off-by: Cliff Cai <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> ---
>> 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) {
>
>