Re: [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS

[Dominik Brodowski <linux@dominikbrodowski.net>]

[-- Attachment #1: Type: text/plain, Size: 4401 bytes --]

On Thu, Dec 04, 2014 at 09:30:56PM +0000, Alan Cox wrote:
> The core pcmcia code blows up all over the place if it allowed a card without
> a valid CIS. We need to allow such cards as the CIS stuff is not on the older
> flash, ROM and SRAM cards. We give it a suitably blank fake CIS instead.
> 
> In order to minimise the risk of misidentifying junk and feeding it to the
> wrong thing we only fix up apparently anonymous cards if the driver for them
> has been enabled.

Unfortunately, this patch does not work well with all of the callers of
pccard_validate_cis(). While it helps for ds.c:pcmcia_card_add() and does
not matter for cistpl.c:pccard_show_cis(), it breaks the callback in
rsrc_nonstatic.c:readable():

There, we test whether iomem resources actually work -- and we test this
by reading the CIS. This patch means that non-working resources are assumed
to work -- and the valid CIS is replaced with the fake CIS in this case.

Therefore, I'd suggest to move the override to the one place where it is
needed -- to ds.c:pcmcia_card_add(). A patch which implements this is below;
it fixes my test setup (which needs rsrc_nonstatic.c).

Alan, could you verify this patch helps with the use case you had in mind
when writing this patch? I inted to apply this patch to the PCMCIA tree only
after such testing.

Best,
	Dominik


--------------------------------8<---------------------------------
pcmcia: do not break rsrc_nonstatic when handling anonymous cards

Patch 1c6c9b1d9d25 caused a regression for rsrc_nonstatic: It relies
on pccard_validate_cis() to determine whether an iomem resource can
be used for PCMCIA cards. This override, however, lead invalid iomem
resources to be accepted -- and lead to a fake CIS being used instead
of the original CIS.

To fix this issue, move the override for anonymous cards to the one
place where it is needed -- when adding a PCMCIA device.

CC: <stable@vger.kernel.org> # for v4.0 and v4.1
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index 64d0515..d444415 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1451,26 +1451,16 @@ int pccard_validate_cis(struct pcmcia_socket *s, unsigned int *info)
 done:
 	/* invalidate CIS cache on failure */
 	if (!dev_ok || !ident_ok || !count) {
-#if defined(CONFIG_MTD_PCMCIA_ANONYMOUS)
-		/* Set up as an anonymous card. If we don't have anonymous
-		   memory support then just error the card as there is no
-		   point trying to second guess.
-
-		   Note: some cards have just a device entry, it may be
-		   worth extending support to cover these in future */
-		if (!dev_ok || !ident_ok) {
-			dev_info(&s->dev, "no CIS, assuming an anonymous memory card.\n");
-			pcmcia_replace_cis(s, "\xFF", 1);
-			count = 1;
-			ret = 0;
-		} else
-#endif
-		{
-			mutex_lock(&s->ops_mutex);
-			destroy_cis_cache(s);
-			mutex_unlock(&s->ops_mutex);
+		mutex_lock(&s->ops_mutex);
+		destroy_cis_cache(s);
+		mutex_unlock(&s->ops_mutex);
+		/* We differentiate between dev_ok, ident_ok and count
+		   failures to allow for an override for anonymous cards
+		   in ds.c */
+		if (!dev_ok || !ident_ok)
 			ret = -EIO;
-		}
+		else
+			ret = -EFAULT;
 	}
 
 	if (info)
diff --git a/drivers/pcmcia/ds.c b/drivers/pcmcia/ds.c
index d3baf0b..e1498a0 100644
--- a/drivers/pcmcia/ds.c
+++ b/drivers/pcmcia/ds.c
@@ -634,8 +634,24 @@ static int pcmcia_card_add(struct pcmcia_socket *s)
 
 	ret = pccard_validate_cis(s, &no_chains);
 	if (ret || !no_chains) {
-		dev_dbg(&s->dev, "invalid CIS or invalid resources\n");
-		return -ENODEV;
+#if defined(CONFIG_MTD_PCMCIA_ANONYMOUS)
+		/* Set up as an anonymous card. If we don't have anonymous
+		   memory support then just error the card as there is no
+		   point trying to second guess.
+
+		   Note: some cards have just a device entry, it may be
+		   worth extending support to cover these in future */
+		if (ret == -EIO) {
+			dev_info(&s->dev, "no CIS, assuming an anonymous memory card.\n");
+			pcmcia_replace_cis(s, "\xFF", 1);
+			no_chains = 1;
+			ret = 0;
+		} else
+#endif
+		{
+			dev_dbg(&s->dev, "invalid CIS or invalid resources\n");
+			return -ENODEV;
+		}
 	}
 
 	if (!pccard_read_tuple(s, BIND_FN_ALL, CISTPL_LONGLINK_MFC, &mfc))

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]