2005-10-28 09:52:19

by Richard Purdie

[permalink] [raw]
Subject: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

This code is the final link is getting akita working but I'm not sure
its the right approach. I'm posting this in the hope someone might see a
better way to achieve this driver's objectives. I'd like to get akita
support into mainline and this is the only barrier.

Background:

The Sharp SL-C1000 (Akita) has an 8 port Maxim 7310 IO Expander
connected via I2C. In this specific case, 5 outputs are used from a
variety of linux subsystems such as the backlight, IR port and sound.

Driver Design Criteria:

* GPIOs need to be available fairly early in the boot procedure as we
can't guarantee when the subsystems will use them
* GPIOs can be changed from interrupt context
* GPIO providers need to be one of the last devices suspended/first
resumed

A Solution:

Firstly, I wrote an I2C driver for the MAX7310 which is included below.
This shouldn't be too controversial. It does export some functions and
I'm not sure what to do about a header file for them. Creating a
specific header file for them seems a little over the top. I realise I
need to sort out the I2C_DRIVERID properly. I'm also aware it has a lot
of different ids and no method to detect whether the chip is really
present (Akita uses 0x18 as the slave id).

I2C drivers appear relatively late in the boot procedure and changing
that isn't practical. I therefore ended up writing akita-ioexp which
acts as an interface between the rest of the kernel drivers and the
max7310 i2c driver.

When akita-ioexp loads, the i2c driver isn't likely to be present. I've
used the init levels to ensure it comes up after the i2c-core but that's
about all we can hope for. It therefore searches for the max7310 and if
not found, schedules a repeat of the search for later, repeating until
it finds the device (using a workqueue). The workqueue is also used to
get around gpio requests from interrupt context.

I had to turn akita-ioexp into a platform device to get the suspend
signal which is used to flush the workqueue. With a platform device
available at machine init time, I can insert it as a parent of the corgi
device chain which means its one of the last devices to be suspended. I
couldn't do this with an i2c device as it won't be around at early init.
I do remember reading about the idea of being able to dynamically alter
the device parent chain which would help with this but I don't think
this exists (yet?).

In this case, we only have output gpios and the timing isn't critical.
We can therefore queue the output until the max7310 becomes available.
This wouldn't work for inputs or devices that would have an issue with
delayed output.

Alternatives:

I can see a case for ditching the generic max7310 driver and putting its
code directly into akita-ioexp. If I'd tried this the other way around,
I know what I would have been told :). Anyhow, this solves some issues
and creates others. The main one would be how to arrange for a very late
suspend call as the device wouldn't be available to set as a parent for
the corgi device tree at early boot. Obviously any such driver would
then be totally akita specific.

