LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] i2c-mpc: add support for the MPC512x processors from Freescale
From: Wolfgang Grandegger @ 2010-01-25 18:33 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Devicetree-discuss, Linuxppc-dev, Linux-i2c, Wolfgang Grandegger
In-Reply-To: <20100125151509.GD5257@pengutronix.de>

Wolfram Sang wrote:
>>>>  
>>>> -static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
>>>> -					    struct mpc_i2c *i2c,
>>>> -					    u32 clock, u32 prescaler)
>>>> +static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
>>>> +					 struct mpc_i2c *i2c,
>>>> +					 u32 clock, u32 prescaler)
>>>>  {
>>>>  	int ret, fdr;
>>>>  
>>>> +	if (clock == -1) {
>>> Could we use 0 for 'no_clock'? This would make the above statement simply
>> "0" is already used to maintain backward compatibility setting a safe
>> divider.
> 
> Ah, now I see:
> 
> 'clock == -1' means 'preserve clocks' (and is checked here in mpc_i2c_setup_52xx())

Yes, this is now necessary because "setup" does not just do clock settings.

> 'clock ==  0' means 'safe divider' (and is checked in mpc_i2c_get_fdr_52xx())

This is for compatibility with old DTS files and last time it was tricky
 to get that right and therefore...

> This is not a beauty ;)
> 
> What about adding a flags variable to the setup-functions?

.. I hesitate to make bigger changes to the code flow, which the
introduction of a flags variable would required. Also it seems to be
overkill to me. I will have a closer look, though. At a minimum I will
replace "-1" with "MPC_I2C_PRESERVE_CLOCK".

Wolfgang.

^ permalink raw reply

* spi_mpc8xxx: Setting cs_change at the last transfer of a message
From: Torsten Fleischer @ 2010-01-25 16:21 UTC (permalink / raw)
  To: spi-devel-general; +Cc: linuxppc-dev

As stated in spi.h (at struct spi_transfer) the driver can keep the chip 
select of the last device active if cs_change is set at the last transfer of a 
message.
The spi_mpc8xxx driver always deactivates the chip select after completing the 
message regardless of cs_change is set or not.

I'd like to patch this. But before doing that I'd like to learn if there are 
any known use cases where the above described behavior of the driver is 
needed.


Thanks
Torsten

^ permalink raw reply

* Re: [PATCH/RFC 1/2] 5200: improve i2c bus error recovery
From: Albrecht Dreß @ 2010-01-25 20:21 UTC (permalink / raw)
  To: Ben Dooks; +Cc: Linux PPC Development, Devicetree Discussions
In-Reply-To: <20100125040653.GS10014@trinity.fluff.org>

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

Am 25.01.10 05:06 schrieb(en) Ben Dooks:
> On Fri, Jan 22, 2010 at 09:17:55PM +0100, Albrecht Dreß wrote:
>> Improve the recovery of the MPC5200B's I2C bus from errors like bus hangs.
> 
> This is very sparse comapred to the large comment below the --- line, maybe some more description should be living up here.

Hmm, that was my interpretation of #15 in Documentation/SubmittingPatches... ;-)  Maybe it should read

<snip>
Improve the recovery of the MPC5200B's I2C bus from errors like bus hangs.  This includes making the bus timeout configurable, a better detection of cases where the bus has to be "fixed" after a timeout, and a more thorough fixup sequence.
</snip>

> Is thios a candidate for an -rc or should it be left to merge window?

Well, basically it was a rfc.  I apparently need it on my 5200B board, but I hoped to get some more insight from the Freescale/I2C gurus (see "open questions" in the post).  Thus merge window, IMHO...

Thanks, Albrecht.

> 
> > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de>
> >
> > ---
> >
> > This patch introduces several improvements to the MPC5200B's I2C driver
> > as to improve the recovery from error conditions I encountered when
> > testing a custom board with several I2C devices attached (eeprom, io
> > expander, rtc, sensors).  The error conditions included cases where the
> > bus if logic of one slave apparently went south, blocking the bus
> > completely.
> >
> > My fixes include:
> > 1. make the bus timeout configurable in fsl_i2c_probe(); the default of
> >    one second is *way* too long for my use case;
> > 2. if a timeout condition occurs in mpc_xfer(), mpc_i2c_fixup() the bus
> >    if *any* of the CF, BB and RXAK flags in the MSR is 1.  I actually
> >    saw different combinations with hangs, not only all three set;
> > 3. improve the fixup procedure by calculating the timing needed from the
> >    real (configured) bus clock, calculated in mpc_i2c_setclock_52xx().
> >    Furthermore, I issue 9 instead of one cycle, as I experienced cases
> >    where the single one is not enough (found this tip in a forum).  As a
> >    side effect, the new scheme needs only 81us @375kHz bus clock instead
> >    of 150us.  I recorded waveforms for 18.4kHz, 85.9kHz and 375kHz, all
> >    looking fine, which I can provide if anyone is interested.
> >
> > Open questions:
> > - is the approach correct at all, in particular the interpretation of
> >   the flags (#2)?
> > - could this code also be used on non-5200 processors?
> >
> > --- linux-2.6.32-orig/drivers/i2c/busses/i2c-mpc.c	2009-12-03 04:51:21.000000000 +0100
> > +++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c	2010-01-22 16:05:13.000000000 +0100
[snip]

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

^ permalink raw reply

* Re: [PATCH/RFC 2/2] 5200: improve i2c bus error recovery
From: Albrecht Dreß @ 2010-01-25 20:26 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Linux PPC Development, Devicetree Discussions,
	Ben Dooks (embedded platforms)
In-Reply-To: <4B5D4E56.3020902@grandegger.com>

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

Am 25.01.10 08:55 schrieb(en) Wolfgang Grandegger:
> Albrecht Dreß wrote:
>> Make the I2C adapter timeout configurable through a Device Tree property which gives the timeout in microseconds.
> 
> My understanding is that software properties should not be defined via the device tree. I think a sysfs entry is more appropriate.

Is the timeout really a /software/ property?  My feeling is that the timeout basically depends upon the bus clock and the chips attached to the bus.  Therefore, it is linked closer to the composition of the board's hardware as described in the device tree, than to any userland software.  Or am I totally wrong here?

Thanks, Albrecht.

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

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Jesse Barnes @ 2010-01-25 20:38 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Ron Mercer, Linux PCI, Jay Vosburgh, linuxppc-dev,
	kaneshige.kenji
In-Reply-To: <4B5D9FC5.5070600@linux.vnet.ibm.com>

On Mon, 25 Jan 2010 11:42:29 -0200
Breno Leitao <leitao@linux.vnet.ibm.com> wrote:

> Hello, 
> 
> I found that qlge is broken on PPC, and it got broken after commit 
> 06a1cbafb253c4c60d6a54a994887f5fbceabcc0. It happens because
> dev->pcie is not set on PPC, because the function
> set_pcie_port_type(), who sets dev->pcie, is not being called on PPC
> PCI code.

You mean dev->is_pcie?

Why isn't pci_scan_device calling pci_setup_device for you?  That
should do the proper PCIe init depending on the device, along with
extracting other device info...

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply

* Re: [PATCH 2/3] i2c-mpc: add support for the MPC512x processors from Freescale
From: Wolfram Sang @ 2010-01-25 20:48 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Devicetree-discuss, Linuxppc-dev, Linux-i2c, Wolfgang Grandegger
In-Reply-To: <4B5DE3FB.10505@grandegger.com>

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

> overkill to me. I will have a closer look, though. At a minimum I will
> replace "-1" with "MPC_I2C_PRESERVE_CLOCK".

Might be also an idea to define it with ~0 (clock is still unsigned). If
possible, the code checking for those two cases (0 and "-1") should be close
together. That could be a compromise until more quirks are needed ;)

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply

* Re: [PATCH 2/3] i2c-mpc: add support for the MPC512x processors from Freescale
From: Wolfgang Grandegger @ 2010-01-25 20:54 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Devicetree-discuss, Linuxppc-dev, Linux-i2c, Wolfgang Grandegger
In-Reply-To: <20100125204833.GB9693@pengutronix.de>

Wolfram Sang wrote:
>> overkill to me. I will have a closer look, though. At a minimum I will
>> replace "-1" with "MPC_I2C_PRESERVE_CLOCK".
> 
> Might be also an idea to define it with ~0 (clock is still unsigned). If
> possible, the code checking for those two cases (0 and "-1") should be close
> together. That could be a compromise until more quirks are needed ;)

