Skip to contentSkip to conversation

Blackfin: on-chip Two Wire Interface I2C driver

PATCH In review Wu, Bryan 4 participants 14 messages Mar 21, 2007 – Mar 23, 2007 I2C subsystem host drivers

lore
Share & tools
lore

Wu, Bryan

author
#0 lore raw

PATCH 4/4 Blackfin: on-chip Two Wire Interface I2C driver

Hi folks,

As GPIO based blackfin driver will be replaced by I2C-GPIO generic driver, we just update this latest version of blackfin on-chip TWI I2C driver according to LKML review.

[PATCH] Blackfin: on-chip Two Wire Interface I2C driver

The i2c linux driver for blackfin architecture which supports blackfin on-chip TWI controller i2c operation.

Signed-off-by Bryan Wu <[email protected]>
Reviewed-by Andrew Morton <[email protected]>
Reviewed-by Alexey Dobriyan <[email protected]>
Reviewed-by Jean Delvare <[email protected]>
Cc David Brownell <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 drivers/i2c/busses/Kconfig        |   16 
 drivers/i2c/busses/Makefile       |    1 
 drivers/i2c/busses/i2c-bfin-twi.c |  653 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 670 insertions(+)
drivers/i2c/busses/Kconfig +16 -0
Index: linux-2.6/drivers/i2c/busses/Kconfig
===================================================================
@@ -91,6 +91,22 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-au1550.
 
+config I2C_BLACKFIN_TWI
+	tristate "Blackfin TWI I2C support"
+	depends on I2C && (BF534 || BF536 || BF537)
+	help
+	  This is the TWI I2C device driver for Blackfin 534/536/537.
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-bfin-twi.
+
+config I2C_BLACKFIN_TWI_CLK_KHZ
+	int "Blackfin TWI I2C clock (kHz)"
+	depends on I2C_BLACKFIN_TWI
+	range 10 400
+	default 50
+	help
+	  The unit of the TWI clock is kilo HZ.
+
 config I2C_ELEKTOR
 	tristate "Elektor ISA card"
 	depends on I2C && ISA && BROKEN_ON_SMP
drivers/i2c/busses/Makefile +1 -0
Index: linux-2.6/drivers/i2c/busses/Makefile
===================================================================
@@ -10,6 +10,7 @@
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
+obj-$(CONFIG_I2C_BLACKFIN_TWI)  += i2c-bfin-twi.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o
drivers/i2c/busses/i2c-bfin-twi.c new file +653 -0

653 lines not on this page · raw message

_

Thanks
-Bryan Wu

#1 lore raw

Hi Bryan,

What's the point of setting the Reply-To header to your own address, other than forcing me to add your name manually each time I reply?

Quoted 3 lines · Bryan

As GPIO based blackfin driver will be replaced by I2C-GPIO generic driver, we just update this latest version of blackfin on-chip TWI I2C driver according to LKML review.

Good idea.

Quoted 11 lines

[PATCH] Blackfin: on-chip Two Wire Interface I2C driver

The i2c linux driver for blackfin architecture which supports blackfin on-chip TWI controller i2c operation.

Signed-off-by Bryan Wu <[email protected]>
Reviewed-by Andrew Morton <[email protected]>
Reviewed-by Alexey Dobriyan <[email protected]>
Reviewed-by Jean Delvare <[email protected]>
Cc David Brownell <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
Quoted 6 lines

---

 drivers/i2c/busses/Kconfig        |   16 
 drivers/i2c/busses/Makefile       |    1 
 drivers/i2c/busses/i2c-bfin-twi.c |  653 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 670 insertions(+)

This is missing a MAINTAINERS entry. Please provide a patch adding it, and I'm push this patch on my i2c stack (for Linux 2.6.22.)

drivers/i2c/busses/i2c-bfin-twi.c line 641

Quoted 12 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +12 -0
+static int __init i2c_bfin_twi_init(void)
+{
+	int ret;
+
+	pr_info("I2C: Blackfin I2C TWI driver\n");
+
+	ret = platform_driver_register(&i2c_bfin_twi_driver);
+	if (ret) {
+		pr_debug("Fail to register i2c_bfin_twi driver\n");
+	}
+	return ret;
+}

