LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/3 v2] add MPC837x USB platform support
From: Li Yang @ 2008-01-08  7:18 UTC (permalink / raw)
  To: galak, linuxppc-dev; +Cc: sfr, Li Yang, paulus

Add chip specific and board specific initialization for MPC837x USB.

Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/platforms/83xx/mpc837x_mds.c |   51 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/83xx/mpc83xx.h     |    3 ++
 arch/powerpc/platforms/83xx/usb.c         |   40 ++++++++++++++++++++++
 3 files changed, 94 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mpc837x_mds.c b/arch/powerpc/platforms/83xx/mpc837x_mds.c
index 166c111..c7579f6 100644
--- a/arch/powerpc/platforms/83xx/mpc837x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc837x_mds.c
@@ -27,6 +27,56 @@ unsigned long isa_io_base = 0;
 unsigned long isa_mem_base = 0;
 #endif
 
+#define BCSR12_USB_SER_MASK	0x8a
+#define BCSR12_USB_SER_PIN	0x80
+#define BCSR12_USB_SER_DEVICE	0x02
+extern int mpc837x_usb_cfg(void);
+
+static int mpc837xmds_usb_cfg(void)
+{
+	struct device_node *np;
+	const void *phy_type, *mode;
+	void __iomem *bcsr_regs = NULL;
+	u8 bcsr12;
+	int ret;
+
+	ret = mpc837x_usb_cfg();
+	if (ret)
+		return ret;
+	/* Map BCSR area */
+	np = of_find_node_by_name(NULL, "bcsr");
+	if (np) {
+		struct resource res;
+
+		of_address_to_resource(np, 0, &res);
+		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
+		of_node_put(np);
+	}
+	if (!bcsr_regs)
+		return -1;
+
+	np = of_find_node_by_name(NULL, "usb");
+	if (!np)
+		return -ENODEV;
+	phy_type = of_get_property(np, "phy_type", NULL);
+	if (phy_type && !strcmp(phy_type, "ulpi")) {
+		clrbits8(bcsr_regs + 12, BCSR12_USB_SER_PIN);
+	} else if (phy_type && !strcmp(phy_type, "serial")) {
+		mode = of_get_property(np, "dr_mode", NULL);
+		bcsr12 = in_8(bcsr_regs + 12) & ~BCSR12_USB_SER_MASK;
+		bcsr12 |= BCSR12_USB_SER_PIN;
+		if (mode && !strcmp(mode, "peripheral"))
+			bcsr12 |= BCSR12_USB_SER_DEVICE;
+		out_8(bcsr_regs + 12, bcsr12);
+	} else {
+		printk(KERN_ERR "USB DR: unsupported PHY\n");
+	}
+
+	of_node_put(np);
+	iounmap(bcsr_regs);
+	return 0;
+}
+
 /* ************************************************************************
  *
  * Setup the architecture
@@ -45,6 +95,7 @@ static void __init mpc837x_mds_setup_arch(void)
 	for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
 		mpc83xx_add_bridge(np);
 #endif
+	mpc837xmds_usb_cfg();
 }
 
 static struct of_device_id mpc837x_ids[] = {
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/platforms/83xx/mpc83xx.h
index b778cb4..88bb748 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -14,6 +14,7 @@
 #define MPC83XX_SCCR_USB_DRCM_11   0x00300000
 #define MPC83XX_SCCR_USB_DRCM_01   0x00100000
 #define MPC83XX_SCCR_USB_DRCM_10   0x00200000
+#define MPC837X_SCCR_USB_DRCM_11   0x00c00000
 
 /* system i/o configuration register low */
 #define MPC83XX_SICRL_OFFS         0x114
@@ -22,6 +23,8 @@
 #define MPC834X_SICRL_USB1         0x20000000
 #define MPC831X_SICRL_USB_MASK     0x00000c00
 #define MPC831X_SICRL_USB_ULPI     0x00000800
+#define MPC837X_SICRL_USB_MASK     0xf0000000
+#define MPC837X_SICRL_USB_ULPI     0x50000000
 
 /* system i/o configuration register high */
 #define MPC83XX_SICRH_OFFS         0x118
diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c
index b45160f..b1de453 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -179,3 +179,43 @@ int mpc831x_usb_cfg(void)
 	return ret;
 }
 #endif /* CONFIG_PPC_MPC831x */
+
+#ifdef CONFIG_PPC_MPC837x
+int mpc837x_usb_cfg(void)
+{
+	void __iomem *immap;
+	struct device_node *np = NULL;
+	const void *prop;
+	int ret = 0;
+
+	np = of_find_compatible_node(NULL, "usb", "fsl-usb2-dr");
+	if (!np)
+		return -ENODEV;
+	prop = of_get_property(np, "phy_type", NULL);
+
+	if (!prop || (strcmp(prop, "ulpi") && strcmp(prop, "serial"))) {
+		printk(KERN_WARNING "837x USB PHY type not supported\n");
+		of_node_put(np);
+		return -EINVAL;
+	}
+
+	/* Map IMMR space for pin and clock settings */
+	immap = ioremap(get_immrbase(), 0x1000);
+	if (!immap) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	/* Configure clock */
+	clrsetbits_be32(immap + MPC83XX_SCCR_OFFS, MPC837X_SCCR_USB_DRCM_11,
+			MPC837X_SCCR_USB_DRCM_11);
+
+	/* Configure pin mux for ULPI/serial */
+	clrsetbits_be32(immap + MPC83XX_SICRL_OFFS, MPC837X_SICRL_USB_MASK,
+			MPC837X_SICRL_USB_ULPI);
+
+	iounmap(immap);
+	of_node_put(np);
+	return ret;
+}
+#endif /* CONFIG_PPC_MPC837x */
-- 
1.5.3.5.643.g40e25

^ permalink raw reply related

* Re: [PATCH] Hwmon for Taco
From: Grant Likely @ 2008-01-08  6:59 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <47831868.3030309@pikatech.com>

On 1/7/08, Sean MacLennan <smaclennan@pikatech.com> wrote:
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index a0445be..1f89186 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -57,6 +57,16 @@ config SENSORS_ABITUGURU3
>        This driver can also be built as a module.  If so, the module
>        will be called abituguru3.
>
> +config SENSORS_AD7414
> +    tristate "Analog Devices AD7414"
> +    depends on I2C && EXPERIMENTAL
> +    help
> +      If you say yes here you get support for the Analog Devices
> +      AD7414 temperature monitoring chip.
> +
> +      This driver can also be built as a module. If so, the module
> +      will be called ad7414.
> +
>  config SENSORS_AD7418
>      tristate "Analog Devices AD7416, AD7417 and AD7418"
>      depends on I2C && EXPERIMENTAL
> @@ -763,4 +773,13 @@ config HWMON_DEBUG_CHIP
>        a problem with I2C support and want to see more of what is going
>        on.
>
> +config PIKA_DTM
> +    tristate "PIKA DTM (Dynamic Thermal Management)"
> +    depends on HWMON && WARP
> +    select SENSORS_AD7414

select is dangerous because it bypasses dependency checking.  Make it
'depends on' instead.

> +    default y
> +    help
> +      Say Y here if you have a PIKA Warp(tm) Appliance. This driver is
> +      required for the DTM to work properly.
> +

This patch should be split in 2; one for the AD7414 driver and one for
the thermal management driver.

>  endif # HWMON
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 55595f6..0c6ee71 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_SENSORS_W83791D)    += w83791d.o
>
>  obj-$(CONFIG_SENSORS_ABITUGURU)    += abituguru.o
>  obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
> +obj-$(CONFIG_SENSORS_AD7414)    += ad7414.o
>  obj-$(CONFIG_SENSORS_AD7418)    += ad7418.o
>  obj-$(CONFIG_SENSORS_ADM1021)    += adm1021.o
>  obj-$(CONFIG_SENSORS_ADM1025)    += adm1025.o
> @@ -69,7 +70,8 @@ obj-$(CONFIG_SENSORS_VT8231)    += vt8231.o
>  obj-$(CONFIG_SENSORS_W83627EHF)    += w83627ehf.o
>  obj-$(CONFIG_SENSORS_W83L785TS)    += w83l785ts.o
>
> +obj-$(CONFIG_PIKA_DTM)        += pika-dtm.o
> +
>  ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
>  EXTRA_CFLAGS += -DDEBUG
>  endif
> -
> --- /dev/null    2005-11-20 22:22:37.000000000 -0500
> +++ drivers/hwmon/pika-dtm.c    2008-01-08 01:23:32.000000000 -0500

This is *very* board specific and not very complex a driver.  It
should probably live with the platform code somewhere in
arch/powerpc/platforms.  You can use the machine_device_initcall()
hook to kick off the thread.

> @@ -0,0 +1,87 @@
> +/*
> + *  drivers/hwmon/pika-dtm.c
> + *
> + *  Overview: On the Warp, the fpga controls the fan. This provides
> + *  the temperature to the fpga.
> + *
> + *  Copyright (c) 2008 PIKA Technologies
> + *    Sean MacLennan <smaclennan@pikatech.com>
> + *
> + *  This program is free software; you can redistribute     it and/or
> modify it
> + *  under  the terms of     the GNU General  Public License as
> published by the
> + *  Free Software Foundation;  either version 2 of the    License, or
> (at your
> + *  option) any later version.

Your mailer chewed up the patch here (line wrap).

> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kthread.h>
> +#include <linux/io.h>
> +#include <linux/of_platform.h>
> +
> +
> +extern int ad7414_get_temp(void);

Bad!  Function decls must be in common header files.

> +
> +static unsigned __iomem *dtm_fpga;
> +static struct task_struct *dtm_thread;
> +
> +
> +static int pika_dtm_thread(void *arg)
> +{
> +    while(!kthread_should_stop()) {
> +        int temp = ad7414_get_temp();
> +
> +        // Write to FPGA

Style; use /* */, not //