I just sent v2. Hope it's OK now.

Wolfgang.

^ permalink raw reply

* [PATCH v2 0/3] i2c-mpc: add support for the Freescale MPC512x and other fixes
From: Wolfgang Grandegger @ 2010-01-25 20:55 UTC (permalink / raw)
  To: Linux-i2c; +Cc: Devicetree-discuss, Linuxppc-dev, Wolfgang Grandegger

From: Wolfgang Grandegger <wg@denx.de>

This patch series adds support for the MPC512x from Freescale to the
i2c-mpc driver. At that occasion, issues with  __devinit[data] have
been fixed and the doc of the FSL I2C dts bindings updated. It has
been tested on a MPC5121ADS, TQM5200 and TQM8560 board

Changes since v1:

- use macro MPC_I2C_CLOCK_PRESERVE/SAFE for the special clock settings.
- document the special DTS node "fsl,mpc5121-i2c-ctrl".
- update and correct the Kconfig help.
- some other minor fixes as suggested by Wolfram.

Wolfgang

Wolfgang Grandegger (3):
  i2c-mpc: use __devinit[data] for initialization functions and data
  i2c-mpc: add support for the MPC512x processors from Freescale
  powerpc: doc/dts-bindings: update doc of FSL I2C bindings

 Documentation/powerpc/dts-bindings/fsl/i2c.txt |   30 +++-
 drivers/i2c/busses/Kconfig                     |    7 +-
 drivers/i2c/busses/i2c-mpc.c                   |  190 +++++++++++++++---------
 3 files changed, 147 insertions(+), 80 deletions(-)

^ permalink raw reply

* [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data
From: Wolfgang Grandegger @ 2010-01-25 20:55 UTC (permalink / raw)
  To: Linux-i2c; +Cc: Devicetree-discuss, Linuxppc-dev, Wolfgang Grandegger
In-Reply-To: <1264452906-32536-1-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

"__devinit[data]" has not yet been used for all initialization functions
and data. To avoid truncating lines, the struct mpc_i2c_match_data has
been renamed to mpc_i2c_data, which is even the better name.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 drivers/i2c/busses/i2c-mpc.c |   99 +++++++++++++++++++----------------------
 1 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index f627001..2cb864e 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -66,7 +66,7 @@ struct mpc_i2c_divider {
 	u16 fdr;	/* including dfsrr */
 };
 
-struct mpc_i2c_match_data {
+struct mpc_i2c_data {
 	void (*setclock)(struct device_node *node,
 			 struct mpc_i2c *i2c,
 			 u32 clock, u32 prescaler);
@@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 }
 
 #ifdef CONFIG_PPC_MPC52xx
-static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
+static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
 	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
 	{36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
@@ -186,7 +186,8 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
 };
 
-int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
+static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
+					  int prescaler)
 {
 	const struct mpc_i2c_divider *div = NULL;
 	unsigned int pvr = mfspr(SPRN_PVR);
@@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void mpc_i2c_setclock_52xx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
@@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node *node,
 		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
 }
 #else /* !CONFIG_PPC_MPC52xx */
-static void mpc_i2c_setclock_52xx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 }
 #endif /* CONFIG_PPC_MPC52xx*/
 
 #ifdef CONFIG_FSL_SOC
