[patch #2 0/6] [#2] SM501 updates

[patch #2 0/6] [#2] SM501 updates

[ben]

Add documentation missed in the SM501 submission
and fix some minor problems that have been found
whilst testing.

This series has the minor typo in the clock
patch found by Mariusz Kozlowski <m.kozlowski@tuxland.pl>

I hope this series is suitable for application as
bugfixes before the next merge window.

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

[patch #2 1/6] SM501: suspend support

[ben]

[-- Attachment #1: simtec-drivers-mfd-sm501-suspend.patch --]
[-- Type: text/plain, Size: 5281 bytes --]

This patch adds support for suspending the core
(mfd driver) of the SM501.
 
Signed-off-by: Ben Dooks <ben-linux@fluff.org>

Index: linux-2.6.21-quilt8/drivers/mfd/sm501.c
===================================================================
--- linux-2.6.21-quilt8.orig/drivers/mfd/sm501.c	2007-06-04 16:06:29.000000000 +0100
+++ linux-2.6.21-quilt8/drivers/mfd/sm501.c	2007-06-13 13:25:50.000000000 +0100
@@ -41,6 +41,9 @@ struct sm501_devdata {
 	struct resource			*regs_claim;
 	struct sm501_platdata		*platdata;
 
+	unsigned int			 in_suspend;
+	unsigned long			 pm_misc;
+
 	int				 unit_power[20];
 	unsigned int			 pdev_id;
 	unsigned int			 irq;
@@ -169,10 +172,31 @@ x		 "M %ld.%ld (%ld), MX1 %ld.%ld (%ld)\
 		fmt_freq(decode_div(pll2, pm1, 8,  1<<12, 15, misc_div)),
 		fmt_freq(decode_div(pll2, pm1, 0,  1<<4,  15, misc_div)));
 }
-#else
-static void sm501_dump_clk(struct sm501_devdata *sm)
+
+static void sm501_dump_regs(struct sm501_devdata *sm)
+{
+	void __iomem *regs = sm->regs;
+
+	dev_info(sm->dev, "System Control   %08x\n", readl(regs + SM501_SYSTEM_CONTROL));
+	dev_info(sm->dev, "Misc Control     %08x\n", readl(regs + SM501_MISC_CONTROL));
+	dev_info(sm->dev, "GPIO Control Low %08x\n", readl(regs + SM501_GPIO31_0_CONTROL));
+	dev_info(sm->dev, "GPIO Control Hi  %08x\n", readl(regs + SM501_GPIO63_32_CONTROL));
+	dev_info(sm->dev, "DRAM Control     %08x\n", readl(regs + SM501_DRAM_CONTROL));
+	dev_info(sm->dev, "Arbitration Ctrl %08x\n", readl(regs + SM501_ARBTRTN_CONTROL));
+	dev_info(sm->dev, "Misc Timing      %08x\n", readl(regs + SM501_MISC_TIMING));
+}
+
+static void sm501_dump_gate(struct sm501_devdata *sm)
 {
+	dev_info(sm->dev, "CurrentGate      %08x\n", readl(sm->regs + SM501_CURRENT_GATE));
+	dev_info(sm->dev, "CurrentClock     %08x\n", readl(sm->regs + SM501_CURRENT_CLOCK));
+	dev_info(sm->dev, "PowerModeControl %08x\n", readl(sm->regs + SM501_POWER_MODE_CONTROL));
 }
+
+#else
+static inline void sm501_dump_gate(struct sm501_devdata *sm) { }
+static inline void sm501_dump_regs(struct sm501_devdata *sm) { }
+static inline void sm501_dump_clk(struct sm501_devdata *sm) { }
 #endif
 
 /* sm501_sync_regs
@@ -185,9 +209,21 @@ static void sm501_sync_regs(struct sm501
 	readl(sm->regs);
 }
 
+static inline void sm501_mdelay(struct sm501_devdata *sm, unsigned int delay)
+{
+	/* during suspend/resume, we are currently not allowed to sleep,
+	 * so change to using mdelay() instead of msleep() if we
+	 * are in one of these paths */
+
+	if (sm->in_suspend)
+		mdelay(delay);
+	else
+		msleep(delay);
+}
+
 /* sm501_misc_control
  *
- * alters the misceleneous control parameters
+ * alters the miscellaneous control parameters
 */
 
 int sm501_misc_control(struct device *dev,
@@ -368,7 +404,7 @@ int sm501_unit_power(struct device *dev,
 	dev_dbg(sm->dev, "gate %08lx, clock %08lx, mode %08lx\n",
 		gate, clock, mode);
 
-	msleep(16);
+	sm501_mdelay(sm, 16);
 
  already:
 	mutex_unlock(&sm->clock_lock);
@@ -538,7 +574,7 @@ unsigned long sm501_set_clock(struct dev
 	dev_info(sm->dev, "gate %08lx, clock %08lx, mode %08lx\n",
 		 gate, clock, mode);
 
-	msleep(16);
+	sm501_mdelay(sm, 16);
 	mutex_unlock(&sm->clock_lock);
 
 	sm501_dump_clk(sm);
@@ -841,9 +877,7 @@ static int sm501_init_dev(struct sm501_d
 		 sm->regs, readl(sm->regs + SM501_DEVICEID),
 		 (unsigned long)mem_avail >> 20, sm->irq);
 
-	dev_info(sm->dev, "CurrentGate      %08x\n", readl(sm->regs+0x38));
-	dev_info(sm->dev, "CurrentClock     %08x\n", readl(sm->regs+0x3c));
-	dev_info(sm->dev, "PowerModeControl %08x\n", readl(sm->regs+0x54));
+	sm501_dump_gate(sm);
 
 	ret = device_create_file(sm->dev, &dev_attr_dbg_regs);
 	if (ret)
@@ -933,6 +967,53 @@ static int sm501_plat_probe(struct platf
 
 }
 
+/* power management support */
+
+static int sm501_plat_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct sm501_devdata *sm = platform_get_drvdata(pdev);
+
+	sm->in_suspend = 1;
+	sm->pm_misc = readl(sm->regs + SM501_MISC_CONTROL);
+
+	sm501_dump_regs(sm);
+	return 0;
+}
+
+static int sm501_plat_resume(struct platform_device *pdev)
+{
+	struct sm501_devdata *sm = platform_get_drvdata(pdev);
+
+	sm501_dump_regs(sm);
+	sm501_dump_gate(sm);
+	sm501_dump_clk(sm);
+
+	/* check to see if we are in the same state as when suspended */
+
+	if (readl(sm->regs + SM501_MISC_CONTROL) != sm->pm_misc) {
+		dev_info(sm->dev, "SM501_MISC_CONTROL changed over sleep\n");
+		writel(sm->pm_misc, sm->regs + SM501_MISC_CONTROL);
+
+		/* our suspend causes the controller state to change,
+		 * either by something attempting setup, power loss,
+		 * or an external reset event on power change */
+
+		if (sm->platdata && sm->platdata->init) {
+			sm501_init_regs(sm, sm->platdata->init);
+		}
+	}
+
+	/* dump our state from resume */
+
+	sm501_dump_regs(sm);
+	sm501_dump_clk(sm);
+
+	sm->in_suspend = 0;
+
+	return 0;
+}
+
+
 /* Initialisation data for PCI devices */
 
 static struct sm501_initdata sm501_pci_initdata = {
@@ -1126,6 +1207,8 @@ static struct platform_driver sm501_plat
 	},
 	.probe		= sm501_plat_probe,
 	.remove		= sm501_plat_remove,
+	.suspend	= sm501_plat_suspend,
+	.resume		= sm501_plat_resume,
 };
 
 static int __init sm501_base_init(void)

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [patch #2 1/6] SM501: suspend support

[Andrew Morton]

On Mon, 18 Jun 2007 21:56:21 +0100
ben@fluff.org.uk wrote:

> +/* power management support */
> +
> +static int sm501_plat_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	struct sm501_devdata *sm = platform_get_drvdata(pdev);
> +
> +	sm->in_suspend = 1;
> +	sm->pm_misc = readl(sm->regs + SM501_MISC_CONTROL);
> +
> +	sm501_dump_regs(sm);
> +	return 0;
> +}
> +
> +static int sm501_plat_resume(struct platform_device *pdev)
> +{
> +	struct sm501_devdata *sm = platform_get_drvdata(pdev);
> +
> +	sm501_dump_regs(sm);
> +	sm501_dump_gate(sm);
> +	sm501_dump_clk(sm);
> +
> +	/* check to see if we are in the same state as when suspended */
> +
> +	if (readl(sm->regs + SM501_MISC_CONTROL) != sm->pm_misc) {
> +		dev_info(sm->dev, "SM501_MISC_CONTROL changed over sleep\n");
> +		writel(sm->pm_misc, sm->regs + SM501_MISC_CONTROL);
> +
> +		/* our suspend causes the controller state to change,
> +		 * either by something attempting setup, power loss,
> +		 * or an external reset event on power change */
> +
> +		if (sm->platdata && sm->platdata->init) {
> +			sm501_init_regs(sm, sm->platdata->init);
> +		}
> +	}
> +
> +	/* dump our state from resume */
> +
> +	sm501_dump_regs(sm);
> +	sm501_dump_clk(sm);
> +
> +	sm->in_suspend = 0;
> +
> +	return 0;
> +}
> +
> +
>  /* Initialisation data for PCI devices */
>  
>  static struct sm501_initdata sm501_pci_initdata = {
> @@ -1126,6 +1207,8 @@ static struct platform_driver sm501_plat
>  	},
>  	.probe		= sm501_plat_probe,
>  	.remove		= sm501_plat_remove,
> +	.suspend	= sm501_plat_suspend,
> +	.resume		= sm501_plat_resume,
>  };

Do we want to compile this in if CONFIG_PM=n?

[patch #2 2/6] SM501: initialise SDRAM clock before bus clocks

[ben]

[-- Attachment #1: simtec-drivers-mfd-sm501-clock-order.patch --]
[-- Type: text/plain, Size: 1299 bytes --]

This init sequence of setting the SDRAM clock before the
bus clock is recommend by Silicon Motion to stop problems
with writes not sticking into registers.

Signed-off-by: Vincent Sanders <vince@simtec.co.uk>
Signed-off-by: Ben Dooks <ben-linux@fluff.org>

Index: linux-2.6.21-quilt8/drivers/mfd/sm501.c
===================================================================
--- linux-2.6.21-quilt8.orig/drivers/mfd/sm501.c	2007-06-07 12:12:38.000000000 +0100
+++ linux-2.6.21-quilt8/drivers/mfd/sm501.c	2007-06-07 12:13:03.000000000 +0100
@@ -833,15 +833,15 @@ static void sm501_init_regs(struct sm501
 	sm501_init_reg(sm, SM501_GPIO31_0_CONTROL, &init->gpio_low);
 	sm501_init_reg(sm, SM501_GPIO63_32_CONTROL, &init->gpio_high);
 
-	if (init->mclk) {
-		dev_info(sm->dev, "setting MCLK to %ld\n", init->mclk);
-		sm501_set_clock(sm->dev, SM501_CLOCK_MCLK, init->mclk);
-	}
-
 	if (init->m1xclk) {
 		dev_info(sm->dev, "setting M1XCLK to %ld\n", init->m1xclk);
 		sm501_set_clock(sm->dev, SM501_CLOCK_M1XCLK, init->m1xclk);
 	}
+
+	if (init->mclk) {
+		dev_info(sm->dev, "setting MCLK to %ld\n", init->mclk);
+		sm501_set_clock(sm->dev, SM501_CLOCK_MCLK, init->mclk);
+	}
 }
 
 static unsigned int sm501_mem_local[] = {

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

[patch #2 3/6] SM501: Fix sm501_init_reg() mask/set order

[ben]

[-- Attachment #1: simtec-drivers-mfd-sm501-reginitfix.patch --]
[-- Type: text/plain, Size: 1104 bytes --]

The order of the set and mask operation in sm501_init_reg()
was setting and then masking the bits set. Correct the order
so that we do not end up with 288MHz SDRAM clocks on certain
systems.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>

Index: linux-2.6.21-quilt8/drivers/mfd/sm501.c
===================================================================
--- linux-2.6.21-quilt8.orig/drivers/mfd/sm501.c	2007-06-07 12:13:03.000000000 +0100
+++ linux-2.6.21-quilt8/drivers/mfd/sm501.c	2007-06-07 12:13:06.000000000 +0100
@@ -803,6 +803,9 @@ static DEVICE_ATTR(dbg_regs, 0666, sm501
 /* sm501_init_reg
  *
  * Helper function for the init code to setup a register
+ *
+ * clear the bits which are set in r->mask, and then set
+ * the bits set in r->set.
 */
 
 static inline void sm501_init_reg(struct sm501_devdata *sm,
@@ -812,8 +815,8 @@ static inline void sm501_init_reg(struct
 	unsigned long tmp;
 
 	tmp = readl(sm->regs + reg);
-	tmp |= r->set;
 	tmp &= ~r->mask;
+	tmp |= r->set;
 	writel(tmp, sm->regs + reg);
 }
 

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

[patch #2 4/6] SM501: Clock updates and checks

[ben]

[-- Attachment #1: simtec-drivers-mfd-sm501-clockupdate.patch --]
[-- Type: text/plain, Size: 2811 bytes --]

Ensure that the M1XCLK and MCLK are sourced from
the same PLL (and refuse to bind the driver if they
are not).

Update the PCI to safe initialisation values, as 72MHz
is the maximum clock for 33MHz PCI bus mastering.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>

Index: linux-2.6.21-quilt8/drivers/mfd/sm501.c
===================================================================
--- linux-2.6.21-quilt8.orig/drivers/mfd/sm501.c	2007-06-07 12:42:22.000000000 +0100
+++ linux-2.6.21-quilt8/drivers/mfd/sm501.c	2007-06-07 13:01:02.000000000 +0100
@@ -845,6 +845,24 @@ static void sm501_init_regs(struct sm501
 		dev_info(sm->dev, "setting MCLK to %ld\n", init->mclk);
 		sm501_set_clock(sm->dev, SM501_CLOCK_MCLK, init->mclk);
 	}
+
+}
+
+/* Check the PLL sources for the M1CLK and M1XCLK
+ *
+ * If the M1CLK and M1XCLKs are not sourced from the same PLL, then
+ * there is a risk (see errata AB-5) that the SM501 will cease proper
+ * function. If this happens, then it is likely the SM501 will
+ * hang the system.
+*/
+
+static int sm501_check_clocks(struct sm501_devdata *sm)
+{
+	unsigned long pwrmode = readl(sm->regs + SM501_CURRENT_CLOCK);
+	unsigned long msrc = (pwrmode & SM501_POWERMODE_M_SRC);
+	unsigned long m1src = (pwrmode & SM501_POWERMODE_M1_SRC);
+
+	return ((msrc == 0 && m1src != 0) || (msrc != 0 && m1src == 0));
 }
 
 static unsigned int sm501_mem_local[] = {
@@ -901,6 +919,12 @@ static int sm501_init_dev(struct sm501_d
 		}
 	}
 
+	ret = sm501_check_clocks(sm);
+	if (ret) {
+		dev_err(sm->dev, "M1X and M clocks sourced from different PLLs\n");
+		return -EINVAL;
+	}
+
 	/* always create a framebuffer */
 	sm501_register_display(sm, &mem_avail);
 
@@ -1034,8 +1058,12 @@ static struct sm501_initdata sm501_pci_i
 	},
 
 	.devices	= SM501_USE_ALL,
-	.mclk		= 100 * MHZ,
-	.m1xclk		= 160 * MHZ,
+
+	/* Errata AB-3 says that 72MHz is the fastest available
+	 * for 33MHZ PCI with proper bus-mastering operation */
+
+	.mclk		= 72 * MHZ,
+	.m1xclk		= 144 * MHZ,
 };
 
 static struct sm501_platdata_fbsub sm501_pdata_fbsub = {
Index: linux-2.6.21-quilt8/include/linux/sm501-regs.h
===================================================================
--- linux-2.6.21-quilt8.orig/include/linux/sm501-regs.h	2007-06-07 12:41:34.000000000 +0100
+++ linux-2.6.21-quilt8/include/linux/sm501-regs.h	2007-06-07 12:42:22.000000000 +0100
@@ -64,6 +64,11 @@
 #define SM501_DEBUG_CONTROL		(0x000034)
 
 /* power management */
+#define SM501_POWERMODE_P2X_SRC		(1<<29)
+#define SM501_POWERMODE_V2X_SRC		(1<<20)
+#define SM501_POWERMODE_M_SRC		(1<<12)
+#define SM501_POWERMODE_M1_SRC		(1<<4)
+
 #define SM501_CURRENT_GATE		(0x000038)
 #define SM501_CURRENT_CLOCK		(0x00003C)
 #define SM501_POWER_MODE_0_GATE		(0x000040)

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

[patch #2 5/6] SM501: Add Documentation/SM501.txt

[ben]

[-- Attachment #1: simtec-drivers-mfd-sm501-docs.patch --]
[-- Type: text/plain, Size: 3094 bytes --]

Add documentation for the SM501 in Documentation/SM501.txt
outlining the SM501 driver.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>

Index: linux-2.6.21-quilt8/Documentation/SM501.txt
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.21-quilt8/Documentation/SM501.txt	2007-06-13 13:12:04.000000000 +0100
@@ -0,0 +1,66 @@
+			SM501 Driver
+			============
+
+Copyright 2006, 2007 Simtec Electronics
+
+Core
+----
+
+The core driver in drivers/mfd provides common services for the
+drivers which manage the specific hardware blocks. These services
+include locking for common registers, clock control and resource
+management.
+
+The core registers drivers for both PCI and generic bus based
+chips via the platform device and driver system.
+
+On detection of a device, the core initialises the chip (which may
+be specified by the platform data) and then exports the selected
+peripheral set as platform devices for the specific drivers.
+
+The core re-uses the platform device system as the platform device
+system provides enough features to support the drivers without the
+need to create a new bus-type and the associated code to go with it.
+
+
+Resources
+---------
+
+Each peripheral has a view of the device which is implicitly narrowed to
+the specific set of resources that peripheral requires in order to
+function correctly.
+
+The centralised memory allocation allows the driver to ensure that the
+maximum possible resource allocation can be made to the video subsystem
+as this is by-far the most resource-sensitive of the on-chip functions.
+
+The primary issue with memory allocation is that of moving the video
+buffers once a display mode is chosen. Indeed when a video mode change
+occurs the memory footprint of the video subsystem changes.
+
+Since video memory is difficult to move without changing the display
+(unless sufficient contiguous memory can be provided for the old and new
+modes simultaneously) the video driver fully utilises the memory area
+given to it by aligning fb0 to the start of the area and fb1 to the end
+of it. Any memory left over in the middle is used for the acceleration
+functions, which are transient and thus their location is less critical
+as it can be moved.
+
+
+Configuration
+-------------
+
+The platform device driver uses a set of platform data to pass
+configurations through to the core and the subsidiary drivers
+so that there can be support for more than one system carrying
+an SM501 built into a single kernel image.
+
+The PCI driver assumes that the PCI card behaves as per the Silicon
+Motion reference design.
+
+There is an errata (AB-5) affecting the selection of the
+of the M1XCLK and M1CLK frequencies. These two clocks
+must be sourced from the same PLL, although they can then
+be divided down individually. If this is not set, then SM501 may
+lock and hang the whole system. The driver will refuse to
+attach if the PLL selection is different.

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

[patch #2 6/6] SM501: Check SM501 ID register on initialisation

[ben]

[-- Attachment #1: simtec-drivers-mfd-sm501-idcheck.patch --]
[-- Type: text/plain, Size: 2081 bytes --]

When binding the driver, check the ID register for a valid
identity, in case the SM501 is not functioning correctly.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>

Index: linux-2.6.21-quilt8/drivers/mfd/sm501.c
===================================================================
--- linux-2.6.21-quilt8.orig/drivers/mfd/sm501.c	2007-06-13 11:33:55.000000000 +0100
+++ linux-2.6.21-quilt8/drivers/mfd/sm501.c	2007-06-13 12:59:31.000000000 +0100
@@ -883,6 +883,7 @@ static int sm501_init_dev(struct sm501_d
 {
 	resource_size_t mem_avail;
 	unsigned long dramctrl;
+	unsigned long devid;
 	int ret;
 
 	mutex_init(&sm->clock_lock);
@@ -890,13 +891,18 @@ static int sm501_init_dev(struct sm501_d
 
 	INIT_LIST_HEAD(&sm->devices);
 
-	dramctrl = readl(sm->regs + SM501_DRAM_CONTROL);
+	devid = readl(sm->regs + SM501_DEVICEID);
 
+	if ((devid & SM501_DEVICEID_IDMASK) != SM501_DEVICEID_SM501) {
+		dev_err(sm->dev, "incorrect device id %08lx\n", devid);
+		return -EINVAL;
+	}
+
+	dramctrl = readl(sm->regs + SM501_DRAM_CONTROL);
 	mem_avail = sm501_mem_local[(dramctrl >> 13) & 0x7];
 
-	dev_info(sm->dev, "SM501 At %p: Version %08x, %ld Mb, IRQ %d\n",
-		 sm->regs, readl(sm->regs + SM501_DEVICEID),
-		 (unsigned long)mem_avail >> 20, sm->irq);
+	dev_info(sm->dev, "SM501 At %p: Version %08lx, %ld Mb, IRQ %d\n",
+		 sm->regs, devid, (unsigned long)mem_avail >> 20, sm->irq);
 
 	sm501_dump_gate(sm);
 
Index: linux-2.6.21-quilt8/include/linux/sm501-regs.h
===================================================================
--- linux-2.6.21-quilt8.orig/include/linux/sm501-regs.h	2007-06-13 11:33:55.000000000 +0100
+++ linux-2.6.21-quilt8/include/linux/sm501-regs.h	2007-06-13 11:33:56.000000000 +0100
@@ -109,6 +109,9 @@
 #define SM501_DEVICEID			(0x000060)
 /* 0x050100A0 */
 
+#define SM501_DEVICEID_SM501		(0x05010000)
+#define SM501_DEVICEID_IDMASK		(0xffff0000)
+
 #define SM501_PLLCLOCK_COUNT		(0x000064)
 #define SM501_MISC_TIMING		(0x000068)
 #define SM501_CURRENT_SDRAM_CLOCK	(0x00006C)

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'