That pr_debug isn't exactly useful. If the registration failed, the user will notice soon enough, as the driver will not be loaded at all and modprobe will return the error code (which is more valuable for debugging than the message itself.)

Signature · 1 line
Jean Delvare
#2 lore raw
In reply to Wu, Bryan

drivers/i2c/busses/Kconfig line 100

Quoted 7 lines
drivers/i2c/busses/Kconfig +7 -0
+config I2C_BLACKFIN_TWI
+       tristate "Blackfin TWI I2C support"
+       depends on I2C && (BF534 || BF536 || BF537)
+       help
+         This is the TWI I2C device driver for Blackfin 534/536/537.
+         This driver can also be built as a module.  If so, the module
+         will be called i2c-bfin-twi.

Pardon my ignorance, but is there any reason to call it "Blackfin TWI" as opposed to just "Blackfin" since TWI and I2C mean the same thing? It lends itself to some redundancies such as:

Quoted 1 line

+ pr_info("I2C: Blackfin I2C TWI driver\n");

-Bob

#3 lore raw
Quoted 8 lines · Bob Copeland

drivers/i2c/busses/Kconfig line 100

Quoted 7 lines
drivers/i2c/busses/Kconfig +7 -0
+config I2C_BLACKFIN_TWI
+       tristate "Blackfin TWI I2C support"
+       depends on I2C && (BF534 || BF536 || BF537)
+       help
+         This is the TWI I2C device driver for Blackfin 534/536/537.
+         This driver can also be built as a module.  If so, the module
+         will be called i2c-bfin-twi.
Quoted 5 lines

Pardon my ignorance, but is there any reason to call it "Blackfin TWI" as opposed to just "Blackfin" since TWI and I2C mean the same thing? It lends itself to some redundancies such as:

Quoted 1 line

+ pr_info("I2C: Blackfin I2C TWI driver\n");

it reflects the hardware manual and all public Analog Devices documentation ... the peripheral is always referred to as a "Two Wire Interface that is fully compatible with the I2C standard" since using the "I2C" term is not free
-mike

#4 lore raw
Quoted 13 lines · Mike Frysinger
Quoted 12 lines · Bob Copeland

drivers/i2c/busses/Kconfig line 100

Quoted 7 lines
drivers/i2c/busses/Kconfig +7 -0
+config I2C_BLACKFIN_TWI
+       tristate "Blackfin TWI I2C support"
+       depends on I2C && (BF534 || BF536 || BF537)
+       help
+         This is the TWI I2C device driver for Blackfin 534/536/537.
+         This driver can also be built as a module.  If so, the module
+         will be called i2c-bfin-twi.

Pardon my ignorance, but is there any reason to call it "Blackfin TWI" as opposed to just "Blackfin" since TWI and I2C mean the same thing? It lends itself to some redundancies such as:

Quoted 6 lines
Quoted 1 line
Quoted 1 line

+ pr_info("I2C: Blackfin I2C TWI driver\n");

it reflects the hardware manual and all public Analog Devices documentation ... the peripheral is always referred to as a "Two Wire Interface that is fully compatible with the I2C standard" since using the "I2C" term is not free