There is a fundamental problem with the lack of a proper gpio interface
in Linux. Every driver does something different with them (be it pxa
specific gpios, SCOOP gpios, those on a IO expander, those on a video
chip (w100fb springs to mind) to name just the Zaurus specific ones.

Is this patch going to be acceptable and/or is there anything we can do
to improve this driver and/or Linux in general to support these kind of
needs?

Richard

[The full Akita machine patch is available as
http://www.rpsys.net/openzaurus/patches/spitz_base_extras-r4.patch ]

Index: linux-2.6.13/arch/arm/mach-pxa/akita-ioexp.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.13/arch/arm/mach-pxa/akita-ioexp.c 2005-10-27 21:41:13.000000000 +0100
@@ -0,0 +1,118 @@
+/*
+ * Support for the IO Expander on the Sharp SL-C1000 (Akita)
+ *
+ * Copyright 2005 Openedhand Ltd.
+ *
+ * Author: Richard Purdie <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/workqueue.h>
+#include <asm/arch/akita.h>
+
+/* These should be somewhere else */
+int max7310_config(struct device *dev, int iomode, int polarity);
+int max7310_set_ouputs(struct device *dev, int outputs);
+
+static void akita_ioexp_work(void *private_);
+
+static struct device *akita_ioexp_device;
+static unsigned char ioexp_output_value = AKITA_IOEXP_IO_OUT;
+DECLARE_WORK(akita_ioexp, akita_ioexp_work, NULL);
+
+void akita_set_ioexp(struct device *dev, unsigned char bit)
+{
+ ioexp_output_value |= bit;
+
+ if (akita_ioexp_device)
+ schedule_work(&akita_ioexp);
+ return;
+}
+
+void akita_reset_ioexp(struct device *dev, unsigned char bit)
+{
+ ioexp_output_value &= ~bit;
+
+ if (akita_ioexp_device)
+ schedule_work(&akita_ioexp);
+ return;
+}
+
+EXPORT_SYMBOL(akita_set_ioexp);
+EXPORT_SYMBOL(akita_reset_ioexp);
+
+static int is_akita_ioexp_device(struct device *dev, void *data)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ return (strncmp(client->name, "max7310", 7) == 0);
+}
+
+static void akita_ioexp_work(void *private_)
+{
+ if (!akita_ioexp_device) {
+ akita_ioexp_device = bus_find_device(&i2c_bus_type, NULL, NULL, is_akita_ioexp_device);
+
+ if (!akita_ioexp_device) {
+ /* if we can't find the device, wait and retry */
+ schedule_delayed_work(&akita_ioexp, msecs_to_jiffies(500));
+ return;
+ }
+
+ max7310_config(akita_ioexp_device, AKITA_IOEXP_IO_DIR, 0);
+ }
+
+ max7310_set_ouputs(akita_ioexp_device, ioexp_output_value);
+}
+
+
+#ifdef CONFIG_PM
+static int akita_ioexp_suspend(struct device *dev, pm_message_t state, uint32_t level)
+{
+ if (level == SUSPEND_POWER_DOWN) {
+ flush_scheduled_work();
+ }
+ return 0;
+}
+
+static int akita_ioexp_resume(struct device *dev, uint32_t level)
+{
+ if (level == RESUME_POWER_ON) {
+ schedule_work(&akita_ioexp);
+ }
+ return 0;
+}
+#else
+#define akita_ioexp_suspend NULL
+#define akita_ioexp_resume NULL
+#endif
+
+
+static int __init akita_ioexp_probe(struct device *dev)
+{
+ schedule_work(&akita_ioexp);
+ return 0;
+}
+
+static struct device_driver akita_ioexp_driver = {
+ .name = "akita-ioexp",
+ .bus = &platform_bus_type,
+ .probe = akita_ioexp_probe,
+ .suspend = akita_ioexp_suspend,
+ .resume = akita_ioexp_resume,
+};
+
+static int __devinit akita_ioexp_init(void)
+{
+ return driver_register(&akita_ioexp_driver);
+}
+
+module_init(akita_ioexp_init);
+


Index: linux-2.6.13/drivers/i2c/chips/max7310.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.13/drivers/i2c/chips/max7310.c 2005-10-27 21:23:25.000000000 +0100
@@ -0,0 +1,201 @@
+/*
+ * max7310.c -- Maxim MAX7310 8 Port IO Expander
+ *
+ * Copyright 2005 Openedhand Ltd.
+ *
+ * Author: Richard Purdie <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define I2C_DRIVERID_MAX7310 0xdafe
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+
+/* MAX7310 Regiser Map */
+#define MAX7310_INPUT 0x00
+#define MAX7310_OUTPUT 0x01
+#define MAX7310_POLINV 0x02
+#define MAX7310_IODIR 0x03 /* 1 = Input, 0 = Output */
+#define MAX7310_TIMEOUT 0x04
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = {
+ 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+ 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+ 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+ 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
+ 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f,
+ 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f,
+ I2C_CLIENT_END
+};
+
+/* I2C Magic */
+I2C_CLIENT_INSMOD;
+
+static int max7310_write(struct i2c_client *client, int address, int data);
+static int max7310_read(struct i2c_client *client, int address);
+static struct i2c_client max7310_template;
+
+/*
+ * Public Interface
+ */
+int max7310_config(struct device *dev, int iomode, int polarity)
+{
+ int ret;
+ struct i2c_client *client = to_i2c_client(dev);
+
+ ret = max7310_write(client, MAX7310_POLINV, polarity);
+ if (ret < 0)
+ return ret;
+ ret = max7310_write(client, MAX7310_IODIR, iomode);
+ return ret;
+}
+
+int max7310_set_ouputs(struct device *dev, int outputs)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ return max7310_write(client, MAX7310_OUTPUT, outputs);
+}
+
+int max7310_read_inputs(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ return max7310_read(client, MAX7310_INPUT);
+}
+
+EXPORT_SYMBOL(max7310_config);
+EXPORT_SYMBOL(max7310_set_ouputs);
+EXPORT_SYMBOL(max7310_read_inputs);
+
+
+/*
+ * Sysfs Functions
+ */
+static ssize_t show_gpio(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "0x%02x\n", max7310_read_inputs(dev));
+}
+
+static ssize_t set_gpio(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+{
+ unsigned long val = simple_strtoul(buf, NULL, 16);
+
+ if (val > 0xff)
+ return -EINVAL;
+
+ max7310_set_ouputs(dev, val);
+
+ return count;
+}
+
+static DEVICE_ATTR(gpio, S_IWUSR | S_IRUGO, show_gpio, set_gpio);
+
+
+/*
+ * I2C Functions
+ */
+static int max7310_write(struct i2c_client *client, int address, int value)
+{
+ u8 data[2];
+
+ data[0] = address & 0xff;
+ data[1] = value & 0xff;
+
+ if (i2c_master_send(client, data, 2) == 2)
+ return 0;
+ return -1;
+}
+
+static int max7310_read(struct i2c_client *client, int address)
+{
+ u8 data[1];
+
+ data[0] = address & 0xff;
+
+ if (i2c_master_recv(client, data, 1) == 1) {
+ return data[0];
+ }
+ return -1;
+}
+
+static int max7310_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+ struct i2c_client *new_client;
+ int err;
+
+ if (!(new_client = kmalloc(sizeof(struct i2c_client), GFP_KERNEL)))
+ return -ENOMEM;
+
+ max7310_template.adapter = adapter;
+ max7310_template.addr = address;
+
+ memcpy(new_client, &max7310_template, sizeof(struct i2c_client));
+
+ if ((err = i2c_attach_client(new_client))) {
+ kfree(new_client);
+ return err;
+ }
+
+ /* Register sysfs hooks */
+ device_create_file(&new_client->dev, &dev_attr_gpio);
+ return 0;
+}
+
+static int max7310_attach_adapter(struct i2c_adapter *adapter)
+{
+ return i2c_probe(adapter, &addr_data, max7310_detect);
+}
+
+static int max7310_detach_client(struct i2c_client *client)
+{
+ int err;
+
+ device_remove_file(&client->dev, &dev_attr_gpio);
+
+ if ((err = i2c_detach_client(client)))
+ return err;
+
+ kfree(client);
+ return 0;
+}
+
+static struct i2c_driver max7310_i2c_driver = {
+ .owner = THIS_MODULE,
+ .name = "max7310",
+ .id = I2C_DRIVERID_MAX7310,
+ .flags = I2C_DF_NOTIFY,
+ .attach_adapter = max7310_attach_adapter,
+ .detach_client = max7310_detach_client,
+};
+
+static struct i2c_client max7310_template = {
+ name: "max7310",
+ flags: I2C_CLIENT_ALLOW_USE,
+ driver: &max7310_i2c_driver,
+};
+
+static int __init max7310_init(void)
+{
+ return i2c_add_driver(&max7310_i2c_driver);
+}
+
+static void __exit max7310_exit(void)
+{
+ i2c_del_driver(&max7310_i2c_driver);
+}
+
+MODULE_AUTHOR("Richard Purdie <[email protected]>");
+MODULE_DESCRIPTION("MAX7310 driver");
+MODULE_LICENSE("GPL");
+
+module_init(max7310_init);
+module_exit(max7310_exit);
Index: linux-2.6.13/drivers/i2c/chips/Kconfig
===================================================================
--- linux-2.6.13.orig/drivers/i2c/chips/Kconfig 2005-10-25 16:03:08.000000000 +0100
+++ linux-2.6.13/drivers/i2c/chips/Kconfig 2005-10-27 14:47:02.000000000 +0100
@@ -126,4 +126,17 @@
This driver can also be built as a module. If so, the module
will be called max6875.

+config MAX7310
+ tristate "Maxim MAX7310 8 Port IO Exander"
+ depends on I2C && EXPERIMENTAL
+ help
+ If you say yes here you get support for the Maxim MAX7310
+ 8 Port IO Exander.
+
+ This chip is found in certain devices such as the Sharp Zaurus
+ SL-C1000 (Akita).
+
+ This driver can also be built as a module. If so, the module
+ will be called max7310.
+
endmenu
Index: linux-2.6.13/drivers/i2c/chips/Makefile
===================================================================
--- linux-2.6.13.orig/drivers/i2c/chips/Makefile 2005-08-29 00:41:01.000000000 +0100
+++ linux-2.6.13/drivers/i2c/chips/Makefile 2005-10-27 14:47:14.000000000 +0100
@@ -12,6 +12,7 @@
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
obj-$(CONFIG_SENSORS_RTC8564) += rtc8564.o
obj-$(CONFIG_ISP1301_OMAP) += isp1301_omap.o
+obj-$(CONFIG_MAX7310) += max7310.o
obj-$(CONFIG_TPS65010) += tps65010.o

ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)





