Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] libertas/sdio: set ECSI and SCSI bits for 1-bit transfers

5 views
Skip to first unread message

Daniel Mack

unread,
Mar 30, 2010, 1:40:01 PM3/30/10
to
When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
However, the 8686 will only drive this line when the ECSI and SCSI bits
are set in the CCCR_IF register.

Thanks to Alagu Sankar for pointing me in the right direction.

Signed-off-by: Daniel Mack <dan...@caiaq.de>
Cc: Alagu Sankar <alagu...@embwise.com>
Cc: Volker Ernst <volker...@txtr.com>
Cc: Dan Williams <dc...@redhat.com>
Cc: John W. Linville <linv...@tuxdriver.com>
Cc: Holger Schurig <hs4...@mail.mn-solutions.de>
Cc: Bing Zhao <bz...@marvell.com>
Cc: libert...@lists.infradead.org
Cc: linux-w...@vger.kernel.org
Cc: linu...@vger.kernel.org
---
drivers/net/wireless/libertas/if_sdio.c | 24 ++++++++++++++++++++++++
include/linux/mmc/sdio.h | 2 ++
2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 7a73f62..d3170f2 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -34,6 +34,8 @@
#include <linux/mmc/card.h>
#include <linux/mmc/sdio_func.h>
#include <linux/mmc/sdio_ids.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/host.h>