Damn, I hate it when legal idiocy takes the lead on technical clarity :(

Signature · 1 line
Jean Delvare
#5 lore raw
Quoted 14 lines · Jean Delvare
Quoted 13 lines · Mike Frysinger
Quoted 12 lines · Bob Copeland

drivers/i2c/busses/Kconfig line 100

Quoted 7 lines
drivers/i2c/busses/Kconfig +7 -0
+config I2C_BLACKFIN_TWI
+       tristate "Blackfin TWI I2C support"
+       depends on I2C && (BF534 || BF536 || BF537)
+       help
+         This is the TWI I2C device driver for Blackfin 534/536/537.
+         This driver can also be built as a module.  If so, the module
+         will be called i2c-bfin-twi.

Pardon my ignorance, but is there any reason to call it "Blackfin TWI" as opposed to just "Blackfin" since TWI and I2C mean the same thing? It lends itself to some redundancies such as:

Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line

+ pr_info("I2C: Blackfin I2C TWI driver\n");

it reflects the hardware manual and all public Analog Devices documentation ... the peripheral is always referred to as a "Two Wire Interface that is fully compatible with the I2C standard" since using the "I2C" term is not free

Damn, I hate it when legal idiocy takes the lead on technical clarity :(

i'm with you 100% but it isnt my call to make ;(
-mike

#6 lore raw
In reply to Wu, Bryan

Bryan,

Quoted 15 lines · Bryan

As GPIO based blackfin driver will be replaced by I2C-GPIO generic driver, we just update this latest version of blackfin on-chip TWI I2C driver according to LKML review.

[PATCH] Blackfin: on-chip Two Wire Interface I2C driver

The i2c linux driver for blackfin architecture which supports blackfin on-chip TWI controller i2c operation.

Signed-off-by Bryan Wu <[email protected]>
Reviewed-by Andrew Morton <[email protected]>
Reviewed-by Alexey Dobriyan <[email protected]>
Reviewed-by Jean Delvare <[email protected]>
Cc David Brownell <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
Quoted 6 lines

---

 drivers/i2c/busses/Kconfig        |   16 
 drivers/i2c/busses/Makefile       |    1 
 drivers/i2c/busses/i2c-bfin-twi.c |  653 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 670 insertions(+)

I overlooked a number of problems in the probe function, sorry.

drivers/i2c/busses/Makefile line 13

Quoted 9 lines

Index: linux-2.6/drivers/i2c/busses/Makefile
===================================================================

drivers/i2c/busses/Makefile +1 -0
@@ -10,6 +10,7 @@
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
+obj-$(CONFIG_I2C_BLACKFIN_TWI)  += i2c-bfin-twi.o

Please align with a tab, not spaces.

Quoted 15 lines
drivers/i2c/busses/Makefile +0 -0
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o

Index: linux-2.6/drivers/i2c/busses/i2c-bfin-twi.c
===================================================================

drivers/i2c/busses/i2c-bfin-twi.c new file +7 -0
(...)
+static int i2c_bfin_twi_probe(struct platform_device *dev)
+{
+	struct bfin_twi_iface *iface = &twi_iface;
+	struct i2c_adapter *p_adap;
+	int rc;
+
+	mutex_init(&(iface->twi_lock));

drivers/i2c/busses/i2c-bfin-twi.c line 568

Quoted 10 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +10 -0
+	spin_lock_init(&(iface->lock));
+	init_completion(&(iface->complete));
+	iface->irq = IRQ_TWI;
+
+	init_timer(&(iface->timeout_timer));
+	iface->timeout_timer.function = bfin_twi_timeout;
+	iface->timeout_timer.data = (unsigned long)iface;
+
+	p_adap = &(iface->adap);
+	p_adap->id = I2C_DRIVERID_BLACKFINTWI;

This isn't defined anywhere, and isn't valid anyway. I2C_DRIVERID_* are for I2C chip drivers, not bus drivers.

drivers/i2c/busses/i2c-bfin-twi.c line 569

Quoted 1 line
drivers/i2c/busses/i2c-bfin-twi.c new file +1 -0
+	strlcpy(p_adap->name, dev->name, I2C_NAME_SIZE);

I2C_NAME_SIZE changed recently, it no longer applies to i2c_adapter.name. Please use sizeof(p_adap->name) instead.

drivers/i2c/busses/i2c-bfin-twi.c line 572

Quoted 3 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +3 -0
+	p_adap->algo = &bfin_twi_algorithm;
+	p_adap->algo_data = iface;
+	p_adap->class = I2C_CLASS_ALL;

This pretty much voids the point of these probing classes. You should only select the classes matching devices which may actually be probed for on this bus. If different boards have different needs, get the right classes from the platform data.

drivers/i2c/busses/i2c-bfin-twi.c line 573

Quoted 1 line
drivers/i2c/busses/i2c-bfin-twi.c new file +1 -0
+	p_adap->dev.parent = &(dev->dev);

Note that parentheses are not needed.

drivers/i2c/busses/i2c-bfin-twi.c line 579

Quoted 6 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +6 -0
+
+	rc = request_irq(iface->irq, bfin_twi_interrupt_entry,
+		SA_INTERRUPT, dev->name, iface);
+	if (rc) {
+		printk(KERN_ERR"i2c-bfin-twi: can't get IRQ %d !\n",
+			iface->irq);

Usually we leave a space between KERN_* and the beginning of the string
- I'm even surprised that it compiles without it. OTOH there is no space before exclamation marks in English.

Oh, and it should be dev_err() anyway.

drivers/i2c/busses/i2c-bfin-twi.c line 589

Quoted 10 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +10 -0
+		return -ENODEV;
+	}
+
+	/* Set TWI internal clock as 10MHz */
+	bfin_write_TWI_CONTROL(((get_sclk() / 1024 / 1024 + 5) / 10) & 0x7F);
+
+	/* Set Twi interface clock as specified */
+	if (CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 400)
+		bfin_write_TWI_CLKDIV((( 5*1024 / 400 ) << 8) |
+			(( 5*1024 / 400 ) & 0xFF));

This can never happen, as you have a range defined in Kconfig.

Quoted 17 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +17 -0
+	else
+		bfin_write_TWI_CLKDIV((( 5*1024 / CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ )
+			<< 8) | (( 5*1024 / CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ )
+			& 0xFF));
+
+	/* Enable TWI */
+	bfin_write_TWI_CONTROL(bfin_read_TWI_CONTROL() | TWI_ENA);
+	SSYNC();
+
+	rc = i2c_add_adapter(p_adap);
+	if (rc < 0)
+		free_irq(iface->irq, iface);
+	else
+		platform_set_drvdata(dev, iface);
+
+	return rc;
+}

drivers/i2c/busses/i2c-bfin-twi.c line 628

Quoted 10 lines
drivers/i2c/busses/i2c-bfin-twi.c new file +9 -0
 
+static struct platform_driver i2c_bfin_twi_driver = {
+	.probe		= i2c_bfin_twi_probe,
+	.remove		= i2c_bfin_twi_remove,
+	.suspend	= i2c_bfin_twi_suspend,
+	.resume		= i2c_bfin_twi_resume,
+	.driver		= {
+		.name	= "i2c-bfin-twi",
+	},
+};

Missing .owner = THIS_MODULE.

Thanks,

Signature · 1 line
Jean Delvare
#7 lore raw
Quoted 6 lines · Jean Delvare
Quoted 1 line

+ p_adap->class = I2C_CLASS_ALL;

This pretty much voids the point of these probing classes. You should only select the classes matching devices which may actually be probed for on this bus. If different boards have different needs, get the right classes from the platform data.

i asked about the class issue previously specifically for this bus driver and was told that they werent really fully defined ... the on-chip I2C interface on the Blackfin chip can handle any I2C device which is why i added this line

any examples of how to go about doing it via boards ? i dont see any other I2C bus driver doing it that way ...
-mike

#8 lore raw

Hi Mike,

Quoted 8 lines · Mike Frysinger
Quoted 6 lines · Jean Delvare
Quoted 1 line

+ p_adap->class = I2C_CLASS_ALL;

This pretty much voids the point of these probing classes. You should only select the classes matching devices which may actually be probed for on this bus. If different boards have different needs, get the right classes from the platform data.

Quoted 4 lines

i asked about the class issue previously specifically for this bus driver and was told that they werent really fully defined ... the on-chip I2C interface on the Blackfin chip can handle any I2C device which is why i added this line

Grep for I2C_CLASS_ and you'll see that the classes are defined and used consistently in the kernel tree. Maybe just not by the embedded folks.

Also note that what matters is not the type of devices that can be connected on the bus, but the type of devices that can be probed for. This was essentially the same so far (general probing was the only way to instantiate an i2c device), but the new i2c-core code that will go into 2.6.22 will make it different. These classes will probably be less used in the future.

Quoted 2 lines

any examples of how to go about doing it via boards ? i dont see any other I2C bus driver doing it that way ...

I don't think many drivers from the embedded world do it properly. But they don't appear to define any class at all, which means that in practice they keep most regular i2c drivers out of their bus. Pretty much the contrary of what you're doing with I2C_CLASS_ALL.

It's really up to you, but if you set the class to I2C_CLASS_ALL, don't later come and complain that other drivers keep probing your I2C bus and possibly attach to your I2C devices. You asked for it.

Signature · 1 line
Jean Delvare
#9 lore raw
Quoted 18 lines · Jean Delvare
Quoted 8 lines · Mike Frysinger
Quoted 6 lines · Jean Delvare
Quoted 1 line

+ p_adap->class = I2C_CLASS_ALL;

This pretty much voids the point of these probing classes. You should only select the classes matching devices which may actually be probed for on this bus. If different boards have different needs, get the right classes from the platform data.

Quoted 4 lines

i asked about the class issue previously specifically for this bus driver and was told that they werent really fully defined ... the on-chip I2C interface on the Blackfin chip can handle any I2C device which is why i added this line

Grep for I2C_CLASS_ and you'll see that the classes are defined and used consistently in the kernel tree. Maybe just not by the embedded folks.

Quoted 6 lines

Also note that what matters is not the type of devices that can be connected on the bus, but the type of devices that can be probed for. This was essentially the same so far (general probing was the only way to instantiate an i2c device), but the new i2c-core code that will go into 2.6.22 will make it different. These classes will probably be less used in the future.

was referring to this:
http://lists.lm-sensors.org/pipermail/i2c/2007-February/000770.html

Quoted 4 lines
Quoted 2 lines

any examples of how to go about doing it via boards ? i dont see any other I2C bus driver doing it that way ...

I don't think many drivers from the embedded world do it properly.

which is why i asked if you had any hints ... if no one else is doing it, it means no standard exists, which means there's nothing for me to copy :)

it isnt obvious to me how to leverage the normal platform_device and resource structures to achieve moving the class out of the driver and into a boards file ... i'm perfectly happy doing this knowing what the correct direction to take things ... but i guess this gets us back to if no one is doing this yet and no one has any proposals, it's on our shoulders to get it resolved satisfactorily

i might also argue (for fun) that some of the bus drivers arent doing it correctly ... they should implement support for the specific I2C implementation and then a different file for the specific [mother]board would contain the specific details

Quoted 4 lines

But
they don't appear to define any class at all, which means that in practice they keep most regular i2c drivers out of their bus. Pretty much the contrary of what you're doing with I2C_CLASS_ALL.

this is certainly true, but i'd rather have the default behavior be "allow any I2C device on the bus" than "only allow I2C devices that dont check the bus type" ...

Quoted 3 lines

It's really up to you, but if you set the class to I2C_CLASS_ALL, don't later come and complain that other drivers keep probing your I2C bus and possibly attach to your I2C devices. You asked for it.

i dont see why we would complain when it's obviously our fault ;)

it also wouldnt be a real issue as it's an embedded board, so it'd only have the drivers enabled for devices that are actually hooked up
...
-mike

#10 lore raw
Quoted 6 lines · Mike Frysinger

it isnt obvious to me how to leverage the normal platform_device and resource structures to achieve moving the class out of the driver and into a boards file ... i'm perfectly happy doing this knowing what the correct direction to take things ... but i guess this gets us back to if no one is doing this yet and no one has any proposals, it's on our shoulders to get it resolved satisfactorily

I'd say: leave things as they are, if a problem arises, we'll see how to fix it. By then, the new i2c driver model will be in mainline, which will gives us a nicer way to address the problem.

Quoted 4 lines

i might also argue (for fun) that some of the bus drivers arent doing it correctly ... they should implement support for the specific I2C implementation and then a different file for the specific [mother]board would contain the specific details

This sounds easy when you're in the embedded world. Now come and try to support the literally thousands of different PC motherboard models out there that way!

Signature · 1 line
Jean Delvare
author
#11 lore raw

PATCH 1/1 Blackfin: on-chip Two Wire Interface I2C driver

Hi folks,

Changlogs:

a) Fixed issues according to Jean's review.
b) Add MAINTAINS infomation
c) add I2C_HW_B_BLACKFIN to i2c-id.h