> +        out_be32(dtm_fpga, temp);
> +
> +        set_current_state(TASK_INTERRUPTIBLE);
> +        schedule_timeout(HZ);
> +    }
> +
> +    return 0;
> +}
> +
> +
> +int __init pika_dtm_init(void)
> +{
> +    struct device_node *np;
> +    struct resource res;
> +
> +    if((np = of_find_compatible_node(NULL, NULL, "pika,fpga")) == NULL) {
> +        printk(KERN_ERR __FILE__ ": Unable to find FPGA\n");
> +        return -ENOENT;
> +    }
> +
> +    /* We do not call of_iomap here since it would map in the entire
> +     * fpga space, which is overkill for 4 bytes.
> +     */

iomapping is not expensive; just map the whole space (it's going to
map a minimum 4k page anyway).  The code will be easier to read if you
just use of_iomap().

> +    if(of_address_to_resource(np, 0, &res) ||
> +       (dtm_fpga = ioremap(res.start + 0x20, 4)) == NULL) {
> +        printk(KERN_ERR __FILE__ ": Unable to map FPGA\n");
> +        return -ENOENT;
> +    }
> +
> +    dtm_thread = kthread_run(pika_dtm_thread, NULL, "pika-dtm");
> +
> +    if(IS_ERR(dtm_thread)) {
> +        iounmap(dtm_fpga);
> +        printk(KERN_ERR __FILE__ ": Unable to start PIKA DTM thread\n");
> +        return PTR_ERR(dtm_thread);
> +    }
> +
> +    return 0;
> +}
> +module_init(pika_dtm_init);
> +
> +
> +void __exit pika_dtm_exit(void)
> +{
> +    kthread_stop(dtm_thread);
> +    iounmap(dtm_fpga);
> +}
> +module_exit(pika_dtm_exit);
> +
> +
> +MODULE_DESCRIPTION("PIKA DTM driver");
> +MODULE_AUTHOR("Sean MacLennan");
> +MODULE_LICENSE("GPL");
> --- /dev/null    2005-11-20 22:22:37.000000000 -0500
> +++ drivers/hwmon/ad7414.c    2008-01-05 20:36:06.000000000 -0500
> @@ -0,0 +1,296 @@
> +/*
> + * An hwmon driver for the Analog Devices AD7414
> + *
> + * Copyright 2006 Stefan Roese <sr@denx.de>, DENX Software Engineering
> + *
> + * Based on ad7418.c
> + * Copyright 2006 Tower Technologies, Alessandro Zummo
> <a.zummo@towertech.it>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +
> +
> +#define DRV_VERSION "0.2"
> +
> +/* straight from the datasheet */
> +#define AD7414_TEMP_MIN (-55000)
> +#define AD7414_TEMP_MAX 125000
> +
> +/* Addresses to scan */
> +static unsigned short normal_i2c[] = { 0x48, 0x4a, I2C_CLIENT_END };
> +
> +/* Insmod parameters */
> +I2C_CLIENT_INSMOD;
> +
> +/* AD7414 registers */
> +#define AD7414_REG_TEMP        0x00
> +#define AD7414_REG_CONF        0x01
> +#define AD7414_REG_T_HIGH    0x02
> +#define AD7414_REG_T_LOW    0x03
> +
> +struct ad7414_data {
> +    struct i2c_client    client;
> +    struct device    *dev;
> +    struct mutex        lock;
> +    char            valid;        /* !=0 if following fields are valid */
> +    unsigned long        last_updated;    /* In jiffies */
> +    u16            temp_input;    /* Register values */
> +    u8            temp_max;
> +    u8            temp_min;
> +    u8            temp_alert;
> +    u8            temp_max_flag;
> +    u8            temp_min_flag;
> +};
> +
> +static int ad7414_attach_adapter(struct i2c_adapter *adapter);
> +static int ad7414_detect(struct i2c_adapter *adapter, int address, int
> kind);
> +static int ad7414_detach_client(struct i2c_client *client);
> +
> +static struct i2c_driver ad7414_driver = {
> +    .driver = {
> +        .name    = "ad7414",
> +    },
> +    .attach_adapter    = ad7414_attach_adapter,
> +    .detach_client    = ad7414_detach_client,
> +};
> +
> +/*
> + * TEMP: 0.001C/bit (-55C to +125C)
> + * REG: (0.5C/bit, two's complement) << 7
> + */
> +static inline int AD7414_TEMP_FROM_REG(u16 reg)
> +{
> +    /* use integer division instead of equivalent right shift to
> +     * guarantee arithmetic shift and preserve the sign
> +     */
> +    return ((s16)reg / 128) * 500;
> +}
> +
> +/* All registers are word-sized, except for the configuration registers.
> + * AD7414 uses a high-byte first convention, which is exactly opposite to
> + * the usual practice.
> + */
> +static int ad7414_read(struct i2c_client *client, u8 reg)
> +{
> +    if (reg == AD7414_REG_TEMP)
> +        return swab16(i2c_smbus_read_word_data(client, reg));
> +    else
> +        return i2c_smbus_read_byte_data(client, reg);
> +}
> +
> +static int ad7414_write(struct i2c_client *client, u8 reg, u16 value)
> +{
> +    return i2c_smbus_write_byte_data(client, reg, value);
> +}
> +
> +/* PIKA Taco - we need to access the temperature in kernel mode. As a
> + * hack we store the device here. This works because we only have one
> + * ad7414 chip.
> + */
> +static struct device *ad7414_dev;

Can you use a list_head instead?  That would allow multiple instances.

This driver shouldn't contain board specific code.

> +
> +static void ad7414_init_client(struct i2c_client *client)
> +{
> +    /* TODO: anything to do here??? */
> +    ad7414_dev = &client->dev;

ick.

> +}
> +
> +static struct ad7414_data *ad7414_update_device(struct device *dev)
> +{
> +    struct i2c_client *client = to_i2c_client(dev);
> +    struct ad7414_data *data = i2c_get_clientdata(client);
> +
> +    mutex_lock(&data->lock);
> +
> +    if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
> +        || !data->valid) {
> +        dev_dbg(&client->dev, "starting ad7414 update\n");
> +
> +        data->temp_input = ad7414_read(client, AD7414_REG_TEMP);
> +        data->temp_alert = (data->temp_input >> 5) & 0x01;
> +        data->temp_max_flag = (data->temp_input >> 4) & 0x01;
> +        data->temp_min_flag = (data->temp_input >> 3) & 0x01;
> +        data->temp_max = ad7414_read(client, AD7414_REG_T_HIGH);
> +        data->temp_min = ad7414_read(client, AD7414_REG_T_LOW);
> +
> +        data->last_updated = jiffies;
> +        data->valid = 1;
> +    }
> +
> +    mutex_unlock(&data->lock);
> +
> +    return data;
> +}
> +
> +int ad7414_get_temp(void)

maybe ad7414_get_temp(int index)?  Would allow for multiple instances.

> +{
> +    if(ad7414_dev) {
> +        struct ad7414_data *data = ad7414_update_device(ad7414_dev);
> +        return data->temp_input;
> +    } else
> +        return 0x1f4; // +125

Style; c++ comment

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH] i2c-ibm_iic driver - new patch
From: Stephen Rothwell @ 2008-01-08  6:36 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev, Stefan Roese, Arnd Bergmann
In-Reply-To: <4783108B.6070308@pikatech.com>

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

On Tue, 08 Jan 2008 00:56:27 -0500 Sean MacLennan <smaclennan@pikatech.com> wrote:
>
> Stephen Rothwell wrote:
> >
> > On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <smaclennan@pikatech.com> wrote:
> >   
> > Please don't post patches as attachments.
>  
> Ok.

Unfortunately, you are using thunderbird and so the patch is now wrapped.
There is a workaround, see Documentation/email-clients.txt.

> > Please split the assignments from the tests.  Here and elsewhere.
> >   
> I made the changes in my code. I am trying to leave the original code as 
> much as possible.

Thats all we ask.

> >> +	} else {
> >> +		if (dev->irq != NO_IRQ){
> >> +		    iic_interrupt_mode(dev, 0);
> >> +		    free_irq(dev->irq, dev);
> >> +		}
> >> +		iounmap(dev->vaddr);
> >> +		kfree(dev);
> >>     
> >
> > Should these last two be after the below brace?
> >
> >   
> I'm not really qualified to answer, but I will anyway ;) I assume the 
> original author is basically saying if he cannot delete the adapter, it 
> is unsafe to free the memory since the i2c code may still use it. If I 
> have read that right, then I agree.

OK, I can see that this is a "that should not happen" condition.

> +    if (iic_force_poll)
> +        dev->irq = NO_IRQ;
> +    else if ((dev->irq = irq_of_parse_and_map(np, 0)) == NO_IRQ)

You missed this one.

Overall looks better, except all your indentation is now 4 spaces. We use
a TAB character for each level of indentation and you should be able to
set your editor to *display* the TABs as 4 places if that is what you like.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH] Hwmon for Taco
From: Sean MacLennan @ 2008-01-08  6:30 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev
In-Reply-To: <1199517763.7291.47.camel@pasglop>

Benjamin Herrenschmidt wrote:
> That should be in the device-tree...
>
> Cheers,
> Ben.
>
>   

