All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs
@ 2014-12-04 21:28 Alan Cox
  2014-12-04 21:28 ` [PATCH 1/5] pcmcia: correct types Alan Cox
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Alan Cox @ 2014-12-04 21:28 UTC (permalink / raw)
  To: linux-kernel, linux-pcmica

These patches update the PCMCIA layer to work on 64bit machines and also to
support anonymous memory cards (a feature that was broken in the conversion
from the old pcmcia_cs model to the pccard drivers).

In addition they fix various bugs found during the work, and provide an
alternative rather less hideously convoluted resource manager that gets used
when the only PCMCIA present is via PCI bridges. In that situation the core
pci layer can do the job perfectly well for us and just needs a bit of
wrapping.

Alan

---

Alan Cox (5):
      pcmcia: correct types
      pcmcia cis: on an out of range CIS read return 0xff, don't just warn
      pcmcia: Fix requery
      pcmcia: handle anonymous cards by generating a fake CIS
      pcmcia: add a new resource manager for non ISA systems


 drivers/pcmcia/Kconfig       |   12 ++-
 drivers/pcmcia/Makefile      |    1 
 drivers/pcmcia/cistpl.c      |   31 ++++++--
 drivers/pcmcia/cs_internal.h |    6 +
 drivers/pcmcia/ds.c          |    3 +
 drivers/pcmcia/rsrc_mgr.c    |    5 +
 drivers/pcmcia/rsrc_pci.c    |  172 ++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 216 insertions(+), 14 deletions(-)
 create mode 100644 drivers/pcmcia/rsrc_pci.c

--
"Writing in comic sans is like talking earnestly whilst you have hiccups"

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

* [PATCH 1/5] pcmcia: correct types
  2014-12-04 21:28 [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs Alan Cox
@ 2014-12-04 21:28 ` Alan Cox
  2014-12-04 21:29 ` [PATCH 2/5] pcmcia cis: on an out of range CIS read return 0xff, don't just warn Alan Cox
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Alan Cox @ 2014-12-04 21:28 UTC (permalink / raw)
  To: linux-kernel, linux-pcmica

We should be using resource_size_t and unsigned types correctly, otherwise
we sign extend the flags on a 64bit box, which is not what we want.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/pcmcia/cs_internal.h |    6 +++---
 drivers/pcmcia/rsrc_mgr.c    |    5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pcmcia/cs_internal.h b/drivers/pcmcia/cs_internal.h
index 7f1953f..e86cd6b 100644
--- a/drivers/pcmcia/cs_internal.h
+++ b/drivers/pcmcia/cs_internal.h
@@ -80,9 +80,9 @@ struct pccard_resource_ops {
  * Stuff internal to module "pcmcia_rsrc":
  */
 extern int static_init(struct pcmcia_socket *s);
-extern struct resource *pcmcia_make_resource(unsigned long start,
-					unsigned long end,
-					int flags, const char *name);
+extern struct resource *pcmcia_make_resource(resource_size_t start,
+					resource_size_t end,
+					unsigned long flags, const char *name);
 
 /*
  * Stuff internal to module "pcmcia_core":
diff --git a/drivers/pcmcia/rsrc_mgr.c b/drivers/pcmcia/rsrc_mgr.c
index aa628ed..df2cb70 100644
--- a/drivers/pcmcia/rsrc_mgr.c
+++ b/drivers/pcmcia/rsrc_mgr.c
@@ -30,8 +30,9 @@ int static_init(struct pcmcia_socket *s)
 	return 0;
 }
 
-struct resource *pcmcia_make_resource(unsigned long start, unsigned long end,
-				int flags, const char *name)
+struct resource *pcmcia_make_resource(resource_size_t start,
+					resource_size_t end,
+					unsigned long flags, const char *name)
 {
 	struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
 


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

* [PATCH 2/5] pcmcia cis: on an out of range CIS read return 0xff, don't just warn
  2014-12-04 21:28 [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs Alan Cox
  2014-12-04 21:28 ` [PATCH 1/5] pcmcia: correct types Alan Cox
@ 2014-12-04 21:29 ` Alan Cox
  2014-12-04 21:30 ` [PATCH 3/5] pcmcia: Fix requery Alan Cox
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Alan Cox @ 2014-12-04 21:29 UTC (permalink / raw)
  To: linux-kernel, linux-pcmica

