Re: [Socketcan-users] Using sja1000_isa on a 64b system

[Wolfgang Grandegger <wg@grandegger.com>]

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

On 11/13/2011 08:31 PM, Wolfgang Grandegger wrote:
> On 11/13/2011 10:15 AM, Willy Lambert wrote:
>> 2011/11/13 Willy Lambert <lambert.willy@gmail.com>:
>>> 2011/11/12 Wolfgang Grandegger <wg@grandegger.com>:
>>>> On 11/12/2011 10:08 PM, Wolfgang Grandegger wrote:
>>>>> On 11/12/2011 10:55 AM, Willy Lambert wrote:
>>>> ...
>>>>>>> I failed to apply the patch, is it for 2.6.38.8 kernels ?
>>>>>
>>>>> No, it's for up-to-date *mainline* linux version 3.1.x. Any chance to
>>>>> switch to David Miller's "net-nex-2.6" tree?. Otherwise I'm going to
>>>>> adapt the patch to 2.6.38.8.
>>>>
>>>> Well, the patch also applies to 2.6.38.8. Maybe you have some problems
>>>> extracting the patch. Therefore I have attached it as attachment this
>>>> time. Good luck.
>>>
>>> It's ok now :
>>> ard@ard-host:~/src/linux-2.6.38.8$ patch -p1 < wg_platform.patch
>>> patching file drivers/net/can/sja1000/Kconfig
>>> patching file drivers/net/can/sja1000/sja1000_isa.c
>>>
>>> I must have done something wrong. I am going to test it now
>>>
>>
>> arg, it doesn't compile I had a look a sources but I don't see the error :
>>
>>   CC [M]  drivers/xenomai/can/rtcan_raw_dev.o
>> drivers/net/can/sja1000/sja1000_isa.c: In function ‘sja1000_isa_init’:
>> drivers/net/can/sja1000/sja1000_isa.c:297: error: invalid storage
>> class for function ‘sja1000_isa_exit’
>> drivers/net/can/sja1000/sja1000_isa.c:296: warning: ISO C90 forbids
>> mixed declarations and code
>> drivers/net/can/sja1000/sja1000_isa.c:307: error: invalid storage
>> class for function ‘__inittest’
>> drivers/net/can/sja1000/sja1000_isa.c:307: warning: ‘alias’ attribute ignored
>> drivers/net/can/sja1000/sja1000_isa.c:308: error: invalid storage
>> class for function ‘__exittest’
>> drivers/net/can/sja1000/sja1000_isa.c:308: warning: ISO C90 forbids
>> mixed declarations and code
>> drivers/net/can/sja1000/sja1000_isa.c:308: warning: ‘alias’ attribute ignored
>> drivers/net/can/sja1000/sja1000_isa.c:308: error: expected declaration
>> or statement at end of input
>> make[5]: *** [drivers/net/can/sja1000/sja1000_isa.o] Error 1
> 
> Argh, a "}" is missing, sorry. I will test the patch more carefully
> before re-sending.

See attachment.

Wolfgang.