Now in the device tree. The name of the file has changed.

Cheers,
    Sean

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index a0445be..1f89186 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -57,6 +57,16 @@ config SENSORS_ABITUGURU3
       This driver can also be built as a module.  If so, the module
       will be called abituguru3.
 
+config SENSORS_AD7414
+    tristate "Analog Devices AD7414"
+    depends on I2C && EXPERIMENTAL
+    help
+      If you say yes here you get support for the Analog Devices
+      AD7414 temperature monitoring chip.
+
+      This driver can also be built as a module. If so, the module
+      will be called ad7414.
+
 config SENSORS_AD7418
     tristate "Analog Devices AD7416, AD7417 and AD7418"
     depends on I2C && EXPERIMENTAL
@@ -763,4 +773,13 @@ config HWMON_DEBUG_CHIP
       a problem with I2C support and want to see more of what is going
       on.
 
+config PIKA_DTM
+    tristate "PIKA DTM (Dynamic Thermal Management)"
+    depends on HWMON && WARP
+    select SENSORS_AD7414
+    default y
+    help
+      Say Y here if you have a PIKA Warp(tm) Appliance. This driver is
+      required for the DTM to work properly.
+
 endif # HWMON
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 55595f6..0c6ee71 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SENSORS_W83791D)    += w83791d.o
 
 obj-$(CONFIG_SENSORS_ABITUGURU)    += abituguru.o
 obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o
+obj-$(CONFIG_SENSORS_AD7414)    += ad7414.o
 obj-$(CONFIG_SENSORS_AD7418)    += ad7418.o
 obj-$(CONFIG_SENSORS_ADM1021)    += adm1021.o
 obj-$(CONFIG_SENSORS_ADM1025)    += adm1025.o
@@ -69,7 +70,8 @@ obj-$(CONFIG_SENSORS_VT8231)    += vt8231.o
 obj-$(CONFIG_SENSORS_W83627EHF)    += w83627ehf.o
 obj-$(CONFIG_SENSORS_W83L785TS)    += w83l785ts.o
 
+obj-$(CONFIG_PIKA_DTM)        += pika-dtm.o
+
 ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
 EXTRA_CFLAGS += -DDEBUG
 endif