-static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
+static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
 	{160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
 	{288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
 	{416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
@@ -258,7 +259,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
 	{49152, 0x011e}, {61440, 0x011f}
 };
 
-u32 mpc_i2c_get_sec_cfg_8xxx(void)
+static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
 {
 	struct device_node *node = NULL;
 	u32 __iomem *reg;
@@ -287,7 +288,8 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void)
 	return val;
 }
 
-int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
+static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
+					  u32 prescaler)
 {
 	const struct mpc_i2c_divider *div = NULL;
 	u32 divider;
@@ -320,9 +322,9 @@ int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void mpc_i2c_setclock_8xxx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
@@ -338,9 +340,9 @@ static void mpc_i2c_setclock_8xxx(struct device_node *node,
 }
 
 #else /* !CONFIG_FSL_SOC */
-static void mpc_i2c_setclock_8xxx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 }
 #endif /* CONFIG_FSL_SOC */
@@ -529,8 +531,8 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
 			clock = *prop;
 
 		if (match->data) {
-			struct mpc_i2c_match_data *data =
-				(struct mpc_i2c_match_data *)match->data;
+			struct mpc_i2c_data *data =
+				(struct mpc_i2c_data *)match->data;
 			data->setclock(op->node, i2c, clock, data->prescaler);
 		} else {
 			/* Backwards compatibility */
@@ -582,44 +584,35 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
 	return 0;
 };
 
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
+	.setclock = mpc_i2c_setclock_52xx,
+};
+
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
+	.setclock = mpc_i2c_setclock_8xxx,
+};
+
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
+	.setclock = mpc_i2c_setclock_8xxx,
+	.prescaler = 2,
+};
+
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
+	.setclock = mpc_i2c_setclock_8xxx,
+	.prescaler = 3,
+};
+
 static const struct of_device_id mpc_i2c_of_match[] = {
-	{.compatible = "mpc5200-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_52xx,
-		},
-	},
-	{.compatible = "fsl,mpc5200b-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_52xx,
-		},
-	},
-	{.compatible = "fsl,mpc5200-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_52xx,
-		},
-	},
-	{.compatible = "fsl,mpc8313-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_8xxx,
-		},
-	},
-	{.compatible = "fsl,mpc8543-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_8xxx,
-			.prescaler = 2,
-		},
-	},
-	{.compatible = "fsl,mpc8544-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_8xxx,
-			.prescaler = 3,
-		},
+	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
+	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
+	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
 	/* Backward compatibility */
-	},
 	{.compatible = "fsl-i2c", },
 	{},
 };
-
 MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
 
 
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH v2 2/3] i2c-mpc: add support for the MPC512x processors from Freescale
From: Wolfgang Grandegger @ 2010-01-25 20:55 UTC (permalink / raw)
  To: Linux-i2c; +Cc: Devicetree-discuss, Linuxppc-dev, Wolfgang Grandegger
In-Reply-To: <1264452906-32536-2-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

The "setclock" initialization functions have been renamed to "setup"
because I2C interrupts must be enabled for the MPC512x. This requires
to handle "fsl,preserve-clocking" in a slighly different way. Also,
the old settings are now reported calling dev_dbg(). For the MPC512x
the clock setup function of the MPC52xx can be re-used. Furthermore,
the Kconfig help has been updated and corrected.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 drivers/i2c/busses/Kconfig   |    7 +-
 drivers/i2c/busses/i2c-mpc.c |  127 ++++++++++++++++++++++++++++++------------
 2 files changed, 95 insertions(+), 39 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 5f318ce..5477e41 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -418,13 +418,12 @@ config I2C_IXP2000
 	  instead.
 
 config I2C_MPC
-	tristate "MPC107/824x/85xx/52xx/86xx"
+	tristate "MPC107/824x/85xx/512x/52xx/83xx/86xx"
 	depends on PPC32
 	help
 	  If you say yes to this option, support will be included for the
-	  built-in I2C interface on the MPC107/Tsi107/MPC8240/MPC8245 and
-	  MPC85xx/MPC8641 family processors. The driver may also work on 52xx
-	  family processors, though interrupts are known not to work.
+	  built-in I2C interface on the MPC107, Tsi107, MPC512x, MPC52xx,
+	  MPC8240, MPC8245, MPC83xx, MPC85xx and MPC8641 family processors.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mpc.
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 2cb864e..bc0281d 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -31,6 +31,9 @@
 
 #define DRV_NAME "mpc-i2c"
 
+#define MPC_I2C_CLOCK_SAFE     0
+#define MPC_I2C_CLOCK_PRESERVE (~0U)
+
 #define MPC_I2C_FDR   0x04
 #define MPC_I2C_CR    0x08
 #define MPC_I2C_SR    0x0c