[PATCH] Blackfin: on-chip Two Wire Interface I2C driver

The i2c linux driver for blackfin architecture which supports blackfin on-chip TWI controller i2c operation.

Signed-off-by Bryan Wu <[email protected]>
Reviewed-by Andrew Morton <[email protected]>
Reviewed-by Alexey Dobriyan <[email protected]>
Reviewed-by Jean Delvare <[email protected]>
Cc David Brownell <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 MAINTAINERS                       |    7 
 drivers/i2c/busses/Kconfig        |   16 
 drivers/i2c/busses/Makefile       |    1 
 drivers/i2c/busses/i2c-bfin-twi.c |  644 ++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-id.h            |    1 
 5 files changed, 669 insertions(+)
drivers/i2c/busses/Kconfig +16 -0
Index: linux-2.6/drivers/i2c/busses/Kconfig
===================================================================
@@ -91,6 +91,22 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-au1550.
 
+config I2C_BLACKFIN_TWI
+	tristate "Blackfin TWI I2C support"
+	depends on I2C && (BF534 || BF536 || BF537)
+	help
+	  This is the TWI I2C device driver for Blackfin 534/536/537.
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-bfin-twi.
+
+config I2C_BLACKFIN_TWI_CLK_KHZ
+	int "Blackfin TWI I2C clock (kHz)"
+	depends on I2C_BLACKFIN_TWI
+	range 10 400
+	default 50
+	help
+	  The unit of the TWI clock is kilo HZ.
+
 config I2C_ELEKTOR
 	tristate "Elektor ISA card"
 	depends on I2C && ISA && BROKEN_ON_SMP