#include "host.h"
#include "decl.h"
@@ -942,6 +944,7 @@ static int if_sdio_probe(struct sdio_func *func,
int ret, i;
unsigned int model;
struct if_sdio_packet *packet;
+ struct mmc_host *host = func->card->host;

lbs_deb_enter(LBS_DEB_SDIO);

@@ -1022,6 +1025,27 @@ static int if_sdio_probe(struct sdio_func *func,
if (ret)
goto disable;

+ /* For 1-bit transfers, we need to enable the interrupt flags in
+ * the CCCR register. Temporarily set the function number to 0
+ * for that. */
+ if ((host->caps & MMC_CAP_SDIO_IRQ) &&
+ (host->ios.bus_width == MMC_BUS_WIDTH_1)) {
+ unsigned int num = func->num;
+ u8 reg;
+
+ func->num = 0;
+ reg = sdio_readb(func, SDIO_CCCR_IF, &ret);
+ if (ret)
+ goto release_int;
+
+ reg |= SDIO_BUS_ECSI | SDIO_BUS_SCSI;
+ sdio_writeb(func, reg, SDIO_CCCR_IF, &ret);
+ if (ret)
+ goto release_int;
+
+ func->num = num;
+ }
+
card->ioport = sdio_readb(func, IF_SDIO_IOPORT, &ret);
if (ret)
goto release_int;
diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h
index 0ebaef5..329a8fa 100644
--- a/include/linux/mmc/sdio.h
+++ b/include/linux/mmc/sdio.h
@@ -94,6 +94,8 @@

#define SDIO_BUS_WIDTH_1BIT 0x00
#define SDIO_BUS_WIDTH_4BIT 0x02
+#define SDIO_BUS_ECSI 0x20 /* Enable continuous SPI interrupt */
+#define SDIO_BUS_SCSI 0x40 /* Support continuous SPI interrupt */

#define SDIO_BUS_ASYNC_INT 0x20

--
1.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Dan Williams

unread,
Mar 30, 2010, 2:40:01 PM3/30/10
to
On Tue, 2010-03-30 at 19:38 +0200, Daniel Mack wrote:
> When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
> However, the 8686 will only drive this line when the ECSI and SCSI bits
> are set in the CCCR_IF register.

While this looks fine enough to me, I don't have enough knowledge of the
SDIO stack to know whether or not this would be considered a layering
violation or not. Given that I don't think there's an active maintainer
for the SDIO subsystem though, I'd say it's fine enough for me...

Any thoughts David since it appears to be at least somewhat SPI related?

Acked-by: Dan Williams <dc...@redhat.com>


--

Daniel Mack

unread,
Mar 30, 2010, 2:50:02 PM3/30/10
to
On Tue, Mar 30, 2010 at 11:37:39AM -0700, Dan Williams wrote:
> On Tue, 2010-03-30 at 19:38 +0200, Daniel Mack wrote:
> > When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
> > However, the 8686 will only drive this line when the ECSI and SCSI bits
> > are set in the CCCR_IF register.
>
> While this looks fine enough to me, I don't have enough knowledge of the
> SDIO stack to know whether or not this would be considered a layering
> violation or not. Given that I don't think there's an active maintainer
> for the SDIO subsystem though, I'd say it's fine enough for me...
>
> Any thoughts David since it appears to be at least somewhat SPI related?

I believe that these bits in question are just abused in this case. The
device is not in SPI mode, so according to the specs, the bits shouldn't
make any difference. This makes me think that this is specific to
libertas, and doesn't belong in any lower level.

> Acked-by: Dan Williams <dc...@redhat.com>

Thanks!

Daniel

Alagu Sankar Vellaichamy

unread,
Mar 31, 2010, 4:30:02 AM3/31/10
to
SCSI is read-only. We ideally should be checking this bit and then
set the ECSI accordingly, rather than setting both ECSI and SCSI.

- Alagu Sankar

Daniel Mack

unread,
Mar 31, 2010, 5:10:01 AM3/31/10
to
On Wed, Mar 31, 2010 at 11:07:06AM +0200, Michał Mirosław wrote:
> 2010/3/30 Daniel Mack <dan...@caiaq.de>:
> [...]
> You should probably just use mmc_io_rw_direct() in this case instead
> of abusing func->num.

Hmm, that function isn't exported, and I didn't want to change this. You
say you'd prefer that? I can cook up something that does it, no problem.


Daniel

Michał Mirosław

unread,
Mar 31, 2010, 5:10:01 AM3/31/10
to
2010/3/30 Daniel Mack <dan...@caiaq.de>:
[...]
> @@ -1022,6 +1025,27 @@ static int if_sdio_probe(struct sdio_func *func,
> � � � �if (ret)
> � � � � � � � �goto disable;
>
> + ďż˝ ďż˝ ďż˝ /* For 1-bit transfers, we need to enable the interrupt flags in
> + ďż˝ ďż˝ ďż˝ ďż˝* the CCCR register. Temporarily set the function number to 0
> + ďż˝ ďż˝ ďż˝ ďż˝* for that. */
> + ďż˝ ďż˝ ďż˝ if ((host->caps & MMC_CAP_SDIO_IRQ) &&
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ (host->ios.bus_width == MMC_BUS_WIDTH_1)) {
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ unsigned int num = func->num;
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ u8 reg;
> +
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ func->num = 0;
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ reg = sdio_readb(func, SDIO_CCCR_IF, &ret);
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ if (ret)
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ goto release_int;
> +
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ reg |= SDIO_BUS_ECSI | SDIO_BUS_SCSI;
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ sdio_writeb(func, reg, SDIO_CCCR_IF, &ret);
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ if (ret)
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ goto release_int;
> +
> + ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ ďż˝ func->num = num;
> + ďż˝ ďż˝ ďż˝ }
> +
> � � � �card->ioport = sdio_readb(func, IF_SDIO_IOPORT, &ret);
> � � � �if (ret)
> � � � � � � � �goto release_int;

You should probably just use mmc_io_rw_direct() in this case instead
of abusing func->num.

Best Regards,
Micha� Miros�aw

Michał Mirosław

unread,
Mar 31, 2010, 6:00:02 AM3/31/10
to

BTW, I can't see any exported functions to access fn#0 directly from
drivers. Maybe it's time to introduce them now - at least CCCR has
some vendor-defined parts that drivers may want to access and there is
a lot of place in CIS area that can be (ab)used by devices.

Best Regards,
Michał Mirosław

Daniel Mack

unread,
Mar 31, 2010, 9:10:02 AM3/31/10
to
On Wed, Mar 31, 2010 at 11:49:49AM +0200, Michał Mirosław wrote:
> W dniu 31 marca 2010 11:08 użytkownik Daniel Mack <dan...@caiaq.de> napisał:
> > Hmm, that function isn't exported, and I didn't want to change this. You
> > say you'd prefer that? I can cook up something that does it, no problem.
>
> BTW, I can't see any exported functions to access fn#0 directly from
> drivers. Maybe it's time to introduce them now - at least CCCR has
> some vendor-defined parts that drivers may want to access and there is
> a lot of place in CIS area that can be (ab)used by devices.

In fact, there is sdio_f0_{read,write}b() - I overlooked them. Will
resend a new patch.

Thanks for checking,
Daniel

Daniel Mack

unread,
Mar 31, 2010, 9:40:01 AM3/31/10
to
When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
However, the 8686 will only drive this line when the ECSI bit is set in
the CCCR_IF register.

Thanks to Alagu Sankar for pointing me in the right direction.

Signed-off-by: Daniel Mack <dan...@caiaq.de>
Cc: Alagu Sankar <alagu...@embwise.com>
Cc: Volker Ernst <volker...@txtr.com>
Cc: Dan Williams <dc...@redhat.com>
Cc: John W. Linville <linv...@tuxdriver.com>
Cc: Holger Schurig <hs4...@mail.mn-solutions.de>
Cc: Bing Zhao <bz...@marvell.com>
Cc: libert...@lists.infradead.org
Cc: linux-w...@vger.kernel.org
Cc: linu...@vger.kernel.org
---

drivers/net/wireless/libertas/if_sdio.c | 21 +++++++++++++++++++++
include/linux/mmc/sdio.h | 2 ++
2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 7a73f62..f89bb8b 100644


--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -34,6 +34,8 @@
#include <linux/mmc/card.h>
#include <linux/mmc/sdio_func.h>
#include <linux/mmc/sdio_ids.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/host.h>

#include "host.h"
#include "decl.h"
@@ -942,6 +944,7 @@ static int if_sdio_probe(struct sdio_func *func,
int ret, i;
unsigned int model;
struct if_sdio_packet *packet;
+ struct mmc_host *host = func->card->host;

lbs_deb_enter(LBS_DEB_SDIO);

@@ -1022,6 +1025,24 @@ static int if_sdio_probe(struct sdio_func *func,
if (ret)
goto disable;

+ /* For 1-bit transfers, we need to enable the interrupt flag in
+ * the CCCR register. Set the MMC_QUIRK_LENIENT_FN0 bit to allow
+ * access to non-vendor registers. */


+ if ((host->caps & MMC_CAP_SDIO_IRQ) &&
+ (host->ios.bus_width == MMC_BUS_WIDTH_1)) {

+ u8 reg;
+
+ func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
+ reg = sdio_f0_readb(func, SDIO_CCCR_IF, &ret);


+ if (ret)
+ goto release_int;
+

+ reg |= SDIO_BUS_ECSI;
+ sdio_f0_writeb(func, reg, SDIO_CCCR_IF, &ret);


+ if (ret)
+ goto release_int;
+ }
+

card->ioport = sdio_readb(func, IF_SDIO_IOPORT, &ret);
if (ret)
goto release_int;

diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h
index 0ebaef5..329a8fa 100644
--- a/include/linux/mmc/sdio.h
+++ b/include/linux/mmc/sdio.h
@@ -94,6 +94,8 @@

#define SDIO_BUS_WIDTH_1BIT 0x00
#define SDIO_BUS_WIDTH_4BIT 0x02
+#define SDIO_BUS_ECSI 0x20 /* Enable continuous SPI interrupt */
+#define SDIO_BUS_SCSI 0x40 /* Support continuous SPI interrupt */

#define SDIO_BUS_ASYNC_INT 0x20

--
1.7.0

--

Daniel Mack

unread,
Mar 31, 2010, 9:40:02 AM3/31/10
to
On Wed, Mar 31, 2010 at 01:53:21PM +0530, Alagu Sankar Vellaichamy wrote:
> SCSI is read-only. We ideally should be checking this bit and then
> set the ECSI accordingly, rather than setting both ECSI and SCSI.

Thanks for noticing. However, the libertas chip does not set SCSI, so we
have to do it unconditionally. New patch has just been sent.

Daniel

Dan Williams

unread,
Apr 5, 2010, 10:50:01 PM4/5/10
to
On Wed, 2010-03-31 at 15:31 +0200, Daniel Mack wrote:
> When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
> However, the 8686 will only drive this line when the ECSI bit is set in
> the CCCR_IF register.
>
> Thanks to Alagu Sankar for pointing me in the right direction.

Could you modify the patch to do this *only* for the 8686? Apparently
only the 8686 this hardware "quirk" and thus this might have odd
side-effects on the 8688 or the 8385.

Check the 'struct if_sdio_card' model member and compare it to
IF_SDIO_MODEL_8686 like if_sdio_probe() does.

Thanks!
Dan


--

Daniel Mack

unread,
Apr 6, 2010, 5:00:02 AM4/6/10
to
When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
However, the 8686 will only drive this line when the ECSI bit is set in
the CCCR_IF register.

Thanks to Alagu Sankar for pointing me in the right direction.

Signed-off-by: Daniel Mack <dan...@caiaq.de>


Cc: Alagu Sankar <alagu...@embwise.com>
Cc: Volker Ernst <volker...@txtr.com>
Cc: Dan Williams <dc...@redhat.com>
Cc: John W. Linville <linv...@tuxdriver.com>
Cc: Holger Schurig <hs4...@mail.mn-solutions.de>
Cc: Bing Zhao <bz...@marvell.com>
Cc: libert...@lists.infradead.org
Cc: linux-w...@vger.kernel.org
Cc: linu...@vger.kernel.org
---

drivers/net/wireless/libertas/if_sdio.c | 22 ++++++++++++++++++++++
include/linux/mmc/sdio.h | 2 ++
2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 7a73f62..33206a9 100644


--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -34,6 +34,8 @@
#include <linux/mmc/card.h>
#include <linux/mmc/sdio_func.h>
#include <linux/mmc/sdio_ids.h>
+#include <linux/mmc/sdio.h>
+#include <linux/mmc/host.h>

#include "host.h"
#include "decl.h"
@@ -942,6 +944,7 @@ static int if_sdio_probe(struct sdio_func *func,
int ret, i;
unsigned int model;
struct if_sdio_packet *packet;
+ struct mmc_host *host = func->card->host;

lbs_deb_enter(LBS_DEB_SDIO);

@@ -1022,6 +1025,25 @@ static int if_sdio_probe(struct sdio_func *func,
if (ret)
goto disable;

+ /* For 1-bit transfers to the 8686 model, we need to enable the
+ * interrupt flag in the CCCR register. Set the MMC_QUIRK_LENIENT_FN0
+ * bit to allow access to non-vendor registers. */
+ if ((card->model == IF_SDIO_MODEL_8686) &&
+ (host->caps & MMC_CAP_SDIO_IRQ) &&

1.7.0.3

Dan Williams

unread,
Apr 6, 2010, 12:10:02 PM4/6/10
to
On Tue, 2010-04-06 at 10:52 +0200, Daniel Mack wrote:
> When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
> However, the 8686 will only drive this line when the ECSI bit is set in
> the CCCR_IF register.
>
> Thanks to Alagu Sankar for pointing me in the right direction.
>
> Signed-off-by: Daniel Mack <dan...@caiaq.de>
> Cc: Alagu Sankar <alagu...@embwise.com>
> Cc: Volker Ernst <volker...@txtr.com>
> Cc: Dan Williams <dc...@redhat.com>
> Cc: John W. Linville <linv...@tuxdriver.com>
> Cc: Holger Schurig <hs4...@mail.mn-solutions.de>
> Cc: Bing Zhao <bz...@marvell.com>
> Cc: libert...@lists.infradead.org
> Cc: linux-w...@vger.kernel.org
> Cc: linu...@vger.kernel.org

Acked-by: Dan Williams <dc...@redhat.com>


--

Daniel Mack

unread,
Apr 13, 2010, 6:40:02 AM4/13/10
to
On Tue, Apr 06, 2010 at 09:07:34AM -0700, Dan Williams wrote:
> On Tue, 2010-04-06 at 10:52 +0200, Daniel Mack wrote:
> > When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
> > However, the 8686 will only drive this line when the ECSI bit is set in
> > the CCCR_IF register.
> >
> > Thanks to Alagu Sankar for pointing me in the right direction.
> >
> > Signed-off-by: Daniel Mack <dan...@caiaq.de>
> > Cc: Alagu Sankar <alagu...@embwise.com>
> > Cc: Volker Ernst <volker...@txtr.com>
> > Cc: Dan Williams <dc...@redhat.com>
> > Cc: John W. Linville <linv...@tuxdriver.com>
> > Cc: Holger Schurig <hs4...@mail.mn-solutions.de>
> > Cc: Bing Zhao <bz...@marvell.com>
> > Cc: libert...@lists.infradead.org
> > Cc: linux-w...@vger.kernel.org
> > Cc: linu...@vger.kernel.org
>
> Acked-by: Dan Williams <dc...@redhat.com>

Was this picked by anyone?
Just asking because I didn't see it in the wireless-2.6.git yet.

Thanks,
Daniel

John W. Linville

unread,
Apr 13, 2010, 9:20:03 AM4/13/10
to
On Tue, Apr 13, 2010 at 12:29:54PM +0200, Daniel Mack wrote:
> On Tue, Apr 06, 2010 at 09:07:34AM -0700, Dan Williams wrote:
> > On Tue, 2010-04-06 at 10:52 +0200, Daniel Mack wrote:
> > > When operating in 1-bit mode, SDAT1 is used as dedicated interrupt line.
> > > However, the 8686 will only drive this line when the ECSI bit is set in
> > > the CCCR_IF register.
> > >
> > > Thanks to Alagu Sankar for pointing me in the right direction.
> > >
> > > Signed-off-by: Daniel Mack <dan...@caiaq.de>
> > > Cc: Alagu Sankar <alagu...@embwise.com>
> > > Cc: Volker Ernst <volker...@txtr.com>
> > > Cc: Dan Williams <dc...@redhat.com>
> > > Cc: John W. Linville <linv...@tuxdriver.com>
> > > Cc: Holger Schurig <hs4...@mail.mn-solutions.de>
> > > Cc: Bing Zhao <bz...@marvell.com>
> > > Cc: libert...@lists.infradead.org
> > > Cc: linux-w...@vger.kernel.org
> > > Cc: linu...@vger.kernel.org
> >
> > Acked-by: Dan Williams <dc...@redhat.com>
>
> Was this picked by anyone?
> Just asking because I didn't see it in the wireless-2.6.git yet.

It is in wireless-next-2.6, queued for 2.6.35.

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

Daniel Mack

unread,
Apr 13, 2010, 9:30:03 AM4/13/10
to
On Tue, Apr 13, 2010 at 09:06:54AM -0400, John W. Linville wrote:
> On Tue, Apr 13, 2010 at 12:29:54PM +0200, Daniel Mack wrote:
> > Was this picked by anyone?
> > Just asking because I didn't see it in the wireless-2.6.git yet.
>
> It is in wireless-next-2.6, queued for 2.6.35.

Thanks a lot!

Daniel

0 new messages