diff mbox

[RFC] add SPARC support to i2c-ali1535

Message ID 4DF75B3D.9090206@geomatys.fr
State RFC
Delegated to: David Miller
Headers show

Commit Message

corentin.labbe June 14, 2011, 12:59 p.m. UTC
Hello

This patch enable i2c-ali1535 to works under SPARC architecture.

I wait for your comments/approval before sending it to the lm_sensors mailing lists.

Thanks in advance
Bests regards,

LABBE Corentin

Comments

David Miller July 6, 2011, 3:04 p.m. UTC | #1
From: "corentin.labbe" <corentin.labbe@geomatys.fr>
Date: Tue, 14 Jun 2011 14:59:41 +0200

> This patch enable i2c-ali1535 to works under SPARC architecture.
> 
> I wait for your comments/approval before sending it to the lm_sensors mailing lists.

Implementing this as a PCI driver doesn't make any sense.

It's be better to probe and fetch the register information
by using the openfirmware tree driver interfaces,
a table of "struct of_device_id", a "platform_driver" that
points to that of_device_id table, and a call to
platform_device_register() to register that platform_driver.

Also, even if you attach the I2C controller, you won't be
able to enumerate the underlying I2C devices properly.
The openfirmware device tree will need to be used in order
to do that as well.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
corentin.labbe Aug. 25, 2011, 9:32 a.m. UTC | #2
Le 06/07/2011 17:04, David Miller a écrit :
> From: "corentin.labbe" <corentin.labbe@geomatys.fr>
> Date: Tue, 14 Jun 2011 14:59:41 +0200
> 
>> This patch enable i2c-ali1535 to works under SPARC architecture.
>>
>> I wait for your comments/approval before sending it to the lm_sensors mailing lists.
> 
> Implementing this as a PCI driver doesn't make any sense.
> 
> It's be better to probe and fetch the register information
> by using the openfirmware tree driver interfaces,
> a table of "struct of_device_id", a "platform_driver" that
> points to that of_device_id table, and a call to
> platform_device_register() to register that platform_driver.
> 
> Also, even if you attach the I2C controller, you won't be
> able to enumerate the underlying I2C devices properly.
> The openfirmware device tree will need to be used in order
> to do that as well.

Hello

It seems that you respond to my first patch and I have sent a second
patch (much simpler and I resend it with this mail for simplicity).

I agree that this driver could be ported to a platform driver but for
the moment my simple goal is to make it work under SPARC.
In a second time, it could be transformed to a platform driver.

Sorry for this very late responce.

Best regards

Signed-off-by: LABBE Corentin <corentin.labbe@geomatys.fr>
---
--- linux/drivers/i2c/busses/i2c-ali1535.c.orig	2011-06-15
18:02:56.000000000 +0200
+++ linux/drivers/i2c/busses/i2c-ali1535.c	2011-06-15 18:05:26.000000000
+0200
@@ -132,7 +132,7 @@
 #define	ALI1535_SMBIO_EN	0x04	/* SMB I/O Space enable		*/

 static struct pci_driver ali1535_driver;
-static unsigned short ali1535_smba;
+static unsigned long ali1535_smba;

 /* Detect whether a ALI1535 can be found, and initialize it, where
necessary.
    Note the differences between kernels with the old PCI BIOS interface and
@@ -142,6 +142,7 @@ static int __devinit ali1535_setup(struc
 {
 	int retval = -ENODEV;
 	unsigned char temp;
+	unsigned short offset;

 	/* Check the following things:
 		- SMB I/O address is initialized
@@ -149,15 +150,27 @@ static int __devinit ali1535_setup(struc
 		- We can use the addresses
 	*/

+	if (pci_enable_device(dev)) {
+		dev_err(&dev->dev, "ALI1535_smb cant enable device");
+		goto exit;
+	}
+
 	/* Determine the address of the SMBus area */