drivers/i2c/busses/Makefile +1 -0
Index: linux-2.6/drivers/i2c/busses/Makefile
===================================================================
@@ -10,6 +10,7 @@
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
+obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o
drivers/i2c/busses/i2c-bfin-twi.c new file +644 -0

644 lines not on this page · raw message

MAINTAINERS +7 -0
Index: linux-2.6/MAINTAINERS
===================================================================
@@ -731,6 +731,13 @@
 W:     http://blackfin.uclinux.org
 S:     Supported
 
+BLACKFIN I2C TWI DRIVER
+P:	Sonic Zhang
+M:	[email protected]
+L:	[email protected] (subscribers-only)
+W:	http://blackfin.uclinux.org
+S:	Supported
+
 BAYCOM/HDLCDRV DRIVERS FOR AX.25
 P:	Thomas Sailer
 M:	[email protected]
include/linux/i2c-id.h +1 -0
Index: linux-2.6/include/linux/i2c-id.h
===================================================================
@@ -195,6 +195,7 @@
 #define I2C_HW_B_EM28XX		0x01001f /* em28xx video capture cards */
 #define I2C_HW_B_CX2341X	0x010020 /* Conexant CX2341X MPEG encoder cards */
 #define I2C_HW_B_INTELFB	0x010021 /* intel framebuffer driver */