2005-10-28 16:08:54

by Russell King

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

On Fri, Oct 28, 2005 at 10:52:09AM +0100, Richard Purdie wrote:
> I2C drivers appear relatively late in the boot procedure and changing
> that isn't practical. I therefore ended up writing akita-ioexp which
> acts as an interface between the rest of the kernel drivers and the
> max7310 i2c driver.

You're not the only one with this problem, and this is one reason I've
never submitted the SA11x0 audio drivers. The L3 bus subsystem inter-
links on some platforms with the I2C subsystem (it physically shares
the same signals but it isn't I2C compatible as such.)

Hence, in order to lock I2C off the bus, we have to take the same
driver-based locks as I2C. This can only really happen if I2C is
initialised first.

Below is the complete patch for the L3 support, which includes moving
the I2C initialisation early.

diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N linus/Documentation/l3/structure linux/Documentation/l3/structure
--- linus/Documentation/l3/structure Thu Jan 1 01:00:00 1970
+++ linux/Documentation/l3/structure Sun Aug 5 18:40:00 2001
@@ -0,0 +1,23 @@
+L3 Bus Driver
+-------------
+
+The structure of the L3 subsystem is as follows:
+
+ ^ ^ ^
+ | | |
+ | +-----v----+ +----v-----+
+ | | device | | device |
+ | | driver 1 | | driver 2 |
+ +----v-----+ +-----^----+ +----^-----+
+ | core | | |
+ | services <--------+------------+
+ +----^-----+
+ |
+ +---v----+ +-----------+
+ | bus <---> algorithm |
+ | driver | | driver |
+ +--------+ +-----------+
+
+Applications talk to the core to obtain bus adapters. Applications talk
+to device drivers to perform actions. Device drivers in turn talk to the
+core to perform L3 bus transactions via the bus and algorithm drivers.
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -r orig/arch/arm/Kconfig linux/./arch/arm/Kconfig
--- orig/arch/arm/Kconfig Wed Jun 29 15:49:19 2005
+++ linux/arch/arm/Kconfig Wed Jun 29 16:07:34 2005
@@ -736,7 +736,7 @@ source "drivers/char/Kconfig"

source "drivers/hwmon/Kconfig"

-#source "drivers/l3/Kconfig"
+source "drivers/l3/Kconfig"

source "drivers/misc/Kconfig"

diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -r orig/drivers/Makefile linux/./drivers/Makefile
--- orig/drivers/Makefile Wed Jun 29 15:49:56 2005
+++ linux/drivers/Makefile Wed Jun 29 15:58:48 2005
@@ -12,6 +12,8 @@ obj-$(CONFIG_ACPI_BOOT) += acpi/
# PnP must come after ACPI since it will eventually need to check if acpi
# was used and do nothing if so
obj-$(CONFIG_PNP) += pnp/
+obj-$(CONFIG_I2C) += i2c/
+obj-$(CONFIG_L3) += l3/

# char/ comes before serial/ etc so that the VT console is the boot-time
# default.
@@ -52,7 +54,6 @@ obj-$(CONFIG_SBUS) += sbus/
obj-$(CONFIG_GAMEPORT) += input/gameport/
obj-$(CONFIG_INPUT) += input/
obj-$(CONFIG_I2O) += message/
-obj-$(CONFIG_I2C) += i2c/
obj-$(CONFIG_W1) += w1/
obj-$(CONFIG_HWMON) += hwmon/
obj-$(CONFIG_PHONE) += telephony/
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej -x .git linus/drivers/i2c/busses/Kconfig linux/drivers/i2c/busses/Kconfig
--- linus/drivers/i2c/busses/Kconfig Wed Jun 22 22:41:27 2005
+++ linux/drivers/i2c/busses/Kconfig Sun Jul 24 20:32:40 2005
@@ -107,6 +107,17 @@ config I2C_HYDRA
This support is also available as a module. If so, the module
will be called i2c-hydra.

+config I2C_BIT_SA1100_GPIO
+ tristate "SA1100 I2C GPIO adapter"
+ depends on I2C && ARCH_SA1100
+ select BIT_SA1100_GPIO
+ help
+ This supports I2C on the SA11x0 processor GPIO pins. This
+ shares support with the L3 driver.
+
+ This support is also available as a module. If so, the module
+ will be called l3-bit-sa1100.
+
config I2C_I801
tristate "Intel 82801 (ICH)"
depends on I2C && PCI
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N orig/drivers/l3/Kconfig linux/drivers/l3/Kconfig
--- orig/drivers/l3/Kconfig Thu Jan 1 01:00:00 1970
+++ linux/drivers/l3/Kconfig Sat Jul 2 20:12:47 2005
@@ -0,0 +1,23 @@
+#
+# L3 bus configuration
+#
+
+menu "L3 serial bus support"
+
+config L3
+ tristate "L3 support"
+
+config L3_BIT_SA1100_GPIO
+ tristate "SA11x0 L3 GPIO adapter"
+ depends on L3 && ARCH_SA1100
+ select BIT_SA1100_GPIO
+
+config BIT_SA1100_GPIO
+ tristate
+ select L3_ALGOBIT if L3
+ select I2C_ALGOBIT if I2C
+
+config L3_ALGOBIT
+ bool
+
+endmenu
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N orig/drivers/l3/Makefile linux/drivers/l3/Makefile
--- orig/drivers/l3/Makefile Thu Jan 1 01:00:00 1970
+++ linux/drivers/l3/Makefile Sat Jul 2 20:13:13 2005
@@ -0,0 +1,8 @@
+#
+# Makefile for the L3 bus driver.
+#
+
+obj-$(CONFIG_L3) += l3-core.o
+obj-$(CONFIG_L3_ALGOBIT) += l3-algo-bit.o
+obj-$(CONFIG_BIT_SA1100_GPIO) += l3-bit-sa11x0.o
+
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N orig/drivers/l3/l3-algo-bit.c linux/drivers/l3/l3-algo-bit.c
--- orig/drivers/l3/l3-algo-bit.c Thu Jan 1 01:00:00 1970
+++ linux/drivers/l3/l3-algo-bit.c Sat Jul 2 17:31:58 2005
@@ -0,0 +1,174 @@
+/*
+ * L3 bus algorithm module.
+ *
+ * Copyright (C) 2001 Russell King, All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Note that L3 buses can share the same pins as I2C buses, so we must
+ * _not_ generate an I2C start condition. An I2C start condition is
+ * defined as a high-to-low transition of the data line while the clock
+ * is high. Therefore, we must only change the data line while the
+ * clock is low.
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/sched.h>
+#include <linux/l3.h>
+
+#include "l3-algo-bit.h"
+
+#define setdat(l3,algo,val) algo->setdat(l3, val)
+#define setclk(l3,algo,val) algo->setclk(l3, val)
+#define setmode(l3,algo,val) algo->setmode(l3, val)
+#define setdatin(l3,algo) algo->setdir(l3, 1)
+#define setdatout(l3,algo) algo->setdir(l3, 0)
+#define getdat(l3,algo) algo->getdat(l3)
+
+/*
+ * Send one byte of data to the chip. Data is latched into the chip on
+ * the rising edge of the clock.
+ */
+static void sendbyte(struct l3_adapter *l3, struct l3_algo_bit_data *algo,
+ unsigned int byte)
+{
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ setclk(l3, algo, 0);
+ udelay(algo->data_hold);
+ setdat(l3, algo, byte & 1);
+ udelay(algo->data_setup);
+ setclk(l3, algo, 1);
+ udelay(algo->clock_high);
+ byte >>= 1;
+ }
+}
+
+/*
+ * Send a set of bytes to the chip. We need to pulse the MODE line
+ * between each byte, but never at the start nor at the end of the
+ * transfer.
+ */
+static void sendbytes(struct l3_adapter *l3, struct l3_algo_bit_data *algo,
+ const u8 *buf, size_t len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ if (i) {
+ udelay(algo->mode_hold);
+ setmode(l3, algo, 0);
+ udelay(algo->mode);
+ }
+ setmode(l3, algo, 1);
+ udelay(algo->mode_setup);
+ sendbyte(l3, algo, buf[i]);
+ }
+}
+
+/*
+ * Read one byte of data from the chip. Data is latched into the chip on
+ * the rising edge of the clock.
+ */
+static unsigned int readbyte(struct l3_adapter *l3,
+ struct l3_algo_bit_data *algo)
+{
+ unsigned int byte = 0;
+ int i;
+
+ for (i = 0; i < 8; i++) {
+ setclk(l3, algo, 0);
+ udelay(algo->data_hold + algo->data_setup);
+ setclk(l3, algo, 1);
+ if (getdat(l3, algo))
+ byte |= 1 << i;
+ udelay(algo->clock_high);
+ }
+
+ return byte;
+}
+
+/*
+ * Read a set of bytes from the chip. We need to pulse the MODE line
+ * between each byte, but never at the start nor at the end of the
+ * transfer.
+ */
+static void readbytes(struct l3_adapter *l3, struct l3_algo_bit_data *algo,
+ u8 *buf, size_t len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ if (i) {
+ udelay(algo->mode_hold);
+ setmode(l3, algo, 0);
+ }
+ setmode(l3, algo, 1);
+ udelay(algo->mode_setup);
+ buf[i] = readbyte(l3, algo);
+ }
+}
+
+static void start(struct l3_adapter *l3, struct l3_algo_bit_data *algo,
+ u8 addr)
+{
+ /*
+ * If we share an I2C bus, ensure that it is in STOP mode
+ */
+ setclk(l3, algo, 1);
+ setdat(l3, algo, 1);
+ setmode(l3, algo, 1);
+ setdatout(l3, algo);
+ udelay(algo->mode);
+
+ setmode(l3, algo, 0);
+ udelay(algo->mode_setup);
+ sendbyte(l3, algo, addr);
+ udelay(algo->mode_hold);
+}
+
+static void stop(struct l3_adapter *l3, struct l3_algo_bit_data *algo)
+{
+ /*
+ * Ensure that we leave the bus in I2C stop mode.
+ */
+ setclk(l3, algo, 1);
+ setdat(l3, algo, 1);
+ setmode(l3, algo, 0);
+ setdatin(l3, algo);
+}
+
+ssize_t l3_algo_bit_read(struct l3_adapter *l3, struct l3_algo_bit_data *algo,
+ u8 addr, u8 *buf, size_t len)
+{
+ start(l3, algo, addr);
+
+ setdatin(l3, algo);
+ readbytes(l3, algo, buf, len);
+
+ stop(l3, algo);
+
+ return 0;
+}
+EXPORT_SYMBOL(l3_algo_bit_read);
+
+ssize_t l3_algo_bit_write(struct l3_adapter *l3, struct l3_algo_bit_data *algo,
+ u8 addr, const u8 *buf, size_t len)
+{
+ start(l3, algo, addr);
+
+ setdatout(l3, algo);
+ sendbytes(l3, algo, buf, len);
+
+ stop(l3, algo);
+
+ return 0;
+}
+EXPORT_SYMBOL(l3_algo_bit_write);
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N orig/drivers/l3/l3-algo-bit.h linux/drivers/l3/l3-algo-bit.h
--- orig/drivers/l3/l3-algo-bit.h Thu Jan 1 01:00:00 1970
+++ linux/drivers/l3/l3-algo-bit.h Sat Jul 2 15:49:41 2005
@@ -0,0 +1,37 @@
+/*
+ * linux/include/linux/l3/algo-bit.h
+ *
+ * Copyright (C) 2001 Russell King, All Rights Reserved.
+ *
+ * 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.
+ *
+ * L3 Bus bit-banging algorithm. Derived from i2c-algo-bit.h by
+ * Simon G. Vogl.
+ */
+#ifndef L3_ALGO_BIT_H
+#define L3_ALGO_BIT_H 1
+
+#include <linux/l3.h>
+
+struct l3_algo_bit_data {
+ void (*setdat) (struct l3_adapter *, int state);
+ void (*setclk) (struct l3_adapter *, int state);
+ void (*setmode)(struct l3_adapter *, int state);
+ void (*setdir) (struct l3_adapter *, int in); /* set data direction */
+ int (*getdat) (struct l3_adapter *);
+
+ /* bus timings (us) */
+ int data_hold;
+ int data_setup;
+ int clock_high;
+ int mode_hold;
+ int mode_setup;
+ int mode;
+};
+
+ssize_t l3_algo_bit_read(struct l3_adapter *, struct l3_algo_bit_data *, u8 addr, u8 *buf, size_t len);
+ssize_t l3_algo_bit_write(struct l3_adapter *, struct l3_algo_bit_data *, u8 addr, const u8 *buf, size_t len);
+
+#endif
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N orig/drivers/l3/l3-bit-sa11x0.c linux/drivers/l3/l3-bit-sa11x0.c
--- orig/drivers/l3/l3-bit-sa11x0.c Thu Jan 1 01:00:00 1970
+++ linux/drivers/l3/l3-bit-sa11x0.c Sat Jul 2 19:38:46 2005
@@ -0,0 +1,296 @@
+/*
+ * linux/drivers/l3/l3-bit-sa1100.c
+ *
+ * Copyright (C) 2001 Russell King
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This is a combined I2C and L3 bus driver.
+ */
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
+#include <asm/system.h>
+#include <asm/hardware.h>
+#include <asm/mach-types.h>
+#include <asm/arch/assabet.h>
+
+#define NAME "l3-bit-sa1100-gpio"
+
+struct bit_data {
+ unsigned int sda;
+ unsigned int scl;
+ unsigned int l3_mode;
+};
+
+static int getsda(void *data)
+{
+ struct bit_data *bits = data;
+
+ return GPLR & bits->sda;
+}
+
+#ifdef CONFIG_I2C_BIT_SA1100_GPIO
+static void i2c_setsda(void *data, int state)
+{
+ struct bit_data *bits = data;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ if (state)
+ GPDR &= ~bits->sda;
+ else {
+ GPCR = bits->sda;
+ GPDR |= bits->sda;
+ }
+ local_irq_restore(flags);
+}
+
+static void i2c_setscl(void *data, int state)
+{
+ struct bit_data *bits = data;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ if (state)
+ GPDR &= ~bits->scl;
+ else {
+ GPCR = bits->scl;
+ GPDR |= bits->scl;
+ }
+ local_irq_restore(flags);
+}
+
+static int i2c_getscl(void *data)
+{
+ struct bit_data *bits = data;
+
+ return GPLR & bits->scl;
+}
+
+static struct i2c_algo_bit_data i2c_bit_data = {
+ .setsda = i2c_setsda,
+ .setscl = i2c_setscl,
+ .getsda = getsda,
+ .getscl = i2c_getscl,
+ .udelay = 10,
+ .mdelay = 10,
+ .timeout = 100,
+};
+
+static struct i2c_adapter i2c_adapter = {
+ .algo_data = &i2c_bit_data,
+};
+
+#define LOCK &i2c_adapter.bus_lock
+
+static int __init i2c_init(struct bit_data *bits)
+{
+ i2c_bit_data.data = bits;
+ return i2c_bit_add_bus(&i2c_adapter);
+}
+
+static void i2c_exit(void)
+{
+ i2c_bit_del_bus(&i2c_adapter);
+}
+
+#else
+static DECLARE_MUTEX(l3_lock);
+#define LOCK &l3_lock
+#define i2c_init(bits) (0)
+#define i2c_exit() do { } while (0)
+#endif
+
+#ifdef CONFIG_L3_BIT_SA1100_GPIO
+
+#include "l3-algo-bit.h"
+
+/*
+ * iPAQs need the clock line driven hard high and low.
+ */
+static void l3_setscl(struct l3_adapter *adap, int state)
+{
+ struct bit_data *bits = adap->data;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ if (state)
+ GPSR = bits->scl;
+ else
+ GPCR = bits->scl;
+ GPDR |= bits->scl;
+ local_irq_restore(flags);
+}
+
+static void l3_setsda(struct l3_adapter *adap, int state)
+{
+ struct bit_data *bits = adap->data;
+
+ if (state)
+ GPSR = bits->sda;
+ else
+ GPCR = bits->sda;
+}
+
+static void l3_setdir(struct l3_adapter *adap, int in)
+{
+ struct bit_data *bits = adap->data;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ if (in)
+ GPDR &= ~bits->sda;
+ else
+ GPDR |= bits->sda;
+ local_irq_restore(flags);
+}
+
+static void l3_setmode(struct l3_adapter *adap, int state)
+{
+ struct bit_data *bits = adap->data;
+
+ if (state)
+ GPSR = bits->l3_mode;
+ else
+ GPCR = bits->l3_mode;
+}
+
+static int l3_getdat(struct l3_adapter *adap)
+{
+ struct bit_data *bits = adap->data;
+
+ return GPLR & bits->scl;
+}
+
+static struct l3_algo_bit_data l3_bit_data = {
+ .setdat = l3_setsda,
+ .setclk = l3_setscl,
+ .setmode = l3_setmode,
+ .setdir = l3_setdir,
+ .getdat = l3_getdat,
+ .data_hold = 1,
+ .data_setup = 1,
+ .clock_high = 1,
+ .mode_hold = 1,
+ .mode_setup = 1,
+};
+
+static int sa11x0_l3_read(struct l3_adapter *l3_adap, u8 addr, u8 *buf, size_t len)
+{
+ return l3_algo_bit_read(l3_adap, &l3_bit_data, addr, buf, len);
+}
+
+static int sa11x0_l3_write(struct l3_adapter *l3_adap, u8 addr, const u8 *buf, size_t len)
+{
+ return l3_algo_bit_write(l3_adap, &l3_bit_data, addr, buf, len);
+}
+
+static struct l3_adapter l3_adapter = {
+ .owner = THIS_MODULE,
+ .name = NAME,
+ .read = sa11x0_l3_read,
+ .write = sa11x0_l3_write,
+ .lock = LOCK,
+};
+
+static int __init l3_init(struct bit_data *bits)
+{
+ l3_adapter.data = bits;
+ return l3_add_adapter(&l3_adapter);
+}
+
+static void __exit l3_exit(void)
+{
+ l3_del_adapter(&l3_adapter);
+}
+#else
+#define l3_init(bits) (0)
+#define l3_exit() do { } while (0)
+#endif
+
+static struct bit_data bit_data;
+
+static int __init bus_init(void)
+{
+ struct bit_data *bit = &bit_data;
+ unsigned long flags;
+ int ret;
+
+ if (machine_is_assabet() || machine_is_pangolin()) {
+ bit->sda = GPIO_GPIO15;
+ bit->scl = GPIO_GPIO18;
+ bit->l3_mode = GPIO_GPIO17;
+ }
+
+ if (machine_is_cerf()) {
+ bit->sda = GPIO_GPIO6;
+ bit->scl = GPIO_GPIO4;
+ bit->l3_mode = GPIO_GPIO5;
+ }
+
+ if (machine_is_h3600() || machine_is_h3100()) {
+ bit->sda = GPIO_GPIO14;
+ bit->scl = GPIO_GPIO16;
+ bit->l3_mode = GPIO_GPIO15;
+ }
+
+ if (machine_is_stork()) {
+ bit->sda = GPIO_GPIO15;
+ bit->scl = GPIO_GPIO18;
+ bit->l3_mode = GPIO_GPIO17;
+ }
+
+ if (!bit->sda)
+ return -ENODEV;
+
+ /*
+ * Default level for L3 mode is low.
+ * We set SCL and SDA high (i2c idle state).
+ */
+ local_irq_save(flags);
+ GPDR &= ~(bit->scl | bit->sda);
+ GPCR = bit->l3_mode | bit->scl | bit->sda;
+ GPDR |= bit->l3_mode;
+ local_irq_restore(flags);
+
+ if (machine_is_assabet()) {
+ /*
+ * Release reset on UCB1300, ADI7171 and UDA1341. We
+ * need to do this here so that we can communicate on
+ * the I2C/L3 buses.
+ */
+ ASSABET_BCR_set(ASSABET_BCR_CODEC_RST);
+ mdelay(1);
+ ASSABET_BCR_clear(ASSABET_BCR_CODEC_RST);
+ mdelay(1);
+ ASSABET_BCR_set(ASSABET_BCR_CODEC_RST);
+ }
+
+ ret = i2c_init(bit);
+ if (ret == 0 && bit->l3_mode) {
+ ret = l3_init(bit);
+ if (ret)
+ i2c_exit();
+ }
+
+ return ret;
+}
+
+static void __exit bus_exit(void)
+{
+ l3_exit();
+ i2c_exit();
+}
+
+module_init(bus_init);
+module_exit(bus_exit);
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' -x .git -N orig/drivers/l3/l3-core.c linux/drivers/l3/l3-core.c
--- orig/drivers/l3/l3-core.c Thu Jan 1 01:00:00 1970
+++ linux/drivers/l3/l3-core.c Sat Jul 2 15:49:24 2005
@@ -0,0 +1,170 @@
+/*
+ * linux/drivers/l3/l3-core.c
+ *
+ * Copyright (C) 2001 Russell King
+ *
+ * General structure taken from i2c-core.c by Simon G. Vogl
+ *
+ * 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.
+ *
+ * See linux/Documentation/l3 for further documentation.
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/l3.h>
+
+static DECLARE_MUTEX(adapter_lock);
+static LIST_HEAD(adapter_list);
+
+/**
+ * l3_add_adapter - register a new L3 bus adapter
+ * @adap: l3_adapter structure for the registering adapter
+ *
+ * Make the adapter available for use by clients using name adap->name.
+ * The adap->adapters list is initialised by this function.
+ *
+ * Returns 0;
+ */
+int l3_add_adapter(struct l3_adapter *adap)
+{
+ down(&adapter_lock);
+ list_add(&adap->adapters, &adapter_list);
+ up(&adapter_lock);
+ return 0;
+}
+
+/**
+ * l3_del_adapter - unregister a L3 bus adapter
+ * @adap: l3_adapter structure to unregister
+ *
+ * Remove an adapter from the list of available L3 Bus adapters.
+ *
+ * Returns 0;
+ */
+int l3_del_adapter(struct l3_adapter *adap)
+{
+ down(&adapter_lock);
+ list_del(&adap->adapters);
+ up(&adapter_lock);
+ return 0;
+}
+
+static struct l3_adapter *__l3_get_adapter(const char *name)
+{
+ struct list_head *l;
+
+ list_for_each(l, &adapter_list) {
+ struct l3_adapter *adap = list_entry(l, struct l3_adapter, adapters);
+
+ if (strcmp(adap->name, name) == 0)
+ return adap;
+ }
+
+ return NULL;
+}
+
+/**
+ * l3_get_adapter - get a reference to an adapter
+ * @name: driver name
+ *
+ * Obtain a l3_adapter structure for the specified adapter. If the adapter
+ * is not currently load, then load it. The adapter will be locked in core
+ * until all references are released via l3_put_adapter.
+ */
+struct l3_adapter *l3_get_adapter(const char *name)
+{
+ struct l3_adapter *adap;
+ int try;
+
+ for (try = 0; try < 2; try ++) {
+ down(&adapter_lock);
+ adap = __l3_get_adapter(name);
+ if (adap && !try_module_get(adap->owner))
+ adap = NULL;
+ up(&adapter_lock);
+
+ if (adap)
+ break;
+
+ if (try == 0)
+ request_module(name);
+ }
+
+ return adap;
+}
+
+/**
+ * l3_put_adapter - release a reference to an adapter
+ * @adap: driver to release reference
+ *
+ * Indicate to the L3 core that you no longer require the adapter reference.
+ * The adapter module may be unloaded when there are no references to its
+ * data structure.
+ *
+ * You must not use the reference after calling this function.
+ */
+void l3_put_adapter(struct l3_adapter *adap)
+{
+ if (adap && adap->owner)
+ module_put(adap->owner);
+}
+
+/**
+ * l3_write - send data to a device on an L3 bus
+ * @adap: L3 bus adapter
+ * @addr: L3 bus address
+ * @buf: buffer for bytes to send
+ * @len: number of bytes to send
+ *
+ * Send len bytes pointed to by buf to device address addr on the L3 bus
+ * described by client.
+ *
+ * Returns the number of bytes transferred, or negative error code.
+ */
+ssize_t l3_write(struct l3_adapter *adap, int addr, const u8 *buf, size_t len)
+{
+ ssize_t ret;
+
+ down(adap->lock);
+ ret = adap->write(adap, addr, buf, len);
+ up(adap->lock);
+
+ return ret;
+}
+
+/**
+ * l3_read - receive data from a device on an L3 bus
+ * @adap: L3 bus adapter
+ * @addr: L3 bus address
+ * @buf: buffer for bytes to receive
+ * @len: number of bytes to receive
+ *
+ * Receive len bytes from device address addr on the L3 bus described by
+ * client to a buffer pointed to by buf.
+ *
+ * Returns the number of bytes transferred, or negative error code.
+ */
+ssize_t l3_read(struct l3_adapter *adap, int addr, u8 *buf, size_t len)
+{
+ ssize_t ret;
+
+ down(adap->lock);
+ ret = adap->read(adap, addr, buf, len);
+ up(adap->lock);
+
+ return ret;
+}
+
+EXPORT_SYMBOL(l3_add_adapter);
+EXPORT_SYMBOL(l3_del_adapter);
+EXPORT_SYMBOL(l3_get_adapter);
+EXPORT_SYMBOL(l3_put_adapter);
+EXPORT_SYMBOL(l3_write);
+EXPORT_SYMBOL(l3_read);
diff -up -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x *.orig -x *.rej -x .git -N orig/include/linux/l3.h linux/include/linux/l3.h
--- orig/include/linux/l3.h Sat Apr 26 08:56:46 1997
+++ linux/include/linux/l3.h Sat Jul 2 15:46:10 2005
@@ -0,0 +1,73 @@
+/*
+ * linux/include/linux/l3/l3.h
+ *
+ * Copyright (C) 2001 Russell King, All Rights Reserved.
+ *
+ * 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.
+ *
+ * Derived from i2c.h by Simon G. Vogl
+ */
+#ifndef L3_H
+#define L3_H
+
+#ifdef __KERNEL__
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+struct l3_adapter;
+struct semaphore;
+
+/*
+ * l3_adapter is the structure used to identify a physical L3 bus along
+ * with the access algorithms necessary to access it.
+ */
+struct l3_adapter {
+ /*
+ * This name is used to uniquely identify the adapter.
+ * It should be the same as the module name.
+ */
+ char name[32];
+
+ /*
+ * perform bus transactions
+ */
+ ssize_t (*read)(struct l3_adapter *, u8 addr, u8 *buf, size_t len);
+ ssize_t (*write)(struct l3_adapter *, u8 addr, const u8 *buf, size_t len);
+
+ /*
+ * This may be NULL, or should point to the module struct
+ */
+ struct module *owner;
+
+ /*
+ * private data for the adapter
+ */
+ void *data;
+
+ /*
+ * Our lock. Unlike the i2c layer, we allow this to be used for
+ * other stuff, like the i2c layer lock. Some people implement
+ * i2c stuff using the same signals as the l3 bus.
+ */
+ struct semaphore *lock;
+
+ /*
+ * List of all adapters.
+ */
+ struct list_head adapters;
+};
+
+extern int l3_add_adapter(struct l3_adapter *);
+extern int l3_del_adapter(struct l3_adapter *);
+extern void l3_put_adapter(struct l3_adapter *);
+extern struct l3_adapter *l3_get_adapter(const char *name);
+
+extern ssize_t l3_write(struct l3_adapter *, int, const u8 *, size_t);
+extern ssize_t l3_read(struct l3_adapter *, int, u8 *, size_t);
+
+#endif
+
+#endif /* L3_H */