@@ -67,9 +70,8 @@ struct mpc_i2c_divider {
 };
 
 struct mpc_i2c_data {
-	void (*setclock)(struct device_node *node,
-			 struct mpc_i2c *i2c,
-			 u32 clock, u32 prescaler);
+	void (*setup)(struct device_node *node, struct mpc_i2c *i2c,
+		      u32 clock, u32 prescaler);
 	u32 prescaler;
 };
 
@@ -164,7 +166,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 	return 0;
 }
 
-#ifdef CONFIG_PPC_MPC52xx
+#if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x)
 static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
 	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
@@ -216,12 +218,18 @@ static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
+	if (clock == MPC_I2C_CLOCK_PRESERVE) {
+		dev_dbg(i2c->dev, "using fdr %d\n",
+			readb(i2c->base + MPC_I2C_FDR));
+		return;
+	}
+
 	ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler);
 	fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */
 
@@ -230,13 +238,50 @@ static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
 	if (ret >= 0)
 		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
 }
-#else /* !CONFIG_PPC_MPC52xx */
-static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+#else /* !(CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x) */
+static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 }
-#endif /* CONFIG_PPC_MPC52xx*/
+#endif /* CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x */
+
+#ifdef CONFIG_PPC_MPC512x
+static void __devinit mpc_i2c_setup_512x(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
+{
+	struct device_node *node_ctrl;
+	void __iomem *ctrl;
+	const u32 *pval;
+	u32 idx;
+
+	/* Enable I2C interrupts for mpc5121 */
+	node_ctrl = of_find_compatible_node(NULL, NULL,
+					    "fsl,mpc5121-i2c-ctrl");
+	if (node_ctrl) {
+		ctrl = of_iomap(node_ctrl, 0);
+		if (ctrl) {
+
+			/* Interrupt enable bits for i2c-0/1/2: bit 24/26/28 */
+			pval = of_get_property(node, "reg", NULL);
+			idx = (*pval & 0xff) / 0x20;
+			setbits32(ctrl, 1 << (24 + idx * 2));
+			iounmap(ctrl);
+		}
+		of_node_put(node_ctrl);
+	}
+
+	/* The clock setup for the 52xx works also fine for the 512x */
+	mpc_i2c_setup_52xx(node, i2c, clock, prescaler);
+}
+#else /* CONFIG_PPC_MPC512x */
+static void __devinit mpc_i2c_setup_512x(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
+{
+}
+#endif /* CONFIG_PPC_MPC512x */
 
 #ifdef CONFIG_FSL_SOC
 static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
@@ -322,12 +367,19 @@ static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setup_8xxx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
+	if (clock == MPC_I2C_CLOCK_PRESERVE) {
+		dev_dbg(i2c->dev, "using dfsrr %d, fdr %d\n",
+			readb(i2c->base + MPC_I2C_DFSRR),
+			readb(i2c->base + MPC_I2C_FDR));
+		return;
+	}
+
 	ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler);
 	fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */
 
@@ -340,9 +392,9 @@ static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
 }
 
 #else /* !CONFIG_FSL_SOC */
-static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setup_8xxx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 }
 #endif /* CONFIG_FSL_SOC */
@@ -496,7 +548,7 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
 {
 	struct mpc_i2c *i2c;
 	const u32 *prop;
-	u32 clock = 0;
+	u32 clock = MPC_I2C_CLOCK_SAFE;
 	int result = 0;
 	int plen;
 
@@ -525,21 +577,21 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
 		}
 	}
 
-	if (!of_get_property(op->node, "fsl,preserve-clocking", NULL)) {
+	if (of_get_property(op->node, "fsl,preserve-clocking", NULL)) {
+		clock = MPC_I2C_PRESERVE_CLOCK;
+	} else {
 		prop = of_get_property(op->node, "clock-frequency", &plen);
 		if (prop && plen == sizeof(u32))
 			clock = *prop;
+	}
 
-		if (match->data) {
-			struct mpc_i2c_data *data =
-				(struct mpc_i2c_data *)match->data;
-			data->setclock(op->node, i2c, clock, data->prescaler);
-		} else {
-			/* Backwards compatibility */
-			if (of_get_property(op->node, "dfsrr", NULL))
-				mpc_i2c_setclock_8xxx(op->node, i2c,
-						      clock, 0);
-		}
+	if (match->data) {
+		struct mpc_i2c_data *data = match->data;
+		data->setup(op->node, i2c, clock, data->prescaler);
+	} else {
+		/* Backwards compatibility */
+		if (of_get_property(op->node, "dfsrr", NULL))
+			mpc_i2c_setup_8xxx(op->node, i2c, clock, 0);
 	}
 
 	dev_set_drvdata(&op->dev, i2c);
@@ -584,21 +636,25 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
 	return 0;
 };
 
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_512x = {
+	.setup = mpc_i2c_setup_512x,
+};
+
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
-	.setclock = mpc_i2c_setclock_52xx,
+	.setup = mpc_i2c_setup_52xx,
 };
 
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
-	.setclock = mpc_i2c_setclock_8xxx,
+	.setup = mpc_i2c_setup_8xxx,
 };
 
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
-	.setclock = mpc_i2c_setclock_8xxx,
+	.setup = mpc_i2c_setup_8xxx,
 	.prescaler = 2,
 };
 
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
-	.setclock = mpc_i2c_setclock_8xxx,
+	.setup = mpc_i2c_setup_8xxx,
 	.prescaler = 3,
 };
 