+#define I2C_HW_B_BLACKFIN	0x010022 /* ADI Blackfin I2C TWI driver */
 
 /* --- PCF 8584 based algorithms					*/
 #define I2C_HW_P_LP		0x020000 /* Parallel port interface */

_

Thanks
-Bryan

#12 lore raw

Hi Bryan,

Quoted 5 lines · Bryan

Changlogs:

a) Fixed issues according to Jean's review.
b) Add MAINTAINS infomation
c) add I2C_HW_B_BLACKFIN to i2c-id.h

I2C_HW_B_* is traditionally used for drivers built on top of the i2c-algo-bit driver, which isn't the case of your driver, so it's a bit confusing. Please instead use:

#define I2C_HW_BLACKFIN 0x190001

I hope we'll be able to get rid of these IDs soon, so we no longer have to care about this mess.

Other than that I'm OK with the patch this time, I'll push it on my stack. I'll fix the ID issue myself, no need to resend. This also means that, from now on, any change to this driver should be provided as an incremental patch on top of this version.

Thanks,

Signature · 1 line
Jean Delvare
author
#13 lore raw
Quoted 9 lines · Jean Delvare

Hi Bryan,

Quoted 5 lines · Bryan

Changlogs:

a) Fixed issues according to Jean's review.
b) Add MAINTAINS infomation
c) add I2C_HW_B_BLACKFIN to i2c-id.h

Quoted 8 lines

I2C_HW_B_* is traditionally used for drivers built on top of the i2c-algo-bit driver, which isn't the case of your driver, so it's a bit confusing. Please instead use:

#define I2C_HW_BLACKFIN 0x190001

I hope we'll be able to get rid of these IDs soon, so we no longer have to care about this mess.

Thanks, I appreciate. When you want to get rid of these IDs, I will do the removing ID job related to this patch.

Quoted 7 lines

Other than that I'm OK with the patch this time, I'll push it on my stack. I'll fix the ID issue myself, no need to resend. This also means that, from now on, any change to this driver should be provided as an incremental patch on top of this version.

Thanks,

OK, I will follow this rule definitely.
Thanks again
-Bryan Wu