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"
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 <[email protected]>
---
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);
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 <[email protected]>
---
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) {
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 <[email protected]>
---
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;
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 <[email protected]>
---
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)
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 <[email protected]>
---
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);
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
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: <[email protected]> # for v4.0 and v4.1
Signed-off-by: Dominik Brodowski <[email protected]>
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))
> 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
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