@@ -606,6 +662,7 @@ static const struct of_device_id mpc_i2c_of_match[] = {
 	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
 	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
 	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc5121-i2c", .data = &mpc_i2c_data_512x, },
 	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
 	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
 	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
@@ -648,5 +705,5 @@ module_exit(fsl_i2c_exit);
 
 MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
 MODULE_DESCRIPTION("I2C-Bus adapter for MPC107 bridge and "
-		   "MPC824x/85xx/52xx processors");
+		   "MPC824x/85xx/512x/52xx processors");
 MODULE_LICENSE("GPL");
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH v2 3/3] powerpc: doc/dts-bindings: update doc of FSL I2C bindings
From: Wolfgang Grandegger @ 2010-01-25 20:55 UTC (permalink / raw)
  To: Linux-i2c; +Cc: Devicetree-discuss, Linuxppc-dev, Wolfgang Grandegger
In-Reply-To: <1264452906-32536-3-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

This patch adds the MPC5121 to the list of supported devices,
enhances the doc of the "clock-frequency" property and removes
the obsolete "cell-index" property from the example nodes.
Furthermore and example for the MPC5121 has been added.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 Documentation/powerpc/dts-bindings/fsl/i2c.txt |   30 +++++++++++++++++++----
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt
index b6d2e21..2f62dae 100644
--- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt
@@ -9,8 +9,9 @@ Recommended properties :
 
  - compatible : compatibility list with 2 entries, the first should
    be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor,
-   e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one
-   should be "fsl-i2c".
+   e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The
+   second one should be "fsl-i2c". For the mpc5121, an additional node
+   "fsl,mpc5121-i2c-ctrl" is required as shown in the example below.
  - interrupts : <a b> where a is the interrupt number and b is a
    field that represents an encoding of the sense and level
    information for the interrupt.  This should be encoded based on
@@ -20,29 +21,46 @@ Recommended properties :
    services interrupts for this device.
  - fsl,preserve-clocking : boolean; if defined, the clock settings
    from the bootloader are preserved (not touched).
- - clock-frequency : desired I2C bus clock frequency in Hz.
+ - clock-frequency : desired I2C bus clock frequency in Hz.  If this
+   property and "fsl,preserve-clocking" is not defined, a safe fixed
+   clock divider value is used (resulting in a small clock frequency).
 
 Examples :
 
+	/* MPC5121 based board */
+	i2c@1740 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "fsl,mpc5121-i2c", "fsl-i2c";
+		reg = <0x1740 0x20>;
+		interrupts = <11 0x8>;
+		interrupt-parent = <&ipic>;
+		clock-frequency = <100000>;
+	};
+
+	i2ccontrol@1760 {
+		compatible = "fsl,mpc5121-i2c-ctrl";
+		reg = <0x1760 0x8>;
+	};
+
+	/* MPC5200B based board */
 	i2c@3d00 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
-		cell-index = <0>;
 		reg = <0x3d00 0x40>;
 		interrupts = <2 15 0>;
 		interrupt-parent = <&mpc5200_pic>;
 		fsl,preserve-clocking;
 	};
 
+	/* MPC8544 base board */
 	i2c@3100 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		cell-index = <1>;
 		compatible = "fsl,mpc8544-i2c", "fsl-i2c";
 		reg = <0x3100 0x100>;
 		interrupts = <43 2>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <400000>;
 	};
-
-- 
1.6.2.5

^ permalink raw reply related

* Re: [PATCH 2/3] i2c-mpc: add support for the MPC512x processors from Freescale
From: Wolfram Sang @ 2010-01-25 21:09 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Devicetree-discuss, Linuxppc-dev, Linux-i2c, Wolfgang Grandegger
In-Reply-To: <4B5E0523.9080701@grandegger.com>

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

> I just sent v2. Hope it's OK now.

Thanks, will check tomorrow.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Jesse Barnes @ 2010-01-26  1:50 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Linux PCI, Jay Vosburgh, Ron Mercer, kaneshige.kenji,
	linuxppc-dev, Breno Leitao
In-Reply-To: <20100125123849.111fa2d1@jbarnes-piketon>

On Mon, 25 Jan 2010 12:38:49 -0800
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Mon, 25 Jan 2010 11:42:29 -0200
> Breno Leitao <leitao@linux.vnet.ibm.com> wrote:
> 
> > Hello, 
> > 
> > I found that qlge is broken on PPC, and it got broken after commit 
> > 06a1cbafb253c4c60d6a54a994887f5fbceabcc0. It happens because
> > dev->pcie is not set on PPC, because the function
> > set_pcie_port_type(), who sets dev->pcie, is not being called on PPC
> > PCI code.
> 
> You mean dev->is_pcie?
> 
> Why isn't pci_scan_device calling pci_setup_device for you?  That
> should do the proper PCIe init depending on the device, along with
> extracting other device info...

Cc'ing Ben for PPC.  Ben, should PPC use pci_scan_device when probing
its root busses?  Sounds like it just uses pci_device_add for each one
it finds instead?

If you don't actually need scanning (though what about hotplug?) we can
move the call to device_add instead...

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Benjamin Herrenschmidt @ 2010-01-26  2:59 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Linux PCI, Jay Vosburgh, Ron Mercer, kaneshige.kenji,
	linuxppc-dev, Breno Leitao
In-Reply-To: <20100125175025.4c74f412@jbarnes-piketon>