--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-10-30 12:39:54

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

Hi!

> This code is the final link is getting akita working but I'm not sure
> its the right approach. I'm posting this in the hope someone might see a
> better way to achieve this driver's objectives. I'd like to get akita
> support into mainline and this is the only barrier.

Well, what you describe is not too nice, but I do not see nicer solutions :-(.

> I2C drivers appear relatively late in the boot procedure and changing
> that isn't practical. I therefore ended up writing akita-ioexp which

It seems that making i2c init early is only sane choice. I realize PC people
will hate it... but apart from that, why is it impractical?

> There is a fundamental problem with the lack of a proper gpio interface
> in Linux. Every driver does something different with them (be it pxa
> specific gpios, SCOOP gpios, those on a IO expander, those on a video
> chip (w100fb springs to mind) to name just the Zaurus specific ones.

Yup. GPIOs are not problem on i386, so noone solved this one :-(.


Pavel
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms

2005-11-02 19:44:55

by Andy Isaacson

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

On Sat, Oct 29, 2005 at 09:08:19PM +0200, Pavel Machek wrote:
> > I2C drivers appear relatively late in the boot procedure and changing
> > that isn't practical. I therefore ended up writing akita-ioexp which
>
> It seems that making i2c init early is only sane choice. I realize PC people
> will hate it... but apart from that, why is it impractical?

FWIW, I have also run into this "I need I2C early in boot, but it's not
inited until late" on SiByte (arch/mips/sibyte/{sb1250,bcm1480}/setup.c).
For the time being in the linux-mips tree we simply have two drivers
talking to the I2C interface - sibyte/swarm/rtc_* and i2c-sibyte.c,
and they are currently lacking even any trivial locking. We haven't
seen any problems yet but that's due to limited exercise - the default
config doesn't hook up any drivers for the other chips on I2C.

How do other arches that have I2C RTCs deal with this problem? Or is
there something wrong with how arch/mips/kernel/time.c:time_init deals
with the rtc?

> > There is a fundamental problem with the lack of a proper gpio interface
> > in Linux. Every driver does something different with them (be it pxa
> > specific gpios, SCOOP gpios, those on a IO expander, those on a video
> > chip (w100fb springs to mind) to name just the Zaurus specific ones.
>
> Yup. GPIOs are not problem on i386, so noone solved this one :-(.

I would also be overjoyed to have a GPIO infrastructure to plug into.

(And I would say "GPIOs are not used on PCs"; I am confident the Geode
driving the seat-back TV on this Song flight has GPIOs...)

-andy

2005-11-02 22:53:09

by Russell King

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

On Wed, Nov 02, 2005 at 11:44:53AM -0800, Andy Isaacson wrote:
> On Sat, Oct 29, 2005 at 09:08:19PM +0200, Pavel Machek wrote:
> > > I2C drivers appear relatively late in the boot procedure and changing
> > > that isn't practical. I therefore ended up writing akita-ioexp which
> >
> > It seems that making i2c init early is only sane choice. I realize PC people
> > will hate it... but apart from that, why is it impractical?
>
> FWIW, I have also run into this "I need I2C early in boot, but it's not
> inited until late" on SiByte (arch/mips/sibyte/{sb1250,bcm1480}/setup.c).
> For the time being in the linux-mips tree we simply have two drivers
> talking to the I2C interface - sibyte/swarm/rtc_* and i2c-sibyte.c,
> and they are currently lacking even any trivial locking. We haven't
> seen any problems yet but that's due to limited exercise - the default
> config doesn't hook up any drivers for the other chips on I2C.
>
> How do other arches that have I2C RTCs deal with this problem? Or is
> there something wrong with how arch/mips/kernel/time.c:time_init deals
> with the rtc?

On ARM, where we have I2C RTCs, I tend to leave xtime well alone in
time_init and just setup the timer. When i2c is initialised, and
the bus and RTC have been detected, I set the time from them at
that point.

I haven't seen any problems with this approach. In fact, I'd
rather time_init() just setup the timer, and we set the time of
day later during the kernels initialisation.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-11-03 07:06:50

by David Brownell

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

> > It seems that making i2c init early is only sane choice. I realize PC people
> > will hate it...

What do you mean by "early"? Other than maybe linking it earlier
in the drivers/Makefile, and probably running some arch-specific i2c
controller (and certain i2c chip) drivers at subsys_initcall rather
than at device_initcall ... does the 2.6.14 kernel need changes?


> > but apart from that, why is it impractical?
>
> FWIW, I have also run into this "I need I2C early in boot, but it's not
> inited until late" on SiByte ...

Likewise on many OMAP boards, which tend to have the power management
and other essential features on I2C.

Some board-specific init code ends up needing to run after the probe()
logic of the tps6501x driver ... like for example, using a GPIO (!)
there to power up the Ethernet controller which may be needed for the
root filesystem. (At least on many development systems!)

You can see where that leads: we patched the i2c subsystem so that
it runs at subsys_initcall ... and also the omap_i2c driver, and
the tps65010 driver. No other drivers need to be changed, just the
ones involved in that board's power management.


Richard:

> I had to turn akita-ioexp into a platform device to get the suspend
> signal which is used to flush the workqueue. With a platform device
> available at machine init time, I can insert it as a parent of the corgi
> device chain which means its one of the last devices to be suspended.

By doing all that stuff as "subsys_initcall", the relevant I2C gpio
hardware will be also be suspended "late" ... without such a fake
platform device. Unless you're doing selective suspend, details of
the device tree matter less than the order used to create devices.

- Dave

2005-11-03 09:07:35

by Richard Purdie

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

On Wed, 2005-11-02 at 23:06 -0800, David Brownell wrote:
> Likewise on many OMAP boards, which tend to have the power management
> and other essential features on I2C.
>
> Some board-specific init code ends up needing to run after the probe()
> logic of the tps6501x driver ... like for example, using a GPIO (!)
> there to power up the Ethernet controller which may be needed for the
> root filesystem. (At least on many development systems!)
>
> You can see where that leads: we patched the i2c subsystem so that
> it runs at subsys_initcall ... and also the omap_i2c driver, and
> the tps65010 driver. No other drivers need to be changed, just the
> ones involved in that board's power management.

Its not as simple as this in my case. Yes, you can patch the maxim i2c
driver and the pxa i2c controller to run at subsys_initcall level which
is fine. akita-ioexp has to run at a higher level (fs_initcall) as it
will run before the i2c subsystem (being in arch as against drivers).

I don't think is feasible to go through the LCD, IrDA and sound
subsystems trying to guarantee what init levels they're going to access
the gpios at once you start getting to fs_initcall level.

This could be a case for putting akita-ioexp into drivers/i2c/chips but
see my other thoughts below...

> Richard:
>
> > I had to turn akita-ioexp into a platform device to get the suspend
> > signal which is used to flush the workqueue. With a platform device
> > available at machine init time, I can insert it as a parent of the corgi
> > device chain which means its one of the last devices to be suspended.
>
> By doing all that stuff as "subsys_initcall", the relevant I2C gpio
> hardware will be also be suspended "late" ... without such a fake
> platform device. Unless you're doing selective suspend, details of
> the device tree matter less than the order used to create devices.

I'd like to keep the device tree correct, then if anyone does want to
selectively suspend things, it will work and it also works if someone
decides to change the device init order around at some later date and
not everything will break.

I'm coming to the view that I should merge the max7310 i2c driver into
akita-ioexp and lose the general case driver for the following reasons:

1. Having a header file around for two functions seems excessive and I
can't see any alternative
2. The MAX7310 has 56 different i2c addresses and no way of detecting
the presence of the chip. Autoprobing on akita means you end up with the
sound codec being taken by the max7310 driver. I don't like to think
what it would do on other systems.
3. There's no interdependency between two drivers and hence it should be
more robust.
4. I can lose the workqueue "find the i2c client" routine as the driver
can know when the i2c device has been detected and the client setup.
(the workqueue is still needed to act on gpio requests in case they come
from interrupt context but the driver becomes simpler and of a more sane
design).

Richard

2005-11-03 15:38:58

by David Brownell

[permalink] [raw]
Subject: Re: [RFC] The driver model, I2C and gpio provision on Sharp SL-C1000 (Akita)

On Thursday 03 November 2005 1:07 am, Richard Purdie wrote:
>
> Its not as simple as this in my case. Yes, you can patch the maxim i2c
> driver and the pxa i2c controller to run at subsys_initcall level which
> is fine. akita-ioexp has to run at a higher level (fs_initcall) as it
> will run before the i2c subsystem (being in arch as against drivers).

The general policy is that the "arch" directories shouldn't have drivers;
they should live in the "drivers" directories. Then there are still issues
of how board-specific code should be handled; hooking via platform_data
doesn't quite solve all the important cases.

And of course I2C doesn't even support platform_data for board-specific data.
Which means that adding board-specific code to I2C drivers is all but inevitable.
(Or in the extreme, making drivers board-specific not general-purpose.)


> I'd like to keep the device tree correct, then if anyone does want to
> selectively suspend things, it will work and it also works if someone
> decides to change the device init order around at some later date and
> not everything will break.

Don't assume that the device tree _model_ is ready to handle selective
suspend yet. For one thing, the pm_parent mechanism is unusable, and
that was originally intended to be the way to handle exceptions like
these (where the "control tree" doesn't match the "power tree", or
however you want to describe it).

I expect that by the time selective suspend is generally usable, the
driver model support for it will have evolved substantially. In any
case, it's not worth too much effort just now to try and make it work.


> I'm coming to the view that I should merge the max7310 i2c driver into
> akita-ioexp and lose the general case driver for the following reasons:
>
> 1. Having a header file around for two functions seems excessive and I
> can't see any alternative

I'd have said that for the tps6501x, but its header now exports
more like seven functions. Only a few are for GPIOs; and that
doesn't even handle (yet!) the various IRQ sources!

A general purpose driver for max7310 (and similar I2C GPIO chips)
should handle IRQs from input pins, too.


> 2. The MAX7310 has 56 different i2c addresses and no way of detecting
> the presence of the chip. Autoprobing on akita means you end up with the
> sound codec being taken by the max7310 driver. I don't like to think
> what it would do on other systems.

IMO this is a general problem with the I2C framework. It should be
possible for board-specific init logic to (a) declare what devices
exist, (b) what drivers they'll use, and (c) provide platform_data
specific to that device, which could include board-specific callbacks.

The closest I think you can come now is to provide driver options
on the kernel command line, like max7310.force, instead of trying
to probe those way-too-many addresses. That can handle (a) and (b)
but not (c); and it's an awkward way to handle static config data.

A related solution is for the driver init code to patch the i2c probe
table based on what board is present (machine_is_akita etc) before
registering that I2C driver. That's less error prone than relying
on command line parameters. And again, that ignores (c).


> 3. There's no interdependency between two drivers and hence it should be
> more robust.
> 4. I can lose the workqueue "find the i2c client" routine as the driver
> can know when the i2c device has been detected and the client setup.

Yeah, the max7310 probe() could benefit from platform data holding a callback
used to register a handle to that chip. The workqueue wouldn't be needed
since the callback could just provide the right device.


> (the workqueue is still needed to act on gpio requests in case they come
> from interrupt context but the driver becomes simpler and of a more sane
> design).

The TPS6501x driver currently just uses keventd rather than a dedicated
workqueue. But again, this is a general problem in the I2C framework;
if it had an asynch message model with completion callbacks, it'd be
easy to issue requests from irq contexts, and task-synchronous calls could
just use "struct completion" to wait for the callback.


That SPI framework code I did was specifically paying attention to those
driver (and I/O) model issues, so it doesn't have the same glitches.
The core I/O model is asynchronous, and board-specific code can provice
platform_data and other hooks that'll kick in on device creation. I'd
hope that once that all shakes out, the I2C stack can start to adopt the
same sort of fixes.

- Dave



> Richard
>