-
--- /dev/null    2005-11-20 22:22:37.000000000 -0500
+++ drivers/hwmon/pika-dtm.c    2008-01-08 01:23:32.000000000 -0500
@@ -0,0 +1,87 @@
+/*
+ *  drivers/hwmon/pika-dtm.c
+ *
+ *  Overview: On the Warp, the fpga controls the fan. This provides
+ *  the temperature to the fpga.
+ *
+ *  Copyright (c) 2008 PIKA Technologies
+ *    Sean MacLennan <smaclennan@pikatech.com>
+ *
+ *  This program is free software; you can redistribute     it and/or 
modify it
+ *  under  the terms of     the GNU General  Public License as 
published by the
+ *  Free Software Foundation;  either version 2 of the    License, or 
(at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kthread.h>
+#include <linux/io.h>
+#include <linux/of_platform.h>
+
+
+extern int ad7414_get_temp(void);
+
+static unsigned __iomem *dtm_fpga;
+static struct task_struct *dtm_thread;
+
+
+static int pika_dtm_thread(void *arg)
+{
+    while(!kthread_should_stop()) {
+        int temp = ad7414_get_temp();
+
+        // Write to FPGA
+        out_be32(dtm_fpga, temp);
+
+        set_current_state(TASK_INTERRUPTIBLE);
+        schedule_timeout(HZ);
+    }
+
+    return 0;
+}
+
+
+int __init pika_dtm_init(void)
+{
+    struct device_node *np;
+    struct resource res;
+
+    if((np = of_find_compatible_node(NULL, NULL, "pika,fpga")) == NULL) {
+        printk(KERN_ERR __FILE__ ": Unable to find FPGA\n");
+        return -ENOENT;
+    }
+
+    /* We do not call of_iomap here since it would map in the entire
+     * fpga space, which is overkill for 4 bytes.
+     */
+    if(of_address_to_resource(np, 0, &res) ||
+       (dtm_fpga = ioremap(res.start + 0x20, 4)) == NULL) {
+        printk(KERN_ERR __FILE__ ": Unable to map FPGA\n");
+        return -ENOENT;
+    }
+
+    dtm_thread = kthread_run(pika_dtm_thread, NULL, "pika-dtm");
+
+    if(IS_ERR(dtm_thread)) {
+        iounmap(dtm_fpga);
+        printk(KERN_ERR __FILE__ ": Unable to start PIKA DTM thread\n");
+        return PTR_ERR(dtm_thread);
+    }
+
+    return 0;
+}
+module_init(pika_dtm_init);
+
+
+void __exit pika_dtm_exit(void)
+{
+    kthread_stop(dtm_thread);
+    iounmap(dtm_fpga);
+}
+module_exit(pika_dtm_exit);
+
+
+MODULE_DESCRIPTION("PIKA DTM driver");
+MODULE_AUTHOR("Sean MacLennan");
+MODULE_LICENSE("GPL");
--- /dev/null    2005-11-20 22:22:37.000000000 -0500
+++ drivers/hwmon/ad7414.c    2008-01-05 20:36:06.000000000 -0500
@@ -0,0 +1,296 @@
+/*
+ * An hwmon driver for the Analog Devices AD7414
+ *
+ * Copyright 2006 Stefan Roese <sr@denx.de>, DENX Software Engineering
+ *
+ * Based on ad7418.c
+ * Copyright 2006 Tower Technologies, Alessandro Zummo 
<a.zummo@towertech.it>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+
+#define DRV_VERSION "0.2"
+
+/* straight from the datasheet */
+#define AD7414_TEMP_MIN (-55000)
+#define AD7414_TEMP_MAX 125000
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = { 0x48, 0x4a, I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD;
+
+/* AD7414 registers */
+#define AD7414_REG_TEMP        0x00
+#define AD7414_REG_CONF        0x01
+#define AD7414_REG_T_HIGH    0x02
+#define AD7414_REG_T_LOW    0x03
+
+struct ad7414_data {
+    struct i2c_client    client;
+    struct device    *dev;
+    struct mutex        lock;
+    char            valid;        /* !=0 if following fields are valid */
+    unsigned long        last_updated;    /* In jiffies */
+    u16            temp_input;    /* Register values */
+    u8            temp_max;
+    u8            temp_min;
+    u8            temp_alert;
+    u8            temp_max_flag;
+    u8            temp_min_flag;
+};
+
+static int ad7414_attach_adapter(struct i2c_adapter *adapter);
+static int ad7414_detect(struct i2c_adapter *adapter, int address, int 
kind);
+static int ad7414_detach_client(struct i2c_client *client);
+
+static struct i2c_driver ad7414_driver = {
+    .driver = {
+        .name    = "ad7414",
+    },
+    .attach_adapter    = ad7414_attach_adapter,
+    .detach_client    = ad7414_detach_client,
+};
+
+/*
+ * TEMP: 0.001C/bit (-55C to +125C)
+ * REG: (0.5C/bit, two's complement) << 7
+ */
+static inline int AD7414_TEMP_FROM_REG(u16 reg)
+{
+    /* use integer division instead of equivalent right shift to
+     * guarantee arithmetic shift and preserve the sign
+     */
+    return ((s16)reg / 128) * 500;
+}
+
+/* All registers are word-sized, except for the configuration registers.
+ * AD7414 uses a high-byte first convention, which is exactly opposite to
+ * the usual practice.
+ */
+static int ad7414_read(struct i2c_client *client, u8 reg)
+{
+    if (reg == AD7414_REG_TEMP)
+        return swab16(i2c_smbus_read_word_data(client, reg));
+    else
+        return i2c_smbus_read_byte_data(client, reg);
+}
+
+static int ad7414_write(struct i2c_client *client, u8 reg, u16 value)
+{
+    return i2c_smbus_write_byte_data(client, reg, value);
+}
+
+/* PIKA Taco - we need to access the temperature in kernel mode. As a
+ * hack we store the device here. This works because we only have one
+ * ad7414 chip.
+ */
+static struct device *ad7414_dev;
+
+static void ad7414_init_client(struct i2c_client *client)
+{
+    /* TODO: anything to do here??? */
+    ad7414_dev = &client->dev;
+}
+
+static struct ad7414_data *ad7414_update_device(struct device *dev)
+{
+    struct i2c_client *client = to_i2c_client(dev);
+    struct ad7414_data *data = i2c_get_clientdata(client);
+
+    mutex_lock(&data->lock);
+
+    if (time_after(jiffies, data->last_updated + HZ + HZ / 2)
+        || !data->valid) {
+        dev_dbg(&client->dev, "starting ad7414 update\n");
+
+        data->temp_input = ad7414_read(client, AD7414_REG_TEMP);
+        data->temp_alert = (data->temp_input >> 5) & 0x01;
+        data->temp_max_flag = (data->temp_input >> 4) & 0x01;
+        data->temp_min_flag = (data->temp_input >> 3) & 0x01;
+        data->temp_max = ad7414_read(client, AD7414_REG_T_HIGH);
+        data->temp_min = ad7414_read(client, AD7414_REG_T_LOW);
+
+        data->last_updated = jiffies;
+        data->valid = 1;
+    }
+
+    mutex_unlock(&data->lock);
+
+    return data;
+}
+
+int ad7414_get_temp(void)
+{
+    if(ad7414_dev) {
+        struct ad7414_data *data = ad7414_update_device(ad7414_dev);
+        return data->temp_input;
+    } else
+        return 0x1f4; // +125
+}
+EXPORT_SYMBOL(ad7414_get_temp);
+
+#define show(value) \
+static ssize_t show_##value(struct device *dev, struct device_attribute 
*attr, char *buf)        \
+{                                    \
+    struct ad7414_data *data = ad7414_update_device(dev);        \
+    return sprintf(buf, "%d\n", AD7414_TEMP_FROM_REG(data->value));    \
+}
+show(temp_input);
+
+#define show_8(value)    \
+static ssize_t show_##value(struct device *dev, struct device_attribute 
*attr, char *buf)        \
+{                                \
+    struct ad7414_data *data = ad7414_update_device(dev);    \
+    return sprintf(buf, "%d\n", data->value);        \
+}
+show_8(temp_max);
+show_8(temp_min);
+show_8(temp_alert);
+show_8(temp_max_flag);
+show_8(temp_min_flag);
+
+#define set(value, reg)    \
+static ssize_t set_##value(struct device *dev, struct device_attribute 
*attr, const char *buf, size_t count)    \
+{                                \
+    struct i2c_client *client = to_i2c_client(dev);        \
+    struct ad7414_data *data = i2c_get_clientdata(client);    \
+    int temp = simple_strtoul(buf, NULL, 10);        \
+                                \
+    mutex_lock(&data->lock);                \
+    data->value = temp;                    \
+    ad7414_write(client, reg, data->value);            \
+    mutex_unlock(&data->lock);                \
+    return count;                        \
+}
+set(temp_max, AD7414_REG_T_HIGH);
+set(temp_min, AD7414_REG_T_LOW);
+
+static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max, 
set_temp_max);
+static DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_min, 
set_temp_min);
+static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input, NULL);
+static DEVICE_ATTR(temp1_alert, S_IRUGO, show_temp_alert, NULL);
+static DEVICE_ATTR(temp1_max_flag, S_IRUGO, show_temp_max_flag, NULL);
+static DEVICE_ATTR(temp1_min_flag, S_IRUGO, show_temp_min_flag, NULL);
+
+static int ad7414_attach_adapter(struct i2c_adapter *adapter)
+{
+    if (!(adapter->class & I2C_CLASS_HWMON))
+        return 0;
+    return i2c_probe(adapter, &addr_data, ad7414_detect);
+}
+
+static struct attribute *ad7414_attributes[] = {
+    &dev_attr_temp1_input.attr,
+    &dev_attr_temp1_max.attr,
+    &dev_attr_temp1_min.attr,
+    &dev_attr_temp1_alert.attr,
+    &dev_attr_temp1_max_flag.attr,
+    &dev_attr_temp1_min_flag.attr,
+    NULL
+};
+
+static const struct attribute_group ad7414_group = {
+    .attrs = ad7414_attributes,
+};
+
+static int ad7414_detect(struct i2c_adapter *adapter, int address, int 
kind)
+{
+    struct i2c_client *client;
+    struct ad7414_data *data;
+    int err = 0;
+
+    if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+                    I2C_FUNC_SMBUS_WORD_DATA))
+        goto exit;
+
+    if (!(data = kzalloc(sizeof(struct ad7414_data), GFP_KERNEL))) {
+        err = -ENOMEM;
+        goto exit;
+    }
+
+    client = &data->client;
+    client->addr = address;
+    client->adapter = adapter;
+    client->driver = &ad7414_driver;
+    client->flags = 0;
+
+    i2c_set_clientdata(client, data);
+
+    mutex_init(&data->lock);
+
+    /* TODO: not testing for AD7414 done yet... */
+
+    strlcpy(client->name, ad7414_driver.driver.name, I2C_NAME_SIZE);
+
+    if ((err = i2c_attach_client(client)))
+        goto exit_free;
+
+    dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n");
+
+    /* Initialize the AD7414 chip */
+    ad7414_init_client(client);
+
+    /* Register sysfs hooks */
+    if ((err = sysfs_create_group(&client->dev.kobj, &ad7414_group)))
+        goto exit_detach;
+
+    data->dev = hwmon_device_register(&client->dev);
+    if (IS_ERR(data->dev)) {
+        err = PTR_ERR(data->dev);
+        goto exit_remove;
+    }
+
+    return 0;
+
+exit_remove:
+    sysfs_remove_group(&client->dev.kobj, &ad7414_group);
+exit_detach:
+    i2c_detach_client(client);
+exit_free:
+    kfree(data);
+exit:
+    return err;
+}
+
+static int ad7414_detach_client(struct i2c_client *client)
+{
+    struct ad7414_data *data = i2c_get_clientdata(client);
+    ad7414_dev = NULL;
+    hwmon_device_unregister(data->dev);
+    sysfs_remove_group(&client->dev.kobj, &ad7414_group);
+    i2c_detach_client(client);
+    kfree(data);
+    return 0;
+}
+
+static int __init ad7414_init(void)
+{
+    return i2c_add_driver(&ad7414_driver);
+}
+
+static void __exit ad7414_exit(void)
+{
+    i2c_del_driver(&ad7414_driver);
+}
+
+MODULE_AUTHOR("Stefan Roese <sr@denx.de>");
+MODULE_DESCRIPTION("AD7414 driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
+
+module_init(ad7414_init);
+module_exit(ad7414_exit);

^ permalink raw reply related

* Re: [PATCH] Fix carry bug in 128-bit unsigned integer adding
From: Kumar Gala @ 2008-01-08  6:27 UTC (permalink / raw)
  To: Liu Yu; +Cc: linuxppc-dev
In-Reply-To: <11996295742378-git-send-email-Yu.Liu@freescale.com>


On Jan 6, 2008, at 8:26 AM, Liu Yu wrote:

> This bug exists in math emulation for powerpc.
> The macro define are mainly used by multiplication.
>
> When adding two unsigned operands ( r = x + y ),
> the carry bit can be counted by whether r is less than x.
> However, when adding three unsigned operands, this method does not  
> work.
>
> The original code below uses this method to count carry,
> it apparently overlook the case of three operands.
> Assume all the operands is 32-bit wide,
> ( r = x + y + last_carry , x = 0, y = 0xffffffff, last_carry = 1),
> then r is no less than x but it actually gets a carry.
>
> I tried to fix this bug, but this patch seems not that pretty.
> Are there any better ideas?
> Comments are always welcomed!

take a look at how include/math-emu/op-4.h implements __FP_FRAC_ADD_4  
& __FP_FRAC_SUB_4.  Will that fix the bug, if so we should make the  
code match how its done there.

- k

^ permalink raw reply

* Re: [PATCH] Fix remainder calculating bug in single floating point division
From: Kumar Gala @ 2008-01-08  6:22 UTC (permalink / raw)
  To: Liu Yu; +Cc: linuxppc-dev
In-Reply-To: <11996296134110-git-send-email-Yu.Liu@freescale.com>


On Jan 6, 2008, at 8:26 AM, Liu Yu wrote:

> This bug exists in the emulation of floating point division for  
> powerpc.
>
> The original code cannot count the remainder correctly.
> I can provide a test case to trigger this bug.
> When use fdiv to count 1.1754941e-38f / 0.9999999f,
> the result is expected to be 1.175494e-38f,
> but we will get 1.174921e-38f in the original case.
>
> Comments are always welcomed!

can you provide the test case that shows the error.

- k

^ permalink raw reply

* Re: [PATCH] Fix remainder calculating bug in single floating point division
From: Kumar Gala @ 2008-01-08  6:20 UTC (permalink / raw)
  To: Dan Malek; +Cc: Liu Yu, linuxppc-dev
In-Reply-To: <66F0B96A-5B53-48D3-BB90-07841F3EA1EE@embeddedalley.com>


On Jan 6, 2008, at 2:44 PM, Dan Malek wrote:

>
> On Jan 6, 2008, at 12:07 PM, Benjamin Herrenschmidt wrote:
>
>> It's nice to see somebody digging in that scary math emu stuff. If  
>> you
>> could also get rid of the warnings, it would be perfect :-)
>
> Yes, it is :-)  I didn't think it would have a life beyond MPC8xx.
>
>> .... that this code was lifted from
>> somewhere else (glibc ? gcc soft-float ?),
>
> It seems like a lifetime ago....  I copied the framework
> from Sparc, and the internals from gcc soft-float.  I didn't
> change any of the internal emulation functions (hence,
> some of the warnings), just the calling interface.
>
> While it's convenient, I still don't think kernel float
> emulation should be a solution.  The tools should
> generate soft-float for the applications and libraries.