On Mon, 2010-01-25 at 17:50 -0800, Jesse Barnes wrote:
> > You mean dev->is_pcie?
> > 
> > Why isn't pci_scan_device calling pci_setup_device for you?  That
> > should do the proper PCIe init depending on the device, along with
> > extracting other device info...
> 
> Cc'ing Ben for PPC.  Ben, should PPC use pci_scan_device when probing
> its root busses?  Sounds like it just uses pci_device_add for each one
> it finds instead?
> 
> If you don't actually need scanning (though what about hotplug?) we can
> move the call to device_add instead...

I need to give it more brain cells than I have available today :-) I'll
have a look tomorrow. In some case we build the PCI stuff up from the
firmware device-tree without actually doing any config space scanning,
so the root busses are treated a bit special.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Benjamin Herrenschmidt @ 2010-01-26  4:23 UTC (permalink / raw)
  To: Joel Schopp
  Cc: Michael Neuling, Peter Zijlstra, ego, linux-kernel, Ingo Molnar,
	linuxppc-dev
In-Reply-To: <4B5DD9F1.2010002@austin.ibm.com>

On Mon, 2010-01-25 at 11:50 -0600, Joel Schopp wrote:
> > Look at the cputhreads.h implementation ... Today we only support
> > power-of-two numbers of threads.
> >   
> I've run 3 threads using cpu hotplug to offline 1 of the 4.  It's 
> certainly a stupid idea, but there you go. 

Oh, you mean you need to use the actual online count ? In which case,
yes, you do indeed need to be a bit more careful... In this case though,
you're probably better off special casing "3" :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Kenji Kaneshige @ 2010-01-26  4:18 UTC (permalink / raw)
  To: Breno Leitao; +Cc: Ron Mercer, Linux PCI, Jay Vosburgh, linuxppc-dev
In-Reply-To: <4B5D9FC5.5070600@linux.vnet.ibm.com>

Breno Leitao wrote:
> Hello, 
> 
> I found that qlge is broken on PPC, and it got broken after commit 
> 06a1cbafb253c4c60d6a54a994887f5fbceabcc0. It happens because dev->pcie 
> is not set on PPC, because the function set_pcie_port_type(), who sets
> dev->pcie, is not being called on PPC PCI code.

Hi Breno,

I'm sorry for the regression. I didn't realize that PPC uses open-coded
PCI scan.

I guess the problem is caused by dev->pcie_cap, right? But I don't
understand what is happening clearly. Could you send me the detailed
information about the problem?

> 
> So, I have two ideas to fix it, the first one is to call 
> set_pcie_port_type() on pci_device_add() instead of pci_setup_device(). 
> Since that PPC device flow calls pci_device_add(), it fixes the problem
> without duplicating the caller for this function. 
> 

The set_pcie_port_type() needs to be called before pci_fixup_device() in
pci_setup_device() because fixup code might refer dev->pcie_cap. So I
don't think the first one is a good idea.

> OTOH, it's also possible to add set_pcie_port_type() on pci.h and call
> it inside the PPC PCI files, specifically on of_create_pci_dev().
> 
> I tested both ideas and they work perfect, so I'd like to figure out 
> which one is the most correct one. Both patches are attached here. 

I think the second one is better. But to prevent similar problems, I
think it would be better to remove open-coded PCI scan in PPC in a
long term, though I don't know the background about that.

Thanks,
Kenji Kaneshige