-	pci_read_config_word(dev, SMBBA, &ali1535_smba);
-	ali1535_smba &= (0xffff & ~(ALI1535_SMB_IOSIZE - 1));
-	if (ali1535_smba == 0) {
+	pci_read_config_word(dev, SMBBA, &offset);
+	dev_dbg(&dev->dev, "ALI1535_smb is at offset 0x%04x", offset);
+	offset &= (0xffff & ~(ALI1535_SMB_IOSIZE - 1));
+	if (offset == 0) {
 		dev_warn(&dev->dev,
 			"ALI1535_smb region uninitialized - upgrade BIOS?\n");
 		goto exit;
 	}

+	ali1535_smba = pci_resource_start(dev, 0) + offset;
+	if ((pci_resource_flags(dev, 0) & IORESOURCE_IO) == 0) {
+		dev_err(&dev->dev, "ALI1535_smb bar 0 is not IORESOURCE_IO");
+		goto exit;
+	}
+
 	retval = acpi_check_region(ali1535_smba, ALI1535_SMB_IOSIZE,
 				   ali1535_driver.name);
 	if (retval)
@@ -165,7 +178,7 @@ static int __devinit ali1535_setup(struc

 	if (!request_region(ali1535_smba, ALI1535_SMB_IOSIZE,
 			    ali1535_driver.name)) {
-		dev_err(&dev->dev, "ALI1535_smb region 0x%x already in use!\n",
+		dev_err(&dev->dev, "ALI1535_smb region 0x%lx already in use!\n",
 			ali1535_smba);
 		goto exit;
 	}
@@ -196,7 +209,7 @@ static int __devinit ali1535_setup(struc
 	*/
 	pci_read_config_byte(dev, SMBREV, &temp);
 	dev_dbg(&dev->dev, "SMBREV = 0x%X\n", temp);
-	dev_dbg(&dev->dev, "ALI1535_smba = 0x%X\n", ali1535_smba);
+	dev_dbg(&dev->dev, "ALI1535_smba = 0x%0lx\n", ali1535_smba);

 	retval = 0;
 exit:
@@ -499,7 +512,7 @@ static int __devinit ali1535_probe(struc
 	ali1535_adapter.dev.parent = &dev->dev;

 	snprintf(ali1535_adapter.name, sizeof(ali1535_adapter.name),
-		"SMBus ALI1535 adapter at %04x", ali1535_smba);
+		"SMBus ALI1535 adapter at %0lx", ali1535_smba);
 	return i2c_add_adapter(&ali1535_adapter);
 }


--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Nov. 20, 2011, 10:35 p.m. UTC | #3
From: "corentin.labbe" <corentin.labbe@geomatys.fr>
Date: Thu, 25 Aug 2011 11:32:55 +0200

> Signed-off-by: LABBE Corentin <corentin.labbe@geomatys.fr>
> ---
> --- linux/drivers/i2c/busses/i2c-ali1535.c.orig	2011-06-15
> 18:02:56.000000000 +0200
> +++ linux/drivers/i2c/busses/i2c-ali1535.c	2011-06-15 18:05:26.000000000
> +0200
> @@ -132,7 +132,7 @@

This seems like a pretty reasonable patch.  But could you please repost it in a way
such that your email client doesn't break up long lines and otherwise corrupt
the patch as it has done here?

Please make a fresh, formal, new posting of the patch.  Complete with a sufficiently
detailed commit log message and appropriate Subject line.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- /root/linux-2.6.38/drivers/i2c/busses/i2c-ali1535.c	2011-03-15 02:20:32.000000000 +0100
+++ drivers/i2c/busses/i2c-ali1535.c	2011-06-10 15:08:49.000000000 +0200
@@ -132,7 +132,60 @@ 
 #define	ALI1535_SMBIO_EN	0x04	/* SMB I/O Space enable		*/
 
 static struct pci_driver ali1535_driver;
+#ifdef CONFIG_SPARC
+static unsigned long ali1535_smba;
+#else
 static unsigned short ali1535_smba;
+#endif
+
+#ifdef CONFIG_SPARC
+static unsigned long ali1535_get_ioport_base_addr(struct pci_dev *dev)
+{
+	struct device_node *of_node, *of_parent_node;
+	const struct linux_prom_pci_ranges *pbm_ranges;
+	int i, type;
+	u32 parent_phys_hi, parent_phys_lo;
+	unsigned long a = 0;
+	const struct linux_prom_pci_ranges *pr;
+	int num_pbm_ranges;
+
+	/* I do the same thing that pci_determine_mem_io_space()
+	 * in arch/sparc/kernel/pci_common.c
+	 * I get the parent pci bus as an of_node and get ioport base address*/
+	of_node = pci_device_to_OF_node(dev);
+	if (of_node == NULL) {
+		dev_err(&dev->dev, "ALI1535_smb cant get OF_node from pci_device\n");
+		return 0;
+	}
+	of_parent_node = of_get_parent(of_node);
+	if (of_parent_node == NULL) {
+		dev_err(&dev->dev, "ALI1535_smb cant get parent pcibus from OF\n");
+		return 0;
+	}
+	pbm_ranges = of_get_property(of_parent_node, "ranges", &i);
+	if (!pbm_ranges) {
+		dev_err(&dev->dev, "ALI1535_smb cant get ranges from OF\n");
+		return 0;
+	}
+	num_pbm_ranges = i / sizeof(*pbm_ranges);
+	for (i = 0; i < num_pbm_ranges; i++) {
+		pr = &pbm_ranges[i];
+		type = (pr->child_phys_hi >> 24) & 0x3;
+		if (type == 1) {
+			parent_phys_hi = pr->parent_phys_hi;
+			parent_phys_lo = pr->parent_phys_lo;
+			if (tlb_type == hypervisor)
+				parent_phys_hi &= 0x0fffffff;
+			a = (((unsigned long)parent_phys_hi << 32UL) |
+				((unsigned long)parent_phys_lo << 0UL));
+			dev_info(&dev->dev, "ALI1535_smb found IOstart at 0x%lx\n", a);
+		}
+	}
+	if (a == 0)
+		dev_err(&dev->dev, "ALI1535_smb error iobase is 0\n");
+	return a;
+}
+#endif
 
 /* Detect whether a ALI1535 can be found, and initialize it, where necessary.
    Note the differences between kernels with the old PCI BIOS interface and
@@ -142,6 +195,7 @@ 
 {
 	int retval = -ENODEV;
 	unsigned char temp;
+	unsigned short offset;
 
 	/* Check the following things:
 		- SMB I/O address is initialized
@@ -150,13 +204,21 @@ 
 	*/
 
 	/* Determine the address of the SMBus area */
-	pci_read_config_word(dev, SMBBA, &ali1535_smba);
-	ali1535_smba &= (0xffff & ~(ALI1535_SMB_IOSIZE - 1));
-	if (ali1535_smba == 0) {
+	pci_read_config_word(dev, SMBBA, &offset);
+	dev_info(&dev->dev, "ALI1535_smb is at offset 0x%04x", offset);
+	offset &= (0xffff & ~(ALI1535_SMB_IOSIZE - 1));
+	if (offset == 0) {
 		dev_warn(&dev->dev,
 			"ALI1535_smb region uninitialized - upgrade BIOS?\n");
 		goto exit;
 	}
+	ali1535_smba = 0;
+#ifdef CONFIG_SPARC
+	ali1535_smba = ali1535_get_ioport_base_addr(dev);
+	if (ali1535_smba == 0)
+		return -ENODEV;
+#endif
+	ali1535_smba += offset;
 
 	retval = acpi_check_region(ali1535_smba, ALI1535_SMB_IOSIZE,
 				   ali1535_driver.name);
@@ -165,8 +227,13 @@ 
 
 	if (!request_region(ali1535_smba, ALI1535_SMB_IOSIZE,
 			    ali1535_driver.name)) {
+#ifdef CONFIG_SPARC
+		dev_err(&dev->dev, "ALI1535_smb region 0x%lx already in use!\n",
+			ali1535_smba);
+#else
 		dev_err(&dev->dev, "ALI1535_smb region 0x%x already in use!\n",
 			ali1535_smba);
+#endif
 		goto exit;
 	}
 
@@ -196,7 +263,6 @@ 
 	*/
 	pci_read_config_byte(dev, SMBREV, &temp);
 	dev_dbg(&dev->dev, "SMBREV = 0x%X\n", temp);
-	dev_dbg(&dev->dev, "ALI1535_smba = 0x%X\n", ali1535_smba);
 
 	retval = 0;
 exit:
@@ -498,8 +564,13 @@ 
 	/* set up the sysfs linkage to our parent device */
 	ali1535_adapter.dev.parent = &dev->dev;
 
+#ifdef CONFIG_SPARC
+	snprintf(ali1535_adapter.name, sizeof(ali1535_adapter.name),
+		"SMBus ALI1535 adapter at %lx", ali1535_smba);
+#else
 	snprintf(ali1535_adapter.name, sizeof(ali1535_adapter.name),
 		"SMBus ALI1535 adapter at %04x", ali1535_smba);
+#endif
 	return i2c_add_adapter(&ali1535_adapter);
 }