If we think this is really true, we could move to using include/math- 
emu/* instead of the files in powerpc/math-emu.

The problem I had was when I tried to recreate the history of the code  
in powerpc/math-emu and how it doesn't really match the glibc code  
base.  There are some differences and I wasn't sure if they were do to  
trying to match PPC HW at a bit level or not.

I was hoping that the work Liu Yu would get as a bit of a testsuite to  
see if there was any harm in moving over to include/math-emu.

- k

^ permalink raw reply

* Re: [PATCH] sbc85xx: remove PCI exclude device for sbc8548/sbc8560
From: Kumar Gala @ 2008-01-08  6:15 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linuxppc-dev
In-Reply-To: <1199745618755-git-send-email-paul.gortmaker@windriver.com>


On Jan 7, 2008, at 4:40 PM, Paul Gortmaker wrote:

> The PCI exclude device for the sbc85xx boards was only filtering out
> the PHB and nothing else.  This functionality is no longer required
> at a board specific level -- it is handled as a more global quirk now.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Can you respin the full patch set with the update .dts to v1.

- k

^ permalink raw reply

* Re: [PATCH/RFC] mpc83xx/85xx SysRQ/brk over 8250 console
From: Kumar Gala @ 2008-01-08  6:11 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: Linuxppc-dev
In-Reply-To: <20080108052743.GA18083@windriver.com>


On Jan 7, 2008, at 11:27 PM, Paul Gortmaker wrote:

> It seems that if a break is rec'd on the serial console (as per SysRQ
> or similar) on some 83xx and 85xx processors, then a pulse of IRQs is
> generated which makes the use of SysRQ itself about 99% impossible.  I
> experimented with trying several ACK strategies, but in the end the
> thing which worked best was just to wait for the surge of events to  
> pass
> (a fraction of a second).  The number of events can be on the order of
> 1000 (as reported by /proc/interrupts)
>
> I really dislike seeing board specific ifdefs within what should be
> board independent code, so I'm hoping that once the problem is known,
> that a more elegant solution will pop into someone's head.  Or at  
> least
> this will hopefully save someone the grief of re-investigating the  
> source
> and start a discussion.
>
> I've seen this on two different 834x boards and also an 8548 based
> board.  At this point, it is not clear to me if the problem extends
> beyond these CPU/soc to all with UARTs at 4500/4600.
>
> While less than ideal, the work-around below which simply ignores the
> events does allow a person to use SysRQ on such platforms.
>
> The definition of the RFE bit (Rx FIFO error) can be found in pretty
> much any of the MPC CPU PDFs (for those with 4500/4600 16550s).
>
> Paul.

This should really get posted to linux-kernel@vger.kernel.org and  
probably linux-serial@vger.kernel.org lists.

- k

^ permalink raw reply

* Re: How complete should the DTS be?
From: Kumar Gala @ 2008-01-08  6:04 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <4782DAD8.1080104@pikatech.com>


On Jan 7, 2008, at 8:07 PM, Sean MacLennan wrote:

> Just a general question about DTS "completeness". Like all 440EP
> processors, the taco has two i2c buses. However, only one bus has
> anything connected to it.
>
> Should I show both bus entries in the DTS, or only the one that is  
> used?
> I have generally only been showing the devices that are present. i.e.
> Only one emac, only one serial port.
>
> Is there a convention for this?


The .dts should reflect the HW as its used.  On some reference boards  
we might put out more info because of the various configs these types  
of boards can be setup in.  However if something has a static config  
just describe that.  So in your example of two i2c buses with only one  
connected, just describe the one that is used.

- k

^ permalink raw reply

* Re: [PATCH] i2c-ibm_iic driver - new patch
From: Sean MacLennan @ 2008-01-08  5:56 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev, Stefan Roese, Arnd Bergmann
In-Reply-To: <20080108155240.503ca3ab.sfr@canb.auug.org.au>

Stephen Rothwell wrote:
> Hi Sean,
>
> On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <smaclennan@pikatech.com> wrote:
>   
>
> Please don't post patches as attachments.
>   
Ok.
>   
>> +static int __devinit iic_probe(struct of_device *ofdev,
>> +							   const struct of_device_id *match)
>>     
>
> Indenting could be better.
>   
Sorry. Since this kernel is in a "work" directory and not in 
/usr/src/linux* my rules for deciding on the tab size didn't work :( I 
tried to correct the tabbing.
> Please split the assignments from the tests.  Here and elsewhere.
>
>   
I made the changes in my code. I am trying to leave the original code as 
much as possible.
>> +		printk(KERN_CRIT "ibm-iic: failed to allocate device data\n");
>>     
>
> I am not sure that these messages are necessary and, even if so, not KERN_CRIT.
>
>   
What would you recommend then? KERN_ERR? These are cut and paste from 
the original driver, so I left them alone. I will try KERN_ERR.
>> +		}
>> +	} else {
>> +		if (dev->irq != NO_IRQ){
>> +		    iic_interrupt_mode(dev, 0);
>> +		    free_irq(dev->irq, dev);
>> +		}
>> +		iounmap(dev->vaddr);
>> +		kfree(dev);
>>     
>
> Should these last two be after the below brace?
>
>   
I'm not really qualified to answer, but I will anyway ;) I assume the 
original author is basically saying if he cannot delete the adapter, it 
is unsafe to free the memory since the i2c code may still use it. If I 
have read that right, then I agree.

Cheers,
    Sean

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index c466c6c..e9e1493 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -241,7 +241,6 @@ config I2C_PIIX4
 
 config I2C_IBM_IIC
     tristate "IBM PPC 4xx on-chip I2C interface"
-    depends on IBM_OCP
     help
       Say Y here if you want to use IIC peripheral found on
       embedded IBM PPC 4xx based systems.
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c 
b/drivers/i2c/busses/i2c-ibm_iic.c
index 9b43ff7..d218a3b 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -6,6 +6,9 @@
  * Copyright (c) 2003, 2004 Zultys Technologies.
  * Eugene Surovegin <eugene.surovegin@zultys.com> or <ebs@ebshome.net>
  *
+ * Copyright (c) 2008 PIKA Technologies
+ * Sean MacLennan <smaclennan@pikatech.com>
+ *
  * Based on original work by
  *     Ian DaSilva  <idasilva@mvista.com>
  *      Armin Kuster <akuster@mvista.com>
@@ -39,12 +42,17 @@
 #include <asm/io.h>
 #include <linux/i2c.h>
 #include <linux/i2c-id.h>
+
+#ifdef CONFIG_IBM_OCP
 #include <asm/ocp.h>
 #include <asm/ibm4xx.h>
+#else
+#include <linux/of_platform.h>
+#endif
 
 #include "i2c-ibm_iic.h"
 
-#define DRIVER_VERSION "2.1"
+#define DRIVER_VERSION "2.2"
 
 MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION);
 MODULE_LICENSE("GPL");
@@ -650,13 +658,14 @@ static inline u8 iic_clckdiv(unsigned int opb)
     opb /= 1000000;
    
     if (opb < 20 || opb > 150){
-        printk(KERN_CRIT "ibm-iic: invalid OPB clock frequency %u MHz\n",
+        printk(KERN_WARNING "ibm-iic: invalid OPB clock frequency %u 
MHz\n",
             opb);
         opb = opb < 20 ? 20 : 150;
     }
     return (u8)((opb + 9) / 10 - 1);
 }
 
+#ifdef CONFIG_IBM_OCP
 /*
  * Register single IIC interface
  */