The current code displays warnings but then proceeds to try and reference
the data through the PCMCIA window. Instead return 0xff. This prevents bogus
CIS data sending us off into hyperspace.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/pcmcia/cistpl.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index 884a984..4ff725c 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -168,9 +168,12 @@ int pcmcia_read_cis_mem(struct pcmcia_socket *s, int attr, u_int addr,
 	} else {
 		u_int inc = 1, card_offset, flags;
 
-		if (addr > CISTPL_MAX_CIS_SIZE)
+		if (addr > CISTPL_MAX_CIS_SIZE) {
 			dev_dbg(&s->dev,
 				"attempt to read CIS mem at addr %#x", addr);
+			memset(ptr, 0xff, len);
+			return -1;
+		}
 
 		flags = MAP_ACTIVE | ((cis_width) ? MAP_16BIT : 0);
 		if (attr) {


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

* [PATCH 3/5] pcmcia: Fix requery
  2014-12-04 21:28 [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs Alan Cox
  2014-12-04 21:28 ` [PATCH 1/5] pcmcia: correct types Alan Cox
  2014-12-04 21:29 ` [PATCH 2/5] pcmcia cis: on an out of range CIS read return 0xff, don't just warn Alan Cox
@ 2014-12-04 21:30 ` Alan Cox
  2014-12-04 21:30 ` [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS Alan Cox
  2014-12-04 21:31 ` [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems Alan Cox
  4 siblings, 0 replies; 11+ messages in thread
From: Alan Cox @ 2014-12-04 21:30 UTC (permalink / raw)
  To: linux-kernel, linux-pcmica

The requery logic goes off and attempts to read the CIS of empty slots. In
most cases this happens not to do any harm - but not all!

Add the missing check and also a WARN() to catch any other offenders.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/pcmcia/cistpl.c |    2 +-
 drivers/pcmcia/ds.c     |    3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index 4ff725c..8b3b492 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1386,7 +1386,7 @@ int pccard_validate_cis(struct pcmcia_socket *s, unsigned int *info)
 	if (!s)
 		return -EINVAL;
 
-	if (s->functions) {
+	if (s->functions || !(s->state & SOCKET_PRESENT)) {
 		WARN_ON(1);
 		return -EINVAL;
 	}
diff --git a/drivers/pcmcia/ds.c b/drivers/pcmcia/ds.c
index 757119b..d3baf0b 100644
--- a/drivers/pcmcia/ds.c
+++ b/drivers/pcmcia/ds.c
@@ -667,6 +667,9 @@ static void pcmcia_requery(struct pcmcia_socket *s)
 {
 	int has_pfc;
 
+	if (!(s->state & SOCKET_PRESENT))
+		return;
+
 	if (s->functions == 0) {
 		pcmcia_card_add(s);
 		return;


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

* [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS
  2014-12-04 21:28 [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs Alan Cox
                   ` (2 preceding siblings ...)
  2014-12-04 21:30 ` [PATCH 3/5] pcmcia: Fix requery Alan Cox
@ 2014-12-04 21:30 ` Alan Cox
  2015-06-14 19:52   ` Dominik Brodowski
  2014-12-04 21:31 ` [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems Alan Cox
  4 siblings, 1 reply; 11+ messages in thread
From: Alan Cox @ 2014-12-04 21:30 UTC (permalink / raw)
  To: linux-kernel, linux-pcmica

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.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/pcmcia/cistpl.c |   24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index 8b3b492..64d0515 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1451,10 +1451,26 @@ int pccard_validate_cis(struct pcmcia_socket *s, unsigned int *info)
 done:
 	/* invalidate CIS cache on failure */
 	if (!dev_ok || !ident_ok || !count) {
-		mutex_lock(&s->ops_mutex);
-		destroy_cis_cache(s);
-		mutex_unlock(&s->ops_mutex);
-		ret = -EIO;
+#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);
+			ret = -EIO;
+		}
 	}
 
 	if (info)


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

* [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems
  2014-12-04 21:28 [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs Alan Cox
                   ` (3 preceding siblings ...)
  2014-12-04 21:30 ` [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS Alan Cox
@ 2014-12-04 21:31 ` Alan Cox
  2015-05-30 14:40   ` Dominik Brodowski
  4 siblings, 1 reply; 11+ messages in thread
From: Alan Cox @ 2014-12-04 21:31 UTC (permalink / raw)
  To: linux-kernel, linux-pcmica

On a pure PCI platform we don't actually need all the complexity of the
rsrc_nonstatic manager, in fact we can just work directly with the pci
allocators and avoid all the complexity (and code bloat).

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/pcmcia/Kconfig    |   12 ++-
 drivers/pcmcia/Makefile   |    1 
 drivers/pcmcia/rsrc_pci.c |  172 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+), 3 deletions(-)
 create mode 100644 drivers/pcmcia/rsrc_pci.c

diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
index b0ce7cd..2bd21a6 100644
--- a/drivers/pcmcia/Kconfig
+++ b/drivers/pcmcia/Kconfig
@@ -69,7 +69,8 @@ config YENTA
 	tristate "CardBus yenta-compatible bridge support"
 	depends on PCI
 	select CARDBUS if !EXPERT
-	select PCCARD_NONSTATIC if PCMCIA != n
+	select PCCARD_NONSTATIC if PCMCIA != n && ISA
+	select PCCARD_PCI if PCMCIA !=n && !ISA
 	---help---
 	  This option enables support for CardBus host bridges.  Virtually
 	  all modern PCMCIA bridges are CardBus compatible.  A "bridge" is
@@ -109,7 +110,8 @@ config YENTA_TOSHIBA
 config PD6729
 	tristate "Cirrus PD6729 compatible bridge support"
 	depends on PCMCIA && PCI
-	select PCCARD_NONSTATIC
+	select PCCARD_NONSTATIC if PCMCIA != n && ISA
+	select PCCARD_PCI if PCMCIA !=n && !ISA
 	help
 	  This provides support for the Cirrus PD6729 PCI-to-PCMCIA bridge
 	  device, found in some older laptops and PCMCIA card readers.
@@ -117,7 +119,8 @@ config PD6729
 config I82092
 	tristate "i82092 compatible bridge support"
 	depends on PCMCIA && PCI
-	select PCCARD_NONSTATIC
+	select PCCARD_NONSTATIC if PCMCIA != n && ISA
+	select PCCARD_PCI if PCMCIA !=n && !ISA
 	help
 	  This provides support for the Intel I82092AA PCI-to-PCMCIA bridge device,
 	  found in some older laptops and more commonly in evaluation boards for the
@@ -289,6 +292,9 @@ config ELECTRA_CF
 	  Say Y here to support the CompactFlash controller on the
 	  PA Semi Electra eval board.
 
+config PCCARD_PCI
+	bool
+
 config PCCARD_NONSTATIC
 	bool
 
diff --git a/drivers/pcmcia/Makefile b/drivers/pcmcia/Makefile
index 27e94b3..f1a7ca0 100644
--- a/drivers/pcmcia/Makefile
+++ b/drivers/pcmcia/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_PCMCIA)				+= pcmcia.o
 pcmcia_rsrc-y					+= rsrc_mgr.o
 pcmcia_rsrc-$(CONFIG_PCCARD_NONSTATIC)		+= rsrc_nonstatic.o
 pcmcia_rsrc-$(CONFIG_PCCARD_IODYN)		+= rsrc_iodyn.o
+pcmcia_rsrc-$(CONFIG_PCCARD_PCI)		+= rsrc_pci.o
 obj-$(CONFIG_PCCARD)				+= pcmcia_rsrc.o
 
 
diff --git a/drivers/pcmcia/rsrc_pci.c b/drivers/pcmcia/rsrc_pci.c
new file mode 100644
index 0000000..8934d3c
--- /dev/null
+++ b/drivers/pcmcia/rsrc_pci.c
@@ -0,0 +1,172 @@
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+#include <pcmcia/ss.h>
+#include <pcmcia/cistpl.h>
+#include "cs_internal.h"
+
+
+struct pcmcia_align_data {
+	unsigned long	mask;
+	unsigned long	offset;
+};
+
+static resource_size_t pcmcia_align(void *align_data,
+				const struct resource *res,
+				resource_size_t size, resource_size_t align)
+{
+	struct pcmcia_align_data *data = align_data;
+	resource_size_t start;
+
+	start = (res->start & ~data->mask) + data->offset;
+	if (start < res->start)
+		start += data->mask + 1;
+	return start;
+}
+
+static struct resource *find_io_region(struct pcmcia_socket *s,
+					unsigned long base, int num,
+					unsigned long align)
+{
+	struct resource *res = pcmcia_make_resource(0, num, IORESOURCE_IO,
+						dev_name(&s->dev));
+	struct pcmcia_align_data data;
+	int ret;
+
+	data.mask = align - 1;
+	data.offset = base & data.mask;
+
+	ret = pci_bus_alloc_resource(s->cb_dev->bus, res, num, 1,
+					     base, 0, pcmcia_align, &data);
+	if (ret != 0) {
+		kfree(res);
+		res = NULL;
+	}
+	return res;
+}
+
+static int res_pci_find_io(struct pcmcia_socket *s, unsigned int attr,
+			unsigned int *base, unsigned int num,
+			unsigned int align, struct resource **parent)
+{
+	int i, ret = 0;
+
+	/* Check for an already-allocated window that must conflict with
+	 * what was asked for.  It is a hack because it does not catch all
+	 * potential conflicts, just the most obvious ones.
+	 */
+	for (i = 0; i < MAX_IO_WIN; i++) {
+		if (!s->io[i].res)
+			continue;
+
+		if (!*base)
+			continue;
+
+		if ((s->io[i].res->start & (align-1)) == *base)
+			return -EBUSY;
+	}
+
+	for (i = 0; i < MAX_IO_WIN; i++) {
+		struct resource *res = s->io[i].res;
+		unsigned int try;
+
+		if (res && (res->flags & IORESOURCE_BITS) !=
+			(attr & IORESOURCE_BITS))
+			continue;
+
+		if (!res) {
+			if (align == 0)
+				align = 0x10000;
+
+			res = s->io[i].res = find_io_region(s, *base, num,
+								align);
+			if (!res)
+				return -EINVAL;
+
+			*base = res->start;
+			s->io[i].res->flags =
+				((res->flags & ~IORESOURCE_BITS) |
+					(attr & IORESOURCE_BITS));
+			s->io[i].InUse = num;
+			*parent = res;
+			return 0;
+		}
+
+		/* Try to extend top of window */
+		try = res->end + 1;
+		if ((*base == 0) || (*base == try)) {
+			ret = adjust_resource(s->io[i].res, res->start,
+					      resource_size(res) + num);
+			if (ret)
+				continue;
+			*base = try;
+			s->io[i].InUse += num;
+			*parent = res;
+			return 0;
+		}
+
+		/* Try to extend bottom of window */
+		try = res->start - num;
+		if ((*base == 0) || (*base == try)) {
+			ret = adjust_resource(s->io[i].res,
+					      res->start - num,
+					      resource_size(res) + num);
+			if (ret)
+				continue;
+			*base = try;
+			s->io[i].InUse += num;
+			*parent = res;
+			return 0;
+		}
+	}
+	return -EINVAL;
+}
+
+static struct resource *res_pci_find_mem(u_long base, u_long num,
+		u_long align, int low, struct pcmcia_socket *s)
+{
+	struct resource *res = pcmcia_make_resource(0, num, IORESOURCE_MEM,
+						dev_name(&s->dev));
+	struct pcmcia_align_data data;
+	unsigned long min;
+	int ret;
+
+	if (align < 0x20000)
+		align = 0x20000;
+	data.mask = align - 1;
+	data.offset = base & data.mask;
+
+	min = 0;
+	if (!low)
+		min = 0x100000UL;
+
+	ret = pci_bus_alloc_resource(s->cb_dev->bus,
+			res, num, 1, min, 0,
+			pcmcia_align, &data);
+
+	if (ret != 0) {
+		kfree(res);
+		res = NULL;
+	}
+	return res;
+}
+
+
+static int res_pci_init(struct pcmcia_socket *s)
+{
+	if (!s->cb_dev || (!s->features & SS_CAP_PAGE_REGS)) {
+		dev_err(&s->dev, "not supported by res_pci\n");
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+struct pccard_resource_ops pccard_nonstatic_ops = {
+	.validate_mem = NULL,
+	.find_io = res_pci_find_io,
+	.find_mem = res_pci_find_mem,
+	.init = res_pci_init,
+	.exit = NULL,
+};
+EXPORT_SYMBOL(pccard_nonstatic_ops);


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

* Re: [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems
  2014-12-04 21:31 ` [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems Alan Cox
@ 2015-05-30 14:40   ` Dominik Brodowski
  2015-06-01 13:59     ` Alan Cox
  0 siblings, 1 reply; 11+ messages in thread
From: Dominik Brodowski @ 2015-05-30 14:40 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel, linux-pcmica

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

Alan,

On Thu, Dec 04, 2014 at 09:31:22PM +0000, Alan Cox wrote:
> On a pure PCI platform we don't actually need all the complexity of the
> rsrc_nonstatic manager, in fact we can just work directly with the pci
> allocators and avoid all the complexity (and code bloat).

I know you're re-working this thing by now, but still:

Can we be certain that BIOS, ACPI etc. properly report all io resources
which must not be utilized by other devices? Does this really depend on ISA
being set in Kconfig? Should this also be enabled for CardBus bridges on the
root PCI bus? And: could doing a check for X86 like in rsrc_nonstatic.c
( avoid anything < 0x100 for ioports ) help to avoid some of the possible
fallout?

Best,

	Dominik

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

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

* Re: [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems
  2015-05-30 14:40   ` Dominik Brodowski
@ 2015-06-01 13:59     ` Alan Cox
  0 siblings, 0 replies; 11+ messages in thread
From: Alan Cox @ 2015-06-01 13:59 UTC (permalink / raw)
  To: Dominik Brodowski; +Cc: linux-kernel, linux-pcmica

On Sat, 2015-05-30 at 16:40 +0200, Dominik Brodowski wrote:
> Alan,
> 
> On Thu, Dec 04, 2014 at 09:31:22PM +0000, Alan Cox wrote:
> > On a pure PCI platform we don't actually need all the complexity of the
> > rsrc_nonstatic manager, in fact we can just work directly with the pci
> > allocators and avoid all the complexity (and code bloat).
> 
> I know you're re-working this thing by now, but still:

It's on the todo list to finish debugging

> Can we be certain that BIOS, ACPI etc. properly report all io resources
> which must not be utilized by other devices? Does this really depend on ISA

No you can't. However there appears to be a convention that for mmio the
windows are aligned on largish boundaries and vendors only hide hardware
so that it's next to existing resources on an alignment such that it
won't get allocated.

I've no idea if it's in a spec anywhere or that's just a "Hey it works
in Windows" bit of history.

> being set in Kconfig? Should this also be enabled for CardBus bridges on the
> root PCI bus? And: could doing a check for X86 like in rsrc_nonstatic.c
> ( avoid anything < 0x100 for ioports ) help to avoid some of the possible
> fallout?

Probably yes.

Added to the TODO list for it.

Alan



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

* Re: [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS
  2014-12-04 21:30 ` [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS Alan Cox
@ 2015-06-14 19:52   ` Dominik Brodowski
  2015-06-15 12:10     ` Alan Cox
  0 siblings, 1 reply; 11+ messages in thread
From: Dominik Brodowski @ 2015-06-14 19:52 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel, linux-pcmica

[-- 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 --]

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

* Re: [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS
  2015-06-14 19:52   ` Dominik Brodowski
@ 2015-06-15 12:10     ` Alan Cox
  2015-06-16  6:19       ` Dominik Brodowski
  0 siblings, 1 reply; 11+ messages in thread
From: Alan Cox @ 2015-06-15 12:10 UTC (permalink / raw)
  To: Dominik Brodowski; +Cc: linux-kernel, linux-pcmica

> 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():

I'm not sure it's the right way to do readable() but we seem to be stuck
with that anyway.

The change looks good to me, and I will try and test it soon but it may
take some time due to various other things going on in life. Can you
submit it for 4.2 anyway and give it a bit of time for any screaming
then push to stable ? The number of people using anonymous cards is
pretty small these days so I think it's better to fix the regression you
have and worry about anonymous cards as a secondary thing.

Alan




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

* Re: [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS
  2015-06-15 12:10     ` Alan Cox
@ 2015-06-16  6:19       ` Dominik Brodowski
  0 siblings, 0 replies; 11+ messages in thread
From: Dominik Brodowski @ 2015-06-16  6:19 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel, linux-pcmica

On Mon, Jun 15, 2015 at 01:10:55PM +0100, Alan Cox wrote:
> > 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():
> 
> I'm not sure it's the right way to do readable() but we seem to be stuck
> with that anyway.

Indeed - any replacement would need much testing at least.
> 
> The change looks good to me, and I will try and test it soon but it may
> take some time due to various other things going on in life. Can you
> submit it for 4.2 anyway and give it a bit of time for any screaming
> then push to stable ?

Have added it to my tree, and will do as you suggest.

Thanks,

	Dominik

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

end of thread, other threads:[~2015-06-16  6:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-04 21:28 [PATCH 0/5] pcmcia: 64bit fixes, anonymous cards, other bugs Alan Cox
2014-12-04 21:28 ` [PATCH 1/5] pcmcia: correct types Alan Cox
2014-12-04 21:29 ` [PATCH 2/5] pcmcia cis: on an out of range CIS read return 0xff, don't just warn Alan Cox
2014-12-04 21:30 ` [PATCH 3/5] pcmcia: Fix requery Alan Cox
2014-12-04 21:30 ` [PATCH 4/5] pcmcia: handle anonymous cards by generating a fake CIS Alan Cox
2015-06-14 19:52   ` Dominik Brodowski
2015-06-15 12:10     ` Alan Cox
2015-06-16  6:19       ` Dominik Brodowski
2014-12-04 21:31 ` [PATCH 5/5] pcmcia: add a new resource manager for non ISA systems Alan Cox
2015-05-30 14:40   ` Dominik Brodowski
2015-06-01 13:59     ` Alan Cox

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.