[-- Attachment #2: sja1000-isa-platform-x86-64.patch --]
[-- Type: text/x-diff, Size: 7240 bytes --]

[PATCH] can: sja1000_isa: convert to platform driver to support x86_64 systems

This driver is currently not supported on x86_64 systems because the
"isa_driver" interface is used. To overcome this limitation, this
driver is converted to a platform driver, similar to the serial 8250
driver.

This patch also fixes some warnings: "comparison is always false due
to limited range of data type".

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/sja1000/Kconfig       |    1 
 drivers/net/can/sja1000/sja1000_isa.c |  109 ++++++++++++++++++++++------------
 2 files changed, 72 insertions(+), 38 deletions(-)

Index: linux-2.6.38.8/drivers/net/can/sja1000/Kconfig
===================================================================
--- linux-2.6.38.8.orig/drivers/net/can/sja1000/Kconfig	2011-11-13 20:24:52.447054533 +0100
+++ linux-2.6.38.8/drivers/net/can/sja1000/Kconfig	2011-11-13 20:24:55.943071866 +0100
@@ -6,7 +6,6 @@
 
 config CAN_SJA1000_ISA
 	tristate "ISA Bus based legacy SJA1000 driver"
-	depends on ISA
 	---help---
 	  This driver adds legacy support for SJA1000 chips connected to
 	  the ISA bus using I/O port, memory mapped or indirect access.
Index: linux-2.6.38.8/drivers/net/can/sja1000/sja1000_isa.c
===================================================================
--- linux-2.6.38.8.orig/drivers/net/can/sja1000/sja1000_isa.c	2011-11-13 20:24:52.431054451 +0100
+++ linux-2.6.38.8/drivers/net/can/sja1000/sja1000_isa.c	2011-11-14 10:44:50.026339684 +0100
@@ -17,7 +17,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/isa.h>
+#include <linux/platform_device.h>
 #include <linux/interrupt.h>
 #include <linux/netdevice.h>
 #include <linux/delay.h>
@@ -44,9 +44,9 @@
 static unsigned long mem[MAXDEV];
 static int __devinitdata irq[MAXDEV];
 static int __devinitdata clk[MAXDEV];
-static char __devinitdata cdr[MAXDEV] = {[0 ... (MAXDEV - 1)] = -1};
-static char __devinitdata ocr[MAXDEV] = {[0 ... (MAXDEV - 1)] = -1};
-static char __devinitdata indirect[MAXDEV] = {[0 ... (MAXDEV - 1)] = -1};
+static unsigned char __devinitdata cdr[MAXDEV] = {[0 ... (MAXDEV - 1)] = 0xff};
+static unsigned char __devinitdata ocr[MAXDEV] = {[0 ... (MAXDEV - 1)] = 0xff};
+static int __devinitdata indirect[MAXDEV] = {[0 ... (MAXDEV - 1)] = -1};
 
 module_param_array(port, ulong, NULL, S_IRUGO);
 MODULE_PARM_DESC(port, "I/O port number");
@@ -54,7 +54,7 @@
 module_param_array(mem, ulong, NULL, S_IRUGO);
 MODULE_PARM_DESC(mem, "I/O memory address");
 
-module_param_array(indirect, byte, NULL, S_IRUGO);
+module_param_array(indirect, int, NULL, S_IRUGO);
 MODULE_PARM_DESC(indirect, "Indirect access via address and data port");
 
 module_param_array(irq, int, NULL, S_IRUGO);
@@ -75,6 +75,8 @@
 #define SJA1000_IOSIZE          0x20
 #define SJA1000_IOSIZE_INDIRECT 0x02
 
+static struct platform_device *sja1000_isa_devs[MAXDEV];
+
 static u8 sja1000_isa_mem_read_reg(const struct sja1000_priv *priv, int reg)
 {
 	return readb(priv->reg_base + reg);
@@ -115,26 +117,18 @@
 	outb(val, base + 1);
 }
 
-static int __devinit sja1000_isa_match(struct device *pdev, unsigned int idx)
-{
-	if (port[idx] || mem[idx]) {
-		if (irq[idx])
-			return 1;
-	} else if (idx)
-		return 0;
-
-	dev_err(pdev, "insufficient parameters supplied\n");
-	return 0;
-}
-
-static int __devinit sja1000_isa_probe(struct device *pdev, unsigned int idx)
+static int __devinit sja1000_isa_probe(struct platform_device *pdev)
 {
 	struct net_device *dev;
 	struct sja1000_priv *priv;
 	void __iomem *base = NULL;
 	int iosize = SJA1000_IOSIZE;
+	int idx = pdev->id;
 	int err;
 
+	dev_dbg(&pdev->dev, "probing idx=%d: port=%#lx, mem=%#lx, irq=%d\n",
+		idx, port[idx], mem[idx], irq[idx]);
+
 	if (mem[idx]) {
 		if (!request_mem_region(mem[idx], iosize, DRV_NAME)) {
 			err = -EBUSY;
@@ -189,31 +183,31 @@
 	else
 		priv->can.clock.freq = CLK_DEFAULT / 2;
 
-	if (ocr[idx] != -1)
+	if (ocr[idx] != 0xff)
 		priv->ocr = ocr[idx] & 0xff;
-	else if (ocr[0] != -1)
+	else if (ocr[0] != 0xff)
 		priv->ocr = ocr[0] & 0xff;
 	else
 		priv->ocr = OCR_DEFAULT;
 
-	if (cdr[idx] != -1)
+	if (cdr[idx] != 0xff)
 		priv->cdr = cdr[idx] & 0xff;
-	else if (cdr[0] != -1)
+	else if (cdr[0] != 0xff)
 		priv->cdr = cdr[0] & 0xff;
 	else
 		priv->cdr = CDR_DEFAULT;
 
-	dev_set_drvdata(pdev, dev);
-	SET_NETDEV_DEV(dev, pdev);
+	dev_set_drvdata(&pdev->dev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
 
 	err = register_sja1000dev(dev);
 	if (err) {
-		dev_err(pdev, "registering %s failed (err=%d)\n",
+		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
 			DRV_NAME, err);
 		goto exit_unmap;
 	}
 
-	dev_info(pdev, "%s device registered (reg_base=0x%p, irq=%d)\n",
+	dev_info(&pdev->dev, "%s device registered (reg_base=0x%p, irq=%d)\n",
 		 DRV_NAME, priv->reg_base, dev->irq);
 	return 0;
 
@@ -229,13 +223,14 @@
 	return err;
 }
 
-static int __devexit sja1000_isa_remove(struct device *pdev, unsigned int idx)
+static int __devexit sja1000_isa_remove(struct platform_device *pdev)
 {
-	struct net_device *dev = dev_get_drvdata(pdev);
+	struct net_device *dev = dev_get_drvdata(&pdev->dev);
 	struct sja1000_priv *priv = netdev_priv(dev);
+	int idx = pdev->id;
 
 	unregister_sja1000dev(dev);
-	dev_set_drvdata(pdev, NULL);
+	dev_set_drvdata(&pdev->dev, NULL);
 
 	if (mem[idx]) {
 		iounmap(priv->reg_base);
@@ -251,29 +246,69 @@
 	return 0;
 }
 
-static struct isa_driver sja1000_isa_driver = {
-	.match = sja1000_isa_match,
+static struct platform_driver sja1000_isa_driver = {
 	.probe = sja1000_isa_probe,
 	.remove = __devexit_p(sja1000_isa_remove),
 	.driver = {
 		.name = DRV_NAME,
+		.owner = THIS_MODULE,
 	},
 };
 
 static int __init sja1000_isa_init(void)
 {
-	int err = isa_register_driver(&sja1000_isa_driver, MAXDEV);
+	int idx, err;
+
+	for (idx = 0; idx < MAXDEV; idx++) {
+		if ((port[idx] || mem[idx]) && irq[idx]) {
+			sja1000_isa_devs[idx] =
+				platform_device_alloc(DRV_NAME, idx);
+			if (!sja1000_isa_devs[idx]) {
+				err = -ENOMEM;
+				goto exit_free_devices;
+			}
+			err = platform_device_add(sja1000_isa_devs[idx]);
+			if (err) {
+				platform_device_put(sja1000_isa_devs[idx]);
+				goto exit_free_devices;
+			}
+			pr_debug("%s: platform device %d: port=%#lx, mem=%#lx, "
+				 "irq=%d\n",
+				 DRV_NAME, idx, port[idx], mem[idx], irq[idx]);
+		} else {
+			pr_err("%s: insufficient parameters supplied\n",
+			       DRV_NAME);
+			goto exit_free_devices;
+		}
+	}
+
+	err = platform_driver_register(&sja1000_isa_driver);
+	if (err)
+		goto exit_free_devices;
+
+	pr_info("Legacy %s driver for max. %d devices registered\n",
+		DRV_NAME, MAXDEV);
+
+	return 0;
+
+exit_free_devices:
+	while (--idx >= 0) {
+		if (sja1000_isa_devs[idx])
+			platform_device_unregister(sja1000_isa_devs[idx]);
+	}
 
-	if (!err)
-		printk(KERN_INFO
-		       "Legacy %s driver for max. %d devices registered\n",
-		       DRV_NAME, MAXDEV);
 	return err;
 }
 
 static void __exit sja1000_isa_exit(void)
 {
-	isa_unregister_driver(&sja1000_isa_driver);
+	int idx;
+
+	platform_driver_unregister(&sja1000_isa_driver);
+	for (idx = 0; idx < MAXDEV; idx++) {
+		if (sja1000_isa_devs[idx])
+			platform_device_unregister(sja1000_isa_devs[idx]);
+	}
 }
 
 module_init(sja1000_isa_init);