> 
> Thanks, 
> Breno
> 
> ----- First idea -----
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 98ffb2d..328c3ab 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -746,7 +746,6 @@ int pci_setup_device(struct pci_dev *dev)
>  	dev->hdr_type = hdr_type & 0x7f;
>  	dev->multifunction = !!(hdr_type & 0x80);
>  	dev->error_state = pci_channel_io_normal;
> -	set_pcie_port_type(dev);
>  	set_pci_aer_firmware_first(dev);
>  
>  	list_for_each_entry(slot, &dev->bus->slots, list)
> @@ -1052,6 +1051,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	/* Initialize various capabilities */
>  	pci_init_capabilities(dev);
>  
> +	set_pcie_port_type(dev);
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> 
> ----- Second idea -----
> 
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index 7311fdf..f8820e8 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -160,6 +160,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,
>  	dev->error_state = pci_channel_io_normal;
>  	dev->dma_mask = 0xffffffff;
>  
> +	set_pcie_port_type(dev);
>  	if (!strcmp(type, "pci") || !strcmp(type, "pciex")) {
>  		/* a PCI-PCI bridge */
>  		dev->hdr_type = PCI_HEADER_TYPE_BRIDGE;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 98ffb2d..f787eea 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -681,7 +681,7 @@ static void pci_read_irq(struct pci_dev *dev)
>  	dev->irq = irq;
>  }
>  
> -static void set_pcie_port_type(struct pci_dev *pdev)
> +void set_pcie_port_type(struct pci_dev *pdev)
>  {
>  	int pos;
>  	u16 reg16;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 174e539..765095b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1269,6 +1269,7 @@ int pcibios_add_platform_entries(struct pci_dev *dev);
>  void pcibios_disable_device(struct pci_dev *dev);
>  int pcibios_set_pcie_reset_state(struct pci_dev *dev,
>  				 enum pcie_reset_state state);
> +extern void set_pcie_port_type(struct pci_dev *pdev);
>  
>  #ifdef CONFIG_PCI_MMCONFIG
>  extern void __init pci_mmcfg_early_init(void);
> 
> 

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Kenji Kaneshige @ 2010-01-26  4:36 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Linux PCI, Jay Vosburgh, Ron Mercer, linuxppc-dev, Breno Leitao
In-Reply-To: <20100125175025.4c74f412@jbarnes-piketon>

Jesse Barnes wrote:
> On Mon, 25 Jan 2010 12:38:49 -0800
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
>> On Mon, 25 Jan 2010 11:42:29 -0200
>> Breno Leitao <leitao@linux.vnet.ibm.com> wrote:
>>
>>> Hello, 
>>>
>>> I found that qlge is broken on PPC, and it got broken after commit 
>>> 06a1cbafb253c4c60d6a54a994887f5fbceabcc0. It happens because
>>> dev->pcie is not set on PPC, because the function
>>> set_pcie_port_type(), who sets dev->pcie, is not being called on PPC
>>> PCI code.
>> You mean dev->is_pcie?
>>
>> Why isn't pci_scan_device calling pci_setup_device for you?  That
>> should do the proper PCIe init depending on the device, along with
>> extracting other device info...
> 
> Cc'ing Ben for PPC.  Ben, should PPC use pci_scan_device when probing
> its root busses?  Sounds like it just uses pci_device_add for each one
> it finds instead?
> 
> If you don't actually need scanning (though what about hotplug?) we can
> move the call to device_add instead...
> 

As I mentioned in the other e-mail, I think the set_pcie_port_type() needs
to be called before pci_fixup_device() call in the pci_setup_device()
because some fixup handler might refer the dev->is_pcie, dev->pcie_cap,
or dev->pcie_type.

Thanks,
Kenji Kaneshige

^ permalink raw reply

* Re: [PATCH 2/4] of/gpio: Add support for two-stage registration for the of_gpio_chips
From: David Brownell @ 2010-01-26  6:36 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: David Brownell, Dmitry Eremin-Solenikov, linux-kernel,
	linuxppc-dev, Bill Gatliff, Andrew Morton
In-Reply-To: <20100125181103.GB13805@oksana.dev.rtsoft.ru>

On Monday 25 January 2010, Anton Vorontsov wrote:
> With this patch there are two ways to register OF GPIO controllers:
> 
> 1. Allocating the of_gpio_chip structure and passing the
>    &of_gc->gc pointer to the gpiochip_add. (Can use container_of
>    to convert the gpio_chip to the of_gpio_chip.)
> 
> 2. Allocating and registering the gpio_chip structure separately
>    from the of_gpio_chip. (Since two allocations are separate,
>    container_of won't work.)
> 
> As time goes by we'll kill the first option.

Why have two options, instead of just the first/simpler one??

^ permalink raw reply

* Re: [PATCH 1/4] gpiolib: Introduce chip addition/removal notifier
From: David Brownell @ 2010-01-26  6:34 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: David Brownell, Dmitry Eremin-Solenikov, linux-kernel,
	linuxppc-dev, Bill Gatliff, Andrew Morton
In-Reply-To: <20100125181100.GA13805@oksana.dev.rtsoft.ru>

On Monday 25 January 2010, Anton Vorontsov wrote:
> 
> +config GPIOLIB_NOTIFIER
> +       bool
> +       help
> +         This symbol is selected by subsystems that need to handle GPIO
> +         chips addition and removal. E.g., this is used for the
> +         OpenFirmware bindings.
> +

I'm no huge fan of notifiers, but I suppose they have their place.

However ... I don't see a lot of win to making this optional.  Just
inline the little two blocking_notifier_call_chain() calls directly,
making this a *LOT* simpler.

- Dave

^ permalink raw reply

* Re: [PATCH 4/4] powerpc/mcu_mpc8349emitx: Remove OF GPIO handling stuff
From: David Brownell @ 2010-01-26  6:43 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: David Brownell, Dmitry Eremin-Solenikov, linux-kernel,
	linuxppc-dev, Bill Gatliff, Andrew Morton
In-Reply-To: <20100125181114.GD13805@oksana.dev.rtsoft.ru>

On Monday 25 January 2010, Anton Vorontsov wrote:
> With the new OF GPIO infrastructure it's much easier to handle I2C
> GPIO controllers, i.e. now drivers don't have to deal with the
> OF-specific bits.

Good, that's how it should have been done in the first place.  :)

Of course, there's still the question of how to get driver-specific
configuration data into the driver...

^ permalink raw reply

* Re: [PATCH/RFC 2/2] 5200: improve i2c bus error recovery
From: Wolfgang Grandegger @ 2010-01-26  7:33 UTC (permalink / raw)
  To: Albrecht Dreß
  Cc: Linux PPC Development, Devicetree Discussions,
	Ben Dooks (embedded platforms)
In-Reply-To: <1264451219.2315.1@antares>

Albrecht Dreß wrote:
> Am 25.01.10 08:55 schrieb(en) Wolfgang Grandegger:
>> Albrecht Dreß wrote:
>>> Make the I2C adapter timeout configurable through a Device Tree
>>> property which gives the timeout in microseconds.
>>
>> My understanding is that software properties should not be defined via
>> the device tree. I think a sysfs entry is more appropriate.
> 
> Is the timeout really a /software/ property?  My feeling is that the
> timeout basically depends upon the bus clock and the chips attached to
> the bus.  Therefore, it is linked closer to the composition of the
> board's hardware as described in the device tree, than to any userland
> software.  Or am I totally wrong here?

Well, yes, it seems to be a border case.

Wolfgang.

^ permalink raw reply

* Re: [PATCH 03/11] powerpc/mpc5121: Add machine restart support
From: Anatolij Gustschin @ 2010-01-26  7:53 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, wd, dzu, Piotr Ziecik
In-Reply-To: <20100120112843.GE5041@pengutronix.de>

On Wed, 20 Jan 2010 12:28:43 +0100
Wolfram Sang <w.sang@pengutronix.de> wrote:

> > +static void __iomem *reset_module_base;
> 
> type here also?

Ok.

> > +
> > +static int __init mpc512x_restart_init(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-reset");
> > +	if (!np)
> > +		return -1;
> > +
> > +	reset_module_base = of_iomap(np, 0);
> > +	of_node_put(np);
> > +
> > +	return 0;
> > +}
> > +
> 
> Drop this empty line?

Ok.

> > +	} else {
> > +		printk(KERN_ERR ": Restart module not mapped.\n");
> 
> The colon is a leftover?

Yes, will fix for using pr_error() and resubmit.

Thanks,
Anatolij

^ permalink raw reply

* Re: [PATCH 07/11] dma: Add MPC512x DMA driver
From: Anatolij Gustschin @ 2010-01-26  8:03 UTC (permalink / raw)
  To: Grant Likely; +Cc: wd, dzu, linuxppc-dev, Dan Williams, Piotr Ziecik
In-Reply-To: <fa686aa41001210922h280be005pda88e5859b2769f1@mail.gmail.com>

On Thu, 21 Jan 2010 10:22:27 -0700
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Tue, Jan 19, 2010 at 1:24 PM, Anatolij Gustschin <agust@denx.de> wrote:
> > From: Piotr Ziecik <kosmo@semihalf.com>
> >
> > Adds initial version of MPC512x DMA driver.
> > Only memory to memory transfers are currenly supported.
>=20
> Comments below on brief review.  I've not looked at the code in-depth.

> > ...
> > =C2=A0drivers/dma/Kconfig =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A07 +
> > =C2=A0drivers/dma/Makefile =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A01 +
> > =C2=A0drivers/dma/mpc512x_dma.c | =C2=A0636 +++++++++++++++++++++++++++=
++++++++++++++++++
> > =C2=A0drivers/dma/mpc512x_dma.h | =C2=A0192 ++++++++++++++
>=20
> Unless the stuff in the .h file is used by other .c files, it really
> belongs in mpc512x_dma.c

Ok, will be moved to .c file in next version.

>=20
> > +static int __init mpc_dma_probe(struct of_device *op,
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 const struc=
t of_device_id *match)
>=20
> __devinit
>=20
> > +static void __exit mpc_dma_remove(struct of_device *op)
>=20
> __devexit

Ok.

> > +{
> > + =C2=A0 =C2=A0 =C2=A0 struct device *dev =3D &op->dev;
> > + =C2=A0 =C2=A0 =C2=A0 struct mpc_dma *mdma =3D dev_get_drvdata(dev);
> > +
> > + =C2=A0 =C2=A0 =C2=A0 devm_free_irq(dev, mdma->irq, mdma);
> > +}
>=20
> No unregistration of the dma device?  No unmapping?  No kfree()?

I will add unregistration of the dma device. Unmapping and
freeing should be done automatically on driver detach, mapping
and allocation is done by devm_ioremap() and devm_kzalloc().

> > +static struct of_platform_driver mpc_dma_driver =3D {
> > + =C2=A0 =C2=A0 =C2=A0 .match_table =C2=A0 =C2=A0=3D mpc_dma_match,
> > + =C2=A0 =C2=A0 =C2=A0 .probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc=
_dma_probe,
> > + =C2=A0 =C2=A0 =C2=A0 .remove =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D __exit_p=
(mpc_dma_remove),
>=20
> __devexit_p()

Ok.

> > +module_exit(mpc_dma_exit);
> > +
> > +/* MODULE API */
>=20
> Meaningless comment.

Ok, will remove it.

Thanks,
Anatolij

^ permalink raw reply

* Re: [PATCH 0/11] Update support for MPC512x
From: Anatolij Gustschin @ 2010-01-26  8:06 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: wd, dzu, netdev, linux-usb, linuxppc-dev, linux-mtd, linux-i2c,
	rtc-linux, Dan Williams
In-Reply-To: <20100120112232.GD5041@pengutronix.de>

On Wed, 20 Jan 2010 12:22:32 +0100
Wolfram Sang <w.sang@pengutronix.de> wrote:

> 
> > The patches are based on v2.6.33-rc4 and cover the following
> > items:
> 
> The USB-patch needs two patches from linux-next. For convenience, don't you
> have a git-tree somewhere? :)

No, I don't have a git tree for this now, sorry.

Anatolij

^ permalink raw reply

* Re: [PATCH 0/11] Update support for MPC512x
From: Wolfram Sang @ 2010-01-26 12:16 UTC (permalink / raw)
  To: Anatolij Gustschin
  Cc: rtc-linux, dzu, netdev, linux-usb, linuxppc-dev, linux-mtd,
	linux-i2c, Dan Williams
In-Reply-To: <20100126090633.74257b1a@wker>

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

> No, I don't have a git tree for this now, sorry.

No problem, I set one up at:

	git://git.pengutronix.de/git/wsa/linux-2.6.git

It is based on 2.6.33-rc5 and contains the newest patches for FEC and I2C. (I
will pick up the patches as they show up on the lists, so this tree will be
rebased!)

This tree includes all MPC5121-patches and the two USB-patches from Anton. I
haven't actually tested them yet, but they all applied cleanly, at least.
Testing will come soon...

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply


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