Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Mon, Sep 17, 2007 at 07:34:08AM +0200, Stefan Roese wrote: My understanding was, that adding many #ifdef's into the code was not the preferred way. But just making a copy seems to be a proffered one? Wow. OCP and/or OF part is quite small part of the driver; another approach would been completely splitting OCP and OF specific part out (e.g. i2c-ibm_iic.c + i2c-ibm_iic_ocp.c / i2c_ibm_iic_of.c). I personally thing it's not worth the effort and just adding couple of ifdef'ed code is good enough, especially as a transitional thing. I could of course change this patch to not add an additional driver but extend the existing driver with a bunch of #ifdef's to support both versions. This approach of multiple drivers seems to be common in the kernel right now: drivers/mtd/maps/physmap.c drivers/mtd/maps/physmap_of.c or drivers/usb/host/ohci-ppc-soc.c drivers/usb/host/ohci-ppc-of.c I don't think these are good examples, physmap.c seems to differ from physmap_of.c significantly. These drivers are mostly a glue, and it makes sense to have different versions, because there isn't much there except for platform/bus/glue specific code. Any other opinions on this? How should this be handled to get accepted upstream? Two different drivers with removing the old one later when arch/ppc is gone, ppc has been going away for the last two years at least and still isn't gone. What makes you think this isn't gonna take another year or two :) ? The old name i2c-ibm_iic is kind of redundant. Nearly all bus drivers are named i2c-platform. Perhaps a better name would be i2c-ppc4xx then. Sure, that'd be a much better choice. -- Eugene ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
Hi Stefan, On Mon, 17 Sep 2007 07:34:08 +0200, Stefan Roese wrote: On Sunday 16 September 2007, Eugene Surovegin wrote: Hmm, I just noticed that you basically added a copy of existing driver with small changes to support OF while keeping OCP one. Why not just add OF support to the existing code (under some ifdef), and then remove OCP support as soon as ppc - powerpc transition is finished? Why have two almost identical code in the tree? My understanding was, that adding many #ifdef's into the code was not the preferred way. I could of course change this patch to not add an additional driver but extend the existing driver with a bunch of #ifdef's to support both versions. This approach of multiple drivers seems to be common in the kernel right now: drivers/mtd/maps/physmap.c drivers/mtd/maps/physmap_of.c or drivers/usb/host/ohci-ppc-soc.c drivers/usb/host/ohci-ppc-of.c Any other opinions on this? How should this be handled to get accepted upstream? Two different drivers with removing the old one later when arch/ppc is gone, or one driver which supports both versions and removing the ocp support in this driver later? I'd prefer a single driver with ifdef's. Only the registration and cleanup parts should be different, the actual I2C bus driving code should be pretty much the same. With some efforts it should be possible to reduce the ifdef clutter to a minimum. (...) The old name i2c-ibm_iic is kind of redundant. Nearly all bus drivers are named i2c-platform. Perhaps a better name would be i2c-ppc4xx then. Agreed. But that being said, changing the name now while the old name has been used for years might cause more trouble than it solves. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Saturday 15 September 2007, Stephen Rothwell wrote: [Just some trivial things] Thanks for the feedback. I'll change those things and resubmit. Best regads, Stefan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] i2c: devtree-aware iic support for PPC4xx
This patch reworks existing ibm-iic driver to an of_platform_device
and enables it to talk to device tree directly. The ocp quirks are
completely removed by this patch.
This is done to enable I2C support for the PPC4xx platforms now
being moved from arch/ppc (OCP) to arch/powerpc (of). The first board
using this driver will be the AMCC Sequoia (PPC440EPx).
Signed-off-by: Stefan Roese [EMAIL PROTECTED]
---
2nd try with some cleanups. Please let me know if there still are some
problems.
Thanks.
commit 5748f81ff53277fa5c16de815b7d6b172ca284e9
tree 8284b3f1c836eb6eb06ee6882ee13b9e8f6cbb6b
parent d0174640eedc1cd756754f03afe2dbb3d56de74e
author Stefan Roese [EMAIL PROTECTED] Sun, 16 Sep 2007 13:46:40 +0200
committer Stefan Roese [EMAIL PROTECTED] Sun, 16 Sep 2007 13:46:40 +0200
drivers/i2c/busses/Kconfig | 12 -
drivers/i2c/busses/Makefile |1
drivers/i2c/busses/i2c-ibm_iic.h |3
drivers/i2c/busses/i2c-ibm_of.c | 858 ++
4 files changed, 873 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 9f3a4cd..12453e2 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -220,7 +220,17 @@ config I2C_PIIX4
config I2C_IBM_IIC
tristate IBM PPC 4xx on-chip I2C interface
- depends on IBM_OCP
+ depends on !PPC_MERGE
+ help
+ Say Y here if you want to use IIC peripheral found on
+ embedded IBM PPC 4xx based systems.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-ibm_iic.
+
+config I2C_IBM_OF
+ tristate IBM PPC 4xx on-chip I2C interface
+ depends on PPC_MERGE
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/Makefile b/drivers/i2c/busses/Makefile
index 5b752e4..0cd0bac 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
obj-$(CONFIG_I2C_I801) += i2c-i801.o
obj-$(CONFIG_I2C_I810) += i2c-i810.o
obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
+obj-$(CONFIG_I2C_IBM_OF) += i2c-ibm_of.o
obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
obj-$(CONFIG_I2C_IXP2000) += i2c-ixp2000.o
obj-$(CONFIG_I2C_IXP4XX) += i2c-ixp4xx.o
diff --git a/drivers/i2c/busses/i2c-ibm_iic.h b/drivers/i2c/busses/i2c-ibm_iic.h
index 59d7b43..485c72c 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.h
+++ b/drivers/i2c/busses/i2c-ibm_iic.h
@@ -23,6 +23,7 @@
#define __I2C_IBM_IIC_H_
#include linux/i2c.h
+#include linux/of.h
struct iic_regs {
u16 mdbuf;
@@ -50,6 +51,8 @@ struct ibm_iic_private {
int irq;
int fast_mode;
u8 clckdiv;
+ struct device_node *np;
+ phys_addr_t paddr;
};
/* IICx_CNTL register */
diff --git a/drivers/i2c/busses/i2c-ibm_of.c b/drivers/i2c/busses/i2c-ibm_of.c
new file mode 100644
index 000..5e5b3e5
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ibm_of.c
@@ -0,0 +1,858 @@
+/*
+ * drivers/i2c/busses/i2c-ibm_of.c
+ *
+ * Support for the IIC peripheral on IBM PPC 4xx
+ *
+ * Copyright (c) 2003, 2004 Zultys Technologies.
+ * Eugene Surovegin [EMAIL PROTECTED] or [EMAIL PROTECTED]
+ *
+ * Based on original work by
+ * Ian DaSilva [EMAIL PROTECTED]
+ * Armin Kuster [EMAIL PROTECTED]
+ * Matt Porter [EMAIL PROTECTED]
+ *
+ * Copyright 2000-2003 MontaVista Software Inc.
+ *
+ * Original driver version was highly leveraged from i2c-elektor.c
+ *
+ * Copyright 1995-97 Simon G. Vogl
+ *1998-99 Hans Berglund
+ *
+ * With some changes from Ky�sti M�lkki [EMAIL PROTECTED]
+ * and even Frodo Looijaard [EMAIL PROTECTED]
+ *
+ * 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/kernel.h
+#include linux/ioport.h
+#include linux/delay.h
+#include linux/slab.h
+#include linux/init.h
+#include linux/interrupt.h
+#include asm/irq.h
+#include asm/io.h
+#include linux/i2c.h
+#include linux/i2c-id.h
+
+#include linux/of_platform.h
+
+#include i2c-ibm_iic.h
+
+#define DRIVER_VERSION 2.1
+
+MODULE_DESCRIPTION(IBM IIC driver v DRIVER_VERSION);
+MODULE_LICENSE(GPL);
+
+static int device_idx = -1;
+
+static int iic_force_poll;
+module_param(iic_force_poll, bool, 0);
+MODULE_PARM_DESC(iic_force_poll, Force polling mode);
+
+static int iic_force_fast;
+module_param(iic_force_fast, bool, 0);
+MODULE_PARM_DESC(iic_fast_poll, Force fast mode (400 kHz));
+
+#define DBG_LEVEL 0
+
+#ifdef DBG
+#undef DBG
+#endif
+
+#ifdef DBG2
+#undef DBG2
+#endif
+
+#if DBG_LEVEL 0
+# define DBG(f,x...) printk(KERN_DEBUG ibm-iic f, ##x)
+#else
+# define
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sun, 16 Sep 2007 18:27:47 +0200 Robert Schwebel [EMAIL PROTECTED] wrote: On Sun, Sep 16, 2007 at 01:52:02PM +0200, Stefan Roese wrote: diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 9f3a4cd..12453e2 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -220,7 +220,17 @@ config I2C_PIIX4 config I2C_IBM_IIC tristate IBM PPC 4xx on-chip I2C interface - depends on IBM_OCP + depends on !PPC_MERGE + help + Say Y here if you want to use IIC peripheral found on + embedded IBM PPC 4xx based systems. Can we agree on one nomenclature - either i2c or iic? + This driver can also be built as a module. If so, the module + will be called i2c-ibm_iic. Are these drivers the same functionality (host i2c driver for 4xx)? If yes, shouldn't all in-tree users be migrated over and the old style driver be removed (with deprecation period)? They are the same functionality, but for two different versions of the arch. The old one is arch/ppc, the new one is arch/powerpc. 4xx is being migrated to arch/powerpc so eventually what you say will happen. The arch/ppc tree is scheduled for removal in June of 2008. For now, we need both drivers since not everything in 4xx has moved yet. josh ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sun, Sep 16, 2007 at 01:52:02PM +0200, Stefan Roese wrote: diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 9f3a4cd..12453e2 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -220,7 +220,17 @@ config I2C_PIIX4 config I2C_IBM_IIC tristate IBM PPC 4xx on-chip I2C interface - depends on IBM_OCP + depends on !PPC_MERGE + help + Say Y here if you want to use IIC peripheral found on + embedded IBM PPC 4xx based systems. Can we agree on one nomenclature - either i2c or iic? + This driver can also be built as a module. If so, the module + will be called i2c-ibm_iic. Are these drivers the same functionality (host i2c driver for 4xx)? If yes, shouldn't all in-tree users be migrated over and the old style driver be removed (with deprecation period)? +#define DBG_LEVEL 0 + +#ifdef DBG +#undef DBG +#endif + +#ifdef DBG2 +#undef DBG2 +#endif + +#if DBG_LEVEL 0 +# define DBG(f,x...)printk(KERN_DEBUG ibm-iic f, ##x) +#else +# define DBG(f,x...)((void)0) +#endif +#if DBG_LEVEL 1 +# define DBG2(f,x...) DBG(f, ##x) +#else +# define DBG2(f,x...) ((void)0) +#endif Any reason why you can't use pr_debug? Robert -- Pengutronix - Linux Solutions for Science and Industry Entwicklungszentrum Nord http://www.pengutronix.de ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sun, Sep 16, 2007 at 01:52:02PM +0200, Stefan Roese wrote: This patch reworks existing ibm-iic driver to an of_platform_device and enables it to talk to device tree directly. The ocp quirks are completely removed by this patch. This is done to enable I2C support for the PPC4xx platforms now being moved from arch/ppc (OCP) to arch/powerpc (of). The first board using this driver will be the AMCC Sequoia (PPC440EPx). Signed-off-by: Stefan Roese [EMAIL PROTECTED] --- 2nd try with some cleanups. Please let me know if there still are some problems. Thanks. commit 5748f81ff53277fa5c16de815b7d6b172ca284e9 tree 8284b3f1c836eb6eb06ee6882ee13b9e8f6cbb6b parent d0174640eedc1cd756754f03afe2dbb3d56de74e author Stefan Roese [EMAIL PROTECTED] Sun, 16 Sep 2007 13:46:40 +0200 committer Stefan Roese [EMAIL PROTECTED] Sun, 16 Sep 2007 13:46:40 +0200 drivers/i2c/busses/Kconfig | 12 - drivers/i2c/busses/Makefile |1 drivers/i2c/busses/i2c-ibm_iic.h |3 drivers/i2c/busses/i2c-ibm_of.c | 858 ++ 4 files changed, 873 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 9f3a4cd..12453e2 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -220,7 +220,17 @@ config I2C_PIIX4 config I2C_IBM_IIC tristate IBM PPC 4xx on-chip I2C interface - depends on IBM_OCP + depends on !PPC_MERGE + help + Say Y here if you want to use IIC peripheral found on + embedded IBM PPC 4xx based systems. + + This driver can also be built as a module. If so, the module + will be called i2c-ibm_iic. + +config I2C_IBM_OF + tristate IBM PPC 4xx on-chip I2C interface + depends on PPC_MERGE 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/Makefile b/drivers/i2c/busses/Makefile index 5b752e4..0cd0bac 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o obj-$(CONFIG_I2C_I801) += i2c-i801.o obj-$(CONFIG_I2C_I810) += i2c-i810.o obj-$(CONFIG_I2C_IBM_IIC)+= i2c-ibm_iic.o +obj-$(CONFIG_I2C_IBM_OF) += i2c-ibm_of.o obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o obj-$(CONFIG_I2C_IXP2000)+= i2c-ixp2000.o obj-$(CONFIG_I2C_IXP4XX) += i2c-ixp4xx.o Hmm, I just noticed that you basically added a copy of existing driver with small changes to support OF while keeping OCP one. Why not just add OF support to the existing code (under some ifdef), and then remove OCP support as soon as ppc - powerpc transition is finished? Why have two almost identical code in the tree? I also personally don't like this _iic - _of name change (you removed peripheral name and added something which has nothing to do with iic, I never heard of OF peripheral in 4xx chips). Whether you use OCP or OF to pass a little information is quite irrelevant to the iic driver operation. If you insist on this approach, please add yourself as a maintainer of this code, because I'm not going to support two identical copies of my code in the kernel tree. -- Eugene ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sun, Sep 16, 2007 at 11:53:30AM -0700, Eugene Surovegin wrote: On Sun, Sep 16, 2007 at 01:52:02PM +0200, Stefan Roese wrote: This patch reworks existing ibm-iic driver to an of_platform_device and enables it to talk to device tree directly. The ocp quirks are completely removed by this patch. This is done to enable I2C support for the PPC4xx platforms now being moved from arch/ppc (OCP) to arch/powerpc (of). The first board using this driver will be the AMCC Sequoia (PPC440EPx). Signed-off-by: Stefan Roese [EMAIL PROTECTED] --- 2nd try with some cleanups. Please let me know if there still are some problems. Thanks. commit 5748f81ff53277fa5c16de815b7d6b172ca284e9 tree 8284b3f1c836eb6eb06ee6882ee13b9e8f6cbb6b parent d0174640eedc1cd756754f03afe2dbb3d56de74e author Stefan Roese [EMAIL PROTECTED] Sun, 16 Sep 2007 13:46:40 +0200 committer Stefan Roese [EMAIL PROTECTED] Sun, 16 Sep 2007 13:46:40 +0200 drivers/i2c/busses/Kconfig | 12 - drivers/i2c/busses/Makefile |1 drivers/i2c/busses/i2c-ibm_iic.h |3 drivers/i2c/busses/i2c-ibm_of.c | 858 ++ 4 files changed, 873 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 9f3a4cd..12453e2 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -220,7 +220,17 @@ config I2C_PIIX4 config I2C_IBM_IIC tristate IBM PPC 4xx on-chip I2C interface - depends on IBM_OCP + depends on !PPC_MERGE + help + Say Y here if you want to use IIC peripheral found on + embedded IBM PPC 4xx based systems. + + This driver can also be built as a module. If so, the module + will be called i2c-ibm_iic. + +config I2C_IBM_OF + tristate IBM PPC 4xx on-chip I2C interface + depends on PPC_MERGE 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/Makefile b/drivers/i2c/busses/Makefile index 5b752e4..0cd0bac 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o obj-$(CONFIG_I2C_I801) += i2c-i801.o obj-$(CONFIG_I2C_I810) += i2c-i810.o obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o +obj-$(CONFIG_I2C_IBM_OF) += i2c-ibm_of.o obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o obj-$(CONFIG_I2C_IXP2000) += i2c-ixp2000.o obj-$(CONFIG_I2C_IXP4XX) += i2c-ixp4xx.o Hmm, I just noticed that you basically added a copy of existing driver with small changes to support OF while keeping OCP one. Why not just add OF support to the existing code (under some ifdef), and then remove OCP support as soon as ppc - powerpc transition is finished? Why have two almost identical code in the tree? I also personally don't like this _iic - _of name change (you removed peripheral name and added something which has nothing to do with iic, I never heard of OF peripheral in 4xx chips). Whether you use OCP or OF to pass a little information is quite irrelevant to the iic driver operation. I concur on this point. Especially since on 4xx the device tree will generally be a flattened device tree which is vaguely OF-like, but not from an actual Open Firmware. If you insist on this approach, please add yourself as a maintainer of this code, because I'm not going to support two identical copies of my code in the kernel tree. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sun, Sep 16, 2007 at 06:27:47PM +0200, Robert Schwebel wrote: On Sun, Sep 16, 2007 at 01:52:02PM +0200, Stefan Roese wrote: diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 9f3a4cd..12453e2 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -220,7 +220,17 @@ config I2C_PIIX4 config I2C_IBM_IIC tristate IBM PPC 4xx on-chip I2C interface - depends on IBM_OCP + depends on !PPC_MERGE + help + Say Y here if you want to use IIC peripheral found on + embedded IBM PPC 4xx based systems. Can we agree on one nomenclature - either i2c or iic? The dual nomenclature comes because linux uses i2c throughout the subsystem, but all the hardware documentation refers to the controller ASIC in question as 'IIC'. So I think the convention is that 'i2c' is used to refer to the type of bus in general, 'iic' is used to refer to this particular type of bus controller. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sunday 16 September 2007, Eugene Surovegin wrote: Hmm, I just noticed that you basically added a copy of existing driver with small changes to support OF while keeping OCP one. Why not just add OF support to the existing code (under some ifdef), and then remove OCP support as soon as ppc - powerpc transition is finished? Why have two almost identical code in the tree? My understanding was, that adding many #ifdef's into the code was not the preferred way. I could of course change this patch to not add an additional driver but extend the existing driver with a bunch of #ifdef's to support both versions. This approach of multiple drivers seems to be common in the kernel right now: drivers/mtd/maps/physmap.c drivers/mtd/maps/physmap_of.c or drivers/usb/host/ohci-ppc-soc.c drivers/usb/host/ohci-ppc-of.c Any other opinions on this? How should this be handled to get accepted upstream? Two different drivers with removing the old one later when arch/ppc is gone, or one driver which supports both versions and removing the ocp support in this driver later? I also personally don't like this _iic - _of name change (you removed peripheral name and added something which has nothing to do with iic, I never heard of OF peripheral in 4xx chips). Whether you use OCP or OF to pass a little information is quite irrelevant to the iic driver operation. The old name i2c-ibm_iic is kind of redundant. Nearly all bus drivers are named i2c-platform. Perhaps a better name would be i2c-ppc4xx then. This of name was borrowed from already existing device-tree aware drivers like drivers/mtd/maps/physmap_of.c or drivers/usb/host/ohci-ppc-of.c. If you insist on this approach, please add yourself as a maintainer of this code, because I'm not going to support two identical copies of my code in the kernel tree. I insist in nothing. I'm just trying to get this device-tree aware I2C driver support upstream. Best regards, Stefan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Mon, Sep 17, 2007 at 07:34:08AM +0200, Stefan Roese wrote: On Sunday 16 September 2007, Eugene Surovegin wrote: Hmm, I just noticed that you basically added a copy of existing driver with small changes to support OF while keeping OCP one. Why not just add OF support to the existing code (under some ifdef), and then remove OCP support as soon as ppc - powerpc transition is finished? Why have two almost identical code in the tree? My understanding was, that adding many #ifdef's into the code was not the preferred way. I could of course change this patch to not add an additional driver but extend the existing driver with a bunch of #ifdef's to support both versions. #ifdefs are yucky, but so is duplication. I'm not sure which is the lesser evil in this case. This approach of multiple drivers seems to be common in the kernel right now: drivers/mtd/maps/physmap.c drivers/mtd/maps/physmap_of.c Not a relevant example. Despite the names, physmap and physmap_of don't really do the same thing at all. I've been meaning to rename physmap_of... or drivers/usb/host/ohci-ppc-soc.c drivers/usb/host/ohci-ppc-of.c Also ibm_emac vs. ibm_new_emac (not merged yet). Any other opinions on this? How should this be handled to get accepted upstream? Two different drivers with removing the old one later when arch/ppc is gone, or one driver which supports both versions and removing the ocp support in this driver later? I also personally don't like this _iic - _of name change (you removed peripheral name and added something which has nothing to do with iic, I never heard of OF peripheral in 4xx chips). Whether you use OCP or OF to pass a little information is quite irrelevant to the iic driver operation. The old name i2c-ibm_iic is kind of redundant. Nearly all bus drivers are named i2c-platform. Perhaps a better name would be i2c-ppc4xx then. This of name was borrowed from already existing device-tree aware drivers like drivers/mtd/maps/physmap_of.c or drivers/usb/host/ohci-ppc-of.c. 'ibm' is not specific enough - it's not like it's used on even a very large fraction of ibm platforms - and 'of' is verging on misleading (since OF != device tree, although they're related). 'iic' isn't arbitrary - it comes from the name used in the documentation. Although 'i2c-ppc4xx' probably is a better name, in any case. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
On Sat, Sep 15, 2007 at 11:08:01AM +0200, Stefan Roese wrote:
This patch reworks existing ibm-iic driver to an of_platform_device
and enables it to talk to device tree directly. The ocp quirks are
completely removed by this patch.
This is done to enable I2C support for the PPC4xx platforms now
being moved from arch/ppc (ocp) to arch/powerpc (of). The first board
using this driver will be the AMCC Sequoia (PPC440EPx).
Signed-off-by: Stefan Roese [EMAIL PROTECTED]
+ /* clckdiv is the same for *all* IIC interfaces,
+ * but I'd rather make a copy than introduce another global. --ebs
+ */
+ /* Parent bus should have frequency filled */
+ prop = of_get_property(of_get_parent(np), clock-frequency, len);
+ if (prop == NULL) {
+ printk(KERN_ERR
+ ibm-iic(%s):no clock-frequency prop on parent bus!\n,
+ dev-np-full_name);
+ goto fail;
+ }
+
+ DBG(%s: clckdiv = %d\n, dev-np-full_name, dev-clckdiv);
+
Where is dev-clkdiv initialized?
My original version used iic_clkdiv() to calculate correct devider
based on OPB frequency. Did you even test this code?
--
Eugene
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c: devtree-aware iic support for PPC4xx
[Just some trivial things]
On Sat, 15 Sep 2007 11:08:01 +0200 Stefan Roese [EMAIL PROTECTED] wrote:
+++ b/drivers/i2c/busses/i2c-ibm_iic.h
@@ -22,7 +22,8 @@
#ifndef __I2C_IBM_IIC_H_
#define __I2C_IBM_IIC_H_
-#include linux/i2c.h
+#include linux/i2c.h
+#include asm/prom.h
Please include linux/of.h, of_device.h or of_platform.h (as appropriate)
instead of asm/prom.h. In this case, since you want struct device_node,
include linux/of.h.
+static irqreturn_t iic_handler(int irq, void *dev_id)
+{
+ struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
You shouldn't cast from (void *).
+static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+ struct ibm_iic_private* dev = (struct
ibm_iic_private*)(i2c_get_adapdata(adap))i
Again the return value of i2c_get_adapdata() is (void *) so shouldn't be
cast.
+ prop = of_get_property(np, iic-mode, len);
+ /* use 400kHz only if stated in dts, 100kHz otherwise */
+ if (prop != NULL) {
+ if (*prop == 400)
+ dev-fast_mode = 1;
+ } else
+ dev-fast_mode = 0;
dev-fast_mode = (prop (*prop == 400));
+static int __devexit iic_remove(struct of_device *ofdev)
+{
+ struct ibm_iic_private *dev =
+ (struct ibm_iic_private *)dev_get_drvdata(ofdev-dev);
dev_get_drvdata() also returns (void *).
+static struct of_platform_driver ibm_iic_driver = {
+ .name = ibm-iic,
+ .match_table = ibm_iic_match,
+ .probe = iic_probe,
+ .remove = iic_remove,
+#if defined(CONFIG_PM)
+ .suspend = NULL,
+ .resume = NULL,
+#endif
These last two will be NULL anyway, so just leave them out.
--
Cheers,
Stephen Rothwell[EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/
pgp9tlYmvJvWF.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