@@ -672,7 +681,7 @@ static int __devinit iic_probe(struct ocp_device *ocp){
             ocp->def->index);
 
     if (!(dev = kzalloc(sizeof(*dev), GFP_KERNEL))) {
-        printk(KERN_CRIT "ibm-iic%d: failed to allocate device data\n",
+        printk(KERN_ERR "ibm-iic%d: failed to allocate device data\n",
             ocp->def->index);
         return -ENOMEM;
     }
@@ -687,7 +696,7 @@ static int __devinit iic_probe(struct ocp_device *ocp){
     }
 
     if (!(dev->vaddr = ioremap(ocp->def->paddr, sizeof(struct iic_regs)))){
-        printk(KERN_CRIT "ibm-iic%d: failed to ioremap device registers\n",
+        printk(KERN_ERR "ibm-iic%d: failed to ioremap device registers\n",
             dev->idx);
         ret = -ENXIO;
         goto fail2;
@@ -746,7 +755,7 @@ static int __devinit iic_probe(struct ocp_device *ocp){
     adap->nr = dev->idx >= 0 ? dev->idx : 0;
 
     if ((ret = i2c_add_numbered_adapter(adap)) < 0) {
-        printk(KERN_CRIT "ibm-iic%d: failed to register i2c adapter\n",
+        printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
             dev->idx);
         goto fail;
     }
@@ -779,7 +788,7 @@ static void __devexit iic_remove(struct ocp_device *ocp)
     struct ibm_iic_private* dev = (struct 
ibm_iic_private*)ocp_get_drvdata(ocp);
     BUG_ON(dev == NULL);
     if (i2c_del_adapter(&dev->adap)){
-        printk(KERN_CRIT "ibm-iic%d: failed to delete i2c adapter :(\n",
+        printk(KERN_ERR "ibm-iic%d: failed to delete i2c adapter :(\n",
             dev->idx);
         /* That's *very* bad, just shutdown IRQ ... */
         if (dev->irq >= 0){
@@ -831,3 +840,186 @@ static void __exit iic_exit(void)
 
 module_init(iic_init);
 module_exit(iic_exit);
+#else
+/*
+ * Register single IIC interface
+ */
+static int __devinit iic_probe(struct of_device *ofdev,
+                   const struct of_device_id *match)
+{
+    static int index = 0;
+    struct device_node *np = ofdev->node;
+    struct ibm_iic_private* dev;
+    struct i2c_adapter* adap;
+    const u32 *indexp, *freq;
+    int ret;
+
+    dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+    if (!dev) {
+        printk(KERN_ERR "ibm-iic: failed to allocate device data\n");
+        return -ENOMEM;
+    }
+
+    /* This assumes we don't mix index and non-index entries. */
+    indexp = of_get_property(np, "index", NULL);
+    dev->idx = indexp ? *indexp : index++;
+
+    dev_set_drvdata(&ofdev->dev, dev);
+
+    dev->vaddr = of_iomap(np, 0);
+    if (dev->vaddr == NULL) {
+        printk(KERN_ERR "ibm-iic%d: failed to ioremap device registers\n",
+            dev->idx);
+        ret = -ENXIO;
+        goto fail1;
+    }
+
+    init_waitqueue_head(&dev->wq);
+
+    if (iic_force_poll)
+        dev->irq = NO_IRQ;
+    else if ((dev->irq = irq_of_parse_and_map(np, 0)) == NO_IRQ)
+        printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
+    else {
+        /* Disable interrupts until we finish initialization,
+           assumes level-sensitive IRQ setup...
+         */
+        iic_interrupt_mode(dev, 0);
+        if (request_irq(dev->irq, iic_handler, 0, "IBM IIC", dev)){
+            printk(KERN_ERR "ibm-iic%d: request_irq %d failed\n",
+                   dev->idx, dev->irq);
+            /* Fallback to the polling mode */
+            dev->irq = NO_IRQ;
+        }
+    }
+
+    if (dev->irq == NO_IRQ)
+        printk(KERN_WARNING "ibm-iic%d: using polling mode\n",
+               dev->idx);
+
+    /* Board specific settings */
+    if (iic_force_fast || of_get_property(np, "fast-mode", NULL))
+        dev->fast_mode = 1;
+    else
+        dev->fast_mode = 0;
+
+    /* clckdiv is the same for *all* IIC interfaces, but I'd rather
+     * make a copy than introduce another global. --ebs
+     */
+    freq = of_get_property(np, "clock-frequency", NULL);
+    if (freq == NULL) {
+        freq = of_get_property(np->parent, "clock-frequency", NULL);
+        if (freq == NULL) {
+            printk(KERN_ERR "ibm-iic%d: Unable to get bus frequency\n",
+                   dev->idx);
+            ret = -EBUSY;
+            goto fail;
+        }
+    }
+
+    dev->clckdiv = iic_clckdiv(*freq);
+    DBG("%d: clckdiv = %d\n", dev->idx, dev->clckdiv);
+
+    /* Initialize IIC interface */
+    iic_dev_init(dev);
+
+    /* Register it with i2c layer */
+    adap = &dev->adap;
+    adap->dev.parent = &ofdev->dev;
+    strcpy(adap->name, "IBM IIC");
+    i2c_set_adapdata(adap, dev);
+    adap->id = I2C_HW_OCP;
+    adap->class = I2C_CLASS_HWMON;
+    adap->algo = &iic_algo;
+    adap->client_register = NULL;
+    adap->client_unregister = NULL;
+    adap->timeout = 1;
+    adap->retries = 1;
+    adap->nr = dev->idx;
+
+    ret = i2c_add_numbered_adapter(adap);
+    if (ret  < 0) {
+        printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
+            dev->idx);
+        goto fail;
+    }
+
+    printk(KERN_INFO "ibm-iic%d: using %s mode\n", dev->idx,
+        dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
+
+    return 0;
+
+fail:
+    if (dev->irq != NO_IRQ){
+        iic_interrupt_mode(dev, 0);
+        free_irq(dev->irq, dev);
+    }
+
+    iounmap(dev->vaddr);
+fail1:
+    dev_set_drvdata(&ofdev->dev, NULL);
+    kfree(dev);
+    return ret;
+}
+
+/*
+ * Cleanup initialized IIC interface
+ */
+static int __devexit iic_remove(struct of_device *ofdev)
+{
+    struct ibm_iic_private* dev = dev_get_drvdata(&ofdev->dev);
+
+    BUG_ON(dev == NULL);
+    if (i2c_del_adapter(&dev->adap)){
+        printk(KERN_ERR "ibm-iic%d: failed to delete i2c adapter :(\n",
+            dev->idx);
+        /* That's *very* bad, just shutdown IRQ ... */
+        if (dev->irq != NO_IRQ){
+            iic_interrupt_mode(dev, 0);
+            free_irq(dev->irq, dev);
+            dev->irq = NO_IRQ;
+        }
+    } else {
+        if (dev->irq != NO_IRQ){
+            iic_interrupt_mode(dev, 0);
+            free_irq(dev->irq, dev);
+        }
+        iounmap(dev->vaddr);
+        kfree(dev);
+    }
+
+    return 0;
+}
+
+
+static const struct of_device_id ibm_iic_match[] =
+{
+    { .type = "i2c", .compatible = "ibm,iic-405ex", },
+    { .type = "i2c", .compatible = "ibm,iic-405gp", },
+    { .type = "i2c", .compatible = "ibm,iic-440gp", },
+    { .type = "i2c", .compatible = "ibm,iic-440gpx", },
+    { .type = "i2c", .compatible = "ibm,iic-440grx", },
+    {}
+};
+
+static struct of_platform_driver ibm_iic_driver =
+{
+    .name   = "ibm-iic",
+    .match_table = ibm_iic_match,
+    .probe  = iic_probe,
+    .remove = iic_remove,
+};
+
+static int __init ibm_iic_init(void)
+{
+    printk(KERN_INFO "IBM IIC driver v" DRIVER_VERSION "\n");
+    return of_register_platform_driver(&ibm_iic_driver);
+}
+module_init(ibm_iic_init);
+
+static void __exit ibm_iic_exit(void)
+{
+    of_unregister_platform_driver(&ibm_iic_driver);
+}
+module_exit(ibm_iic_exit);
+#endif

^ permalink raw reply related

* RE: [PATCH] add MPC837x MDS board default device tree
From: Li Yang @ 2008-01-08  5:49 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev list, David Gibson
In-Reply-To: <026A8588-DC44-4EBB-885B-6D70117BF1A1@kernel.crashing.org>

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org=20
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org]=20
> On Behalf Of Kumar Gala
> Sent: Tuesday, January 08, 2008 1:46 PM
> To: Li Yang
> Cc: linuxppc-dev list; David Gibson
> Subject: Re: [PATCH] add MPC837x MDS board default device tree

{snip}

>=20
> Any updates on new versions of the .dts for mpc837x?

I have just posted them yesterday.

- Leo

^ permalink raw reply

* Re: [PATCH] add MPC837x MDS board default device tree
From: Kumar Gala @ 2008-01-08  5:46 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev list, David Gibson
In-Reply-To: <4A496992-9B46-427D-8F40-447E92F8CA3D@kernel.crashing.org>


On Dec 14, 2007, at 2:12 AM, Kumar Gala wrote:

>
> On Dec 6, 2007, at 8:01 PM, David Gibson wrote:
>
>> On Wed, Dec 05, 2007 at 06:37:53PM +0800, Li Yang wrote:
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> ---
>>> Update SATA nodes; remove serdes nodes; add aliases and labels.
>>>
>>> arch/powerpc/boot/dts/mpc8377_mds.dts |  270 +++++++++++++++++++++++
>>> ++++++++
>>> arch/powerpc/boot/dts/mpc8378_mds.dts |  256 +++++++++++++++++++++++
>>> ++++++
>>> arch/powerpc/boot/dts/mpc8379_mds.dts |  284 +++++++++++++++++++++++
>>> ++++++++++
>>> 3 files changed, 810 insertions(+), 0 deletions(-)
>>> create mode 100644 arch/powerpc/boot/dts/mpc8377_mds.dts
>>> create mode 100644 arch/powerpc/boot/dts/mpc8378_mds.dts
>>> create mode 100644 arch/powerpc/boot/dts/mpc8379_mds.dts
>>>
>>> diff --git a/arch/powerpc/boot/dts/mpc8377_mds.dts b/arch/powerpc/
>>> boot/dts/mpc8377_mds.dts
>>> new file mode 100644
>>> index 0000000..919ffd0
>>> --- /dev/null
>>> +++ b/arch/powerpc/boot/dts/mpc8377_mds.dts
>> [snip]
>>> +	aliases {
>>> +		ethernet0 = "/soc@e0000000/ethernet@24000";
>>> +		ethernet1 = "/soc@e0000000/ethernet@25000";
>>> +		serial0 = "/soc@e0000000/serial@4500";
>>> +		serial1 = "/soc@e0000000/serial@4600";
>>> +		pci0 = "/pci@e0008500";
>>
>> You can use path references for these now.
>>
>>> +	};
>>> +
>>> +	cpus {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		PowerPC,837x@0 {
>>
>> This should absolutely not have a "x" in it.  Either have an exact
>> model, or just use "cpu@9" (in which case you can put the model in
>> "compatible").
>>
>> [snip]
>>> +		i2c@3000 {
>>> +			device_type = "i2c";
>>
>> Drop the device_type.  No "i2c" device_type class is defined.
>>
>> [snip]
>>> +		/* phy type (ULPI, UTMI, UTMI_WIDE, SERIAL) */
>>> +		usb@23000 {
>>> +			device_type = "usb";
>>
>> Drop device_type here too.
>>
>>> +			compatible = "fsl-usb2-dr";
>>> +			reg = <23000 1000>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +			interrupt-parent = < &ipic >;
>>> +			interrupts = <26 8>;
>>> +			phy_type = "utmi_wide";
>>> +		};
>>> +
>>> +		mdio@24520 {
>>> +			device_type = "mdio";
>>> +			compatible = "gianfar";
>>
>> Grr... not your fault, but this crap in the gianfar driver where it
>> uses the same compatible property for the mdio and MAC has to be
>> fixed.
>
> I've fixed this now.  Look at my tree/patch.
>
>> [snip]
>>
>>> +		serial1:serial@4600 {
>>
>> Standard style puts a space after the colon.
>>
>>> +			device_type = "serial";
>>> +			compatible = "ns16550";
>>> +			reg = <4600 100>;
>>> +			clock-frequency = <0>;
>>> +			interrupts = <a 8>;
>>> +			interrupt-parent = < &ipic >;
>>> +		};
>>> +
>>> +		crypto@30000 {
>>> +			model = "SEC3";
>>> +			compatible = "talitos";
>>
>> This driver, also, needs fixing to recognize a better formatted
>> compatible property.
>
> Can you respin with David's changes as well as mirror my cleanup of
> the other 83xx/85xx .dts


Any updates on new versions of the .dts for mpc837x?

- k

^ permalink raw reply

* [PATCH/RFC] mpc83xx/85xx SysRQ/brk over 8250 console
From: Paul Gortmaker @ 2008-01-08  5:27 UTC (permalink / raw)
  To: Linuxppc-dev

It seems that if a break is rec'd on the serial console (as per SysRQ
or similar) on some 83xx and 85xx processors, then a pulse of IRQs is
generated which makes the use of SysRQ itself about 99% impossible.  I
experimented with trying several ACK strategies, but in the end the
thing which worked best was just to wait for the surge of events to pass
(a fraction of a second).  The number of events can be on the order of
1000 (as reported by /proc/interrupts)

I really dislike seeing board specific ifdefs within what should be
board independent code, so I'm hoping that once the problem is known,
that a more elegant solution will pop into someone's head.  Or at least
this will hopefully save someone the grief of re-investigating the source
and start a discussion.

I've seen this on two different 834x boards and also an 8548 based
board.  At this point, it is not clear to me if the problem extends
beyond these CPU/soc to all with UARTs at 4500/4600.

While less than ideal, the work-around below which simply ignores the
events does allow a person to use SysRQ on such platforms.

The definition of the RFE bit (Rx FIFO error) can be found in pretty
much any of the MPC CPU PDFs (for those with 4500/4600 16550s).

Paul.

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index f94109c..2761c64 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1426,6 +1426,24 @@ serial8250_handle_port(struct uart_8250_port *up)
 
 	status = serial_inp(up, UART_LSR);
 
+#if defined(CONFIG_MPC834x) || defined(CONFIG_MPC8540)
+	/*
+	 * There appears to be a quirk in the implementation on some 8xxx
+	 * where after a break is rec'd (UART_LSR_BI), the UART generates
+	 * a short duration burst of bogus IRQ events with the signature
+	 * of RFE set (along with "normal" bits set) in the LSR.
+	 */
+
+#define RFE_8xxx_ERR_BITS (	UART_LSR_RFE	| UART_LSR_TEMT	| \
+				UART_LSR_THRE	| UART_LSR_BI	| \
+				UART_LSR_DR	)
+
+	if (status == RFE_8xxx_ERR_BITS) {
+		spin_unlock_irqrestore(&up->port.lock, flags);
+		return;
+	}
+#endif
+
 	DEBUG_INTR("status = %x...", status);
 
 	if (status & UART_LSR_DR)
diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
index 96c0d93..1ea6436 100644
--- a/include/linux/serial_reg.h
+++ b/include/linux/serial_reg.h
@@ -111,6 +111,7 @@
 #define UART_MCR_DTR		0x01 /* DTR complement */
 
 #define UART_LSR	5	/* In:  Line Status Register */
+#define UART_LSR_RFE		0x80 /* Rx FIFO Error (BE, FE, or PE) */
 #define UART_LSR_TEMT		0x40 /* Transmitter empty */
 #define UART_LSR_THRE		0x20 /* Transmit-hold-register empty */
 #define UART_LSR_BI		0x10 /* Break interrupt indicator */

^ permalink raw reply related

* Re: [PATCH] i2c-ibm_iic driver - new patch
From: Stephen Rothwell @ 2008-01-08  4:52 UTC (permalink / raw)
  To: Sean MacLennan; +Cc: linuxppc-dev, Stefan Roese, Arnd Bergmann
In-Reply-To: <4782D9E0.4070600@pikatech.com>

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

Hi Sean,

On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <smaclennan@pikatech.com> wrote:
>

Please don't post patches as attachments.

> +static int __devinit iic_probe(struct of_device *ofdev,
> +							   const struct of_device_id *match)

Indenting could be better.

> +{

> +	if (!(dev = kzalloc(sizeof(*dev), GFP_KERNEL))) {

Please split the assignments from the tests.  Here and elsewhere.

> +		printk(KERN_CRIT "ibm-iic: failed to allocate device data\n");

I am not sure that these messages are necessary and, even if so, not KERN_CRIT.

> +	if(iic_force_poll)

Space after "if"

> +	if (dev->irq != NO_IRQ) {
	.
	.
> +	}
> +
> +	if (dev->irq == NO_IRQ)

	else instead?

> +		printk(KERN_WARNING "ibm-iic%d: using polling mode\n",
> +			   dev->idx);
> +static int __devexit iic_remove(struct of_device *ofdev)
> +{
> +	struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_get_drvdata(&ofdev->dev);

Unnecessary cast.

> +	if (i2c_del_adapter(&dev->adap)){
> +		printk(KERN_CRIT "ibm-iic%d: failed to delete i2c adapter :(\n",
> +			dev->idx);

This is not a KERN_CRIT situation ...

> +		/* That's *very* bad, just shutdown IRQ ... */
> +		if (dev->irq >= 0){

What is that testing? For NO_IRQ as below?

> +		    iic_interrupt_mode(dev, 0);
> +		    free_irq(dev->irq, dev);
> +		    dev->irq = -1;

NO_IRQ?

> +		}
> +	} else {
> +		if (dev->irq != NO_IRQ){
> +		    iic_interrupt_mode(dev, 0);
> +		    free_irq(dev->irq, dev);
> +		}
> +		iounmap(dev->vaddr);
> +		kfree(dev);

Should these last two be after the below brace?

> +	}
> +
> +	return 0;
> +}
> +
> +
> +static struct of_device_id ibm_iic_match[] =

This should be const.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: How to write a device driver in such a fashion .????
From: Misbah khan @ 2008-01-08  4:31 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <14660018.post@talk.nabble.com>


using mmap() in application to access the h/w is only possible if  the device
memory is mapped in the kernel virtual memory how come we directly map to
the physical memory to virtual from user space ??? 

----Misbah 

Misbah khan wrote:
> 
> Hi all ....
> 
> I need your suggession to write a driver with such requirements :-
> 
> On Initiation by the application of which key is pressed driver should
> perform the task accordingly. There are 7 keys so seven different task the
> driver will perform. 
> 
> 1 . I do not want to use any entry point such as read/write/ioctl. In such
> a case how to access the driver.
> 2. We dont want to implement this as a chracrter driver.
> 3. The driver shall look pretty much like an application where in
> asscssing the H/W memory map .
> 
> Hence no entry point and no registeration but an application could be able
> to communicate ....
> 
> I need your suggession in this regard .....
> 
> -----Misbah  
> 

-- 
View this message in context: http://www.nabble.com/How-to-write-a-device-driver-in-such-a-fashion-.-----tp14660018p14682271.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH 7/8] pseries: phyp dump: Unregister and print dump areas.
From: Stephen Rothwell @ 2008-01-08  4:25 UTC (permalink / raw)
  To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, lkessler, strosake
In-Reply-To: <4782C5CB.4080802@austin.ibm.com>

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

Hi Manish,

Just more trivial stuff.

On Mon, 07 Jan 2008 18:37:31 -0600 Manish Ahuja <ahuja@austin.ibm.com> wrote:
>
>  static void register_dump_area(struct phyp_dump_header *ph, unsigned long addr)
>  {
>  	int rc;
> -	ph->cpu_data.destination_address += addr;
> -	ph->hpte_data.destination_address += addr;
> -	ph->kernel_data.destination_address += addr;
>  
> +	/* Add addr value if not initialized before */
> +	if (ph->cpu_data.destination_address == 0) {
> +		ph->cpu_data.destination_address += addr;

Could be just '=' like further down, right?

> +static void invalidate_last_dump(struct phyp_dump_header *ph, unsigned long addr)
> +{

> +	if (rc)
> +	{

We prefer
	if (rc) {

> +static void unregister_dump_area(struct phyp_dump_header *ph)
> +{

> +	if (rc)
> +	{

And again.

>  static ssize_t
>  show_release_region(struct kset * kset, char *buf)

We normally put the function name on the same line as the type.

>  {
> -	return sprintf(buf, "ola\n");
> +	u64 second_addr_range;
> +
> +	/* total reserved size - start of scratch area */
> +	second_addr_range = phdr.cpu_data.destination_address -
> +				phyp_dump_info->init_reserve_size;
> +	return sprintf(buf, "CPU:0x%lx-0x%lx: HPTE:0x%lx-0x%lx:"
> +			    " DUMP:0x%lx-0x%lx, 0x%lx-0x%lx:\n",
> +	  phdr.cpu_data.destination_address, phdr.cpu_data.length_copied,
> +	  phdr.hpte_data.destination_address, phdr.hpte_data.length_copied,
> +	  phdr.kernel_data.destination_address, phdr.kernel_data.length_copied,
> +	  phyp_dump_info->init_reserve_start, second_addr_range);

This indentation should be (probably) two tabs.

> +	/* re-register the dump area, if old dump was invalid */
> +	if ((dump_header) && (dump_header->status & DUMP_ERROR_FLAG)) {
            ^           ^
Extra parentheses.

> +		invalidate_last_dump (&phdr, dump_area_start);
> +		register_dump_area (&phdr, dump_area_start);

No spaces after function names.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH 6/8] pseries: phyp dump: debugging print routines.
From: Stephen Rothwell @ 2008-01-08  4:03 UTC (permalink / raw)
  To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, lkessler, strosake
In-Reply-To: <4782C53D.4060506@austin.ibm.com>

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

Hi Manish,

Just a trivial comment.

On Mon, 07 Jan 2008 18:35:09 -0600 Manish Ahuja <ahuja@austin.ibm.com> wrote:
>
> +	printk(KERN_INFO "dump disk section = %d\n",ph->dump_disk_section);

Please put a space after the ','.  Here and later.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH][POWERPC] Workaround for iommu page alignment (#2)
From: Benjamin Herrenschmidt @ 2008-01-08  4:02 UTC (permalink / raw)
  To: Olof Johansson; +Cc: linuxppc-dev list, Paul Mackerras, Linux Kernel list
In-Reply-To: <20080108034824.GA7983@lixom.net>

> And sloppy of me to not catch it. Anyway:
> 
> Acked-by: Olof Johansson <olof@lixom.net>
> 
> I wonder how long until there's a device that has some other < PAGE_SIZE
> alignment bug^Wrequirement that we'll need to meet too. :(

Yeah, it's a worry...

Ben.

^ permalink raw reply

* Re: [PATCH 5/8] pseries: phyp dump: register dump area.
From: Stephen Rothwell @ 2008-01-08  3:59 UTC (permalink / raw)
  To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, lkessler, strosake
In-Reply-To: <4782C3AE.8060202@austin.ibm.com>

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

Hi Manish,

On Mon, 07 Jan 2008 18:28:30 -0600 Manish Ahuja <ahuja@austin.ibm.com> wrote:
>
> +++ linux-2.6.24-rc3-git1/arch/powerpc/platforms/pseries/phyp_dump.c	2007-11-21 16:06:52.000000000 -0600
> +static unsigned long init_dump_header(struct phyp_dump_header *ph)
> +{

> +	/* Get the required dump region sizes */
> +	rtas = of_find_node_by_path("/rtas");

You need to of_node_put(rtas) somewhere.

> +	if (sizes[0] == 1)
> +		cpu_state_size = *((unsigned long *) &sizes[1]);

We normally don't put spaces after casts.

> +	ph->first_offset_section =
> +		(u32) &(((struct phyp_dump_header *) 0)->cpu_data);

		(u32)offsetof(struct phyp_dump_header, cpu_data);

> +static void register_dump_area(struct phyp_dump_header *ph, unsigned long addr)
> +{

> +	if (rc)
> +	{
> +		printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n", rc);
> +	}

The braces are not needed.

> +
> +	dump_area_length = init_dump_header (&phdr);

We don't put spaces after function names.

> +	free_area_length = phyp_dump_info->init_reserve_size - dump_area_length;
> +	dump_area_start = phyp_dump_info->init_reserve_start + free_area_length;
> +	dump_area_start = dump_area_start & PAGE_MASK; /* align down */
> +	free_area_length = dump_area_start - phyp_dump_info->init_reserve_start;
> +
>  	if (dump_header == NULL) {
> -		release_all();
> -		return 0;
> +		register_dump_area (&phdr, dump_area_start);

Ditto.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH 4/8] pseries: phyp dump: use sysfs to release reserved mem
From: Stephen Rothwell @ 2008-01-08  3:45 UTC (permalink / raw)
  To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, lkessler, strosake
In-Reply-To: <4782C225.3060305@austin.ibm.com>

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

On Mon, 07 Jan 2008 18:21:57 -0600 Manish Ahuja <ahuja@austin.ibm.com> wrote:
>
> +static int __init phyp_dump_setup(void)
> +{
>
> +	/* Is there dump data waiting for us? */
> +	rtas = of_find_node_by_path("/rtas");
> +	dump_header = of_get_property(rtas, "ibm,kernel-dump", &header_len);
                                                               ^^^^^^^^^^^
You could pass NULL here as header_len appears to be unused. Also you
need "of_node_put(rtas)" somewhere (probably just here would do).

> +	if (dump_header == NULL) {
> +		release_all();
> +		return 0;
> +	}
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH][POWERPC] Workaround for iommu page alignment (#2)
From: Olof Johansson @ 2008-01-08  3:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev list, Paul Mackerras, Linux Kernel list
In-Reply-To: <1199748862.6734.20.camel@pasglop>

On Tue, Jan 08, 2008 at 10:34:22AM +1100, Benjamin Herrenschmidt wrote:
> powerpc: Workaround for iommu page alignment
> 
> Our iommu page size is currently always 4K. That means with our current
> code, drivers may do a dma_map_sg() of a 64K page and obtain a dma_addr_t
> that is only 4K aligned.
> 
> This works fine in most cases except some infiniband HW it seems, where
> they tell the HW about the page size and it ignores the low bits of the
> DMA address.
> 
> This works around it by making our IOMMU code enforce a PAGE_SIZE alignment
> for mappings of objects that are page aligned in the first place and whose
> size is larger or equal to a page.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> And this version actually does what the comment says (I had forgotten
> to quilt ref... a common mistake).

And sloppy of me to not catch it. Anyway:

Acked-by: Olof Johansson <olof@lixom.net>

I wonder how long until there's a device that has some other < PAGE_SIZE
alignment bug^Wrequirement that we'll need to meet too. :(


-Olof

^ permalink raw reply

* RE: [PATCH 3/3] USB device tree cleanups
From: Li Yang @ 2008-01-08  3:32 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20080108101028.db21d18a.sfr@canb.auug.org.au>

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org=20
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org]=20
> On Behalf Of Stephen Rothwell
> Sent: Tuesday, January 08, 2008 7:10 AM
> To: Li Yang
> Cc: linuxppc-dev@ozlabs.org; paulus@samba.org
> Subject: Re: [PATCH 3/3] USB device tree cleanups
>=20
> On Mon,  7 Jan 2008 20:03:20 +0800 Li Yang=20
> <leoli@freescale.com> wrote:
> >
> > +++ b/arch/powerpc/sysdev/fsl_soc.c
> > @@ -533,9 +533,7 @@ static int __init fsl_usb_of_init(void)
> >  		*usb_dev_dr_client =3D NULL;
> >  	int ret;
> > =20
> > -	for (np =3D NULL, i =3D 0;
> > -	     (np =3D of_find_compatible_node(np, "usb",=20
> "fsl-usb2-mph")) !=3D NULL;
> > -	     i++) {
> > +	for_each_compatible_node(np, NULL, "fsl-usb2-mph") {
>=20
> 'i' is no longer being updated (or indeed set to anything)=20
> but is still used in the loop - at least in Paulus' current tree).

Good catch. My careless mistake.  Thanks.

- Leo

^ permalink raw reply

* Re: [PATCH 2/8] pseries: phyp dump: config file
From: Stephen Rothwell @ 2008-01-08  3:18 UTC (permalink / raw)
  To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, lkessler, strosake
In-Reply-To: <4782C0C8.2050702@austin.ibm.com>

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

On Mon, 07 Jan 2008 18:16:08 -0600 Manish Ahuja <ahuja@austin.ibm.com> wrote:
>
> 
> Add hypervisor-assisted dump to kernel config
> 
> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

This patch should probably come last in the series so that all the code
is there before some bisecting autobuilder tries to configure it.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH 3/8] pseries: phyp dump: reserve-release proof-of-concept
From: Stephen Rothwell @ 2008-01-08  3:16 UTC (permalink / raw)
  To: Manish Ahuja; +Cc: mahuja, linuxppc-dev, linasvepstas, lkessler, strosake
In-Reply-To: <4782C2FB.7020105@austin.ibm.com>

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

Hi Manish,

Just some trivial things ...

On Mon, 07 Jan 2008 18:25:31 -0600 Manish Ahuja <ahuja@austin.ibm.com> wrote:
>
> 
> +++ linux-2.6.24-rc2-git4/include/asm-powerpc/phyp_dump.h	2007-11-19 17:44:21.000000000 -0600
> +#ifndef _PPC64_PHYP_DUMP_H

We more usually use _ASM_POWERPC_PHYP_DUMP_H

> +#define _PPC64_PHYP_DUMP_H
> +
> +#ifdef CONFIG_PHYP_DUMP

Do these things really need protecting by this CONFIG variable? i.e. does
anything change depending on the visibility of these various symbols?

> +++ linux-2.6.24-rc2-git4/arch/powerpc/platforms/pseries/phyp_dump.c	2007-11-19 19:07:49.000000000 -0600
> @@ -0,0 +1,71 @@
> +/*
> + * Hypervisor-assisted dump
> + *
> + * Linas Vepstas, Manish Ahuja 2007
> + * Copyrhgit (c) 2007 IBM Corp.
      ^^^^^^^^^
typo.  and you should really be using '©' in new copyright notices (or
nothing).

> +/**
> + * release_memory_range -- release memory previously lmb_reserved
> + * @start_pfn: starting physical frame number
> + * @nr_pages: number of pages to free.
> + *
> + * This routine will release memory that had been previously
> + * lmb_reserved in early boot. The released memory becomes
> + * available for genreal use.
                    ^^^^^^^
typo.

> +release_memory_range(unsigned long start_pfn, unsigned long nr_pages)
> +{
> +	struct page *rpage;
> +	unsigned long end_pfn;
> +	long i;
> +
> +	end_pfn = start_pfn + nr_pages;
> +
> +	for (i=start_pfn; i <= end_pfn; i++) {

spaces around '='

> +static int __init phyp_dump_setup(void)
> +{

> +}
> +
> +subsys_initcall(phyp_dump_setup);

Normally we don't leave a blank line here.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox