Subject: Re: PPCboot and latest kernel
vijay sharma wrote: > Linux/PowerPC load: > Finalizing device tree... flat tree at 0x2f65300 <== Beyond this point no > output is available. I have found that on my MPC8272 system running 2.6.28 & cuImage, the /chosen/linux,stdout-path node is respected by the boot wrapper, but the main kernel always uses the first serial port mentioned in the device tree as the console. If you have multiple serial ports in your .dts, try re-ordering them. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: How to bring up fs_enet on 2.6.27?
Daniel Ng wrote: >> Now, I'm seeing these boot messages: >> >> f0010d40:00 not found >> eth0: Could not attach to PHY Daniel, These messages are typical of having the wrong GPIO pins in the mdio node or the wrong MDIO address (reg property) in the ethernet-phy node. >> Currently, our PHY >> attributes eg. 'auto-negotiate' are not changeable, so we aren't >> actually using MDC+MDIO even the MDC+MDIO lines exist. The driver definitely tries to talk to the PHY using the GPIO pins and address specified and if it doesn't respond, it won't attach. >> Also, the PHY >> interrupt line is not wired up. Hence the PHY0 interrupts field is <0 >> 8> (or should it be removed altogether?). The mdio driver works without interrupts. I have no interrupt-parent or interrupts properties on my ethernet-phy node. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Calling wait_event_interruptible_timeout() in I2C wait functions
Timur Tabi wrote: > However, it appears that this is not common behavior for I2C driver. In > fact, only these six drivers ever call wait_event_interruptible_timeout(): > > i2c-cpm.c I don't know about the others, but in i2c-cpm.c the use of interruptible wait seems incorrect. Maybe it could be made correct, but as is, it does not correctly clean up the hardware state or return a useful value when interrupted by a signal. It's not clear what to do, anyway - it's hard to know which messages of the interrupted transaction have actually taken effect in the hardware. I think it's better to use uninterruptible wait here and just live with the delayed signal handling (one second delay in the unlikely worst case for i2c-cpm). In general, I think it's best to consider I2C I/O to be uninterruptible, like disk I/O. The only reason to make it interruptible is to make sure you don't end up with an unkillable process due to an I/O error, and that is adequately handled by a timeout (and re-initialization of the I2C interface in that case). -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: How to bring up fs_enet on 2.6.27?
Daniel Ng wrote: > Should the device just be available as 'eth2', so that I can do > 'ifconfig eth2 192.168.1.33'? It will be eth0 if you have no other network devices. > // FCC2- > reg = <0x11320 0x20 0x8500 0x100 0x113b0 0x1>; > fsl,cpm-command = <0x12000300>; That's not the right fsl,cpm-command for FCC2. You want 0x16200300. Also, for my similar board, I had to add (for immap at F000): virtual-reg = <0xF0011320 0xF0008500 0xF00113B0>; But I can't explain why the driver isn't attaching for you. Did you try it built-in instead of as a module? -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Erratic MPC8248 CPM2 I2C behaviour
Jocke, > The I2C_CHIP_ERRATA is mine and refers to mpc8xx, mpc860 in my case. The > driver was adapted by me some years ago in 2.4 and now someone has > ported it to 2.6 it seems. Thanks for the background info. Any idea which particular errata it was meant to fix? I took a quick look at MPC860 errata document and it seems that CPM5 and CPM6 both suggest this workaround. I don't think any of the 8xx CPM errata are present on 82xx, but it would be good to enable the same or similar workaround on CPM2 for 82xx erratum CPM98, probably even when I2C_CHIP_ERRATA is not defined. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Erratic MPC8248 CPM2 I2C behaviour
Hi, Laurent, > While the problem seems to be similar to CPM98, I don't understand how it > could happen on the first character of the first I2C transfer. I agree, that is hard to explain given that i2c-cpm keeps the controller shut off until the very moment of the first transfer. Can you check that the bus is really idle (SCL is high) when the timeout happens? If one of the slave devices is in a bad state and pulling SCL low, I think the behavior you see is expected (CPM will wait forever for the bus to be idle). The same is probably true if a GPIO pin is mis-configured. > As explained in my previous mail to Joakim, I spent some more time last > Friday > investigating the problem, and it seems the baud rate generator configuration > plays an important role. The default configuration (60kHz nominal => > 65.104kHz > using a 25MHz brg clock and a /32 predivider) leads to timeouts, while I > haven't been able to reproduce the problem with the i2c-mpc8260.c > configuration (100kHz nominal => 104.167kHz using a 25MHz brg clock and a /8 > predivider). It would be interesting to try each driver with the other's clock settings. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Erratic MPC8248 CPM2 I2C behaviour
(replying to myself again) I wrote: > But the key difference is that we see a persistent failure, while the > erratum only mentions a problem with "the next transaction". I think the timeout recovery code is not adequate for these CPM errors, and that is why a transient error becomes a persistent one. The function cpm_i2c_force_close in i2c-cpm.c sends a CPM_CR_CLOSE_RX_BD command, which will abort any receive transaction in progress, but if it's the transmit phase that got the CPM hung up, there is no code to abort that (and there does not exist a CP command to do so, as far as I can tell). So the I2C disable/enable seems to be an appropriate fix for the persistent failure part of the problem. Again, this doesn't explain how it gets hung up the first time. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Erratic MPC8248 CPM2 I2C behaviour
I wrote: > Our production equipment is using Linux 2.6 with the out-of-tree > i2c-algo-8260.c by Dan Malek and Brad Parker. Oops, I meant to say Linux 2.4.20 (MontaVista). -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Erratic MPC8248 CPM2 I2C behaviour
Laurent Pinchart <[EMAIL PROTECTED]> wrote: > Transmission timeout after one second. The first TX buffer descriptor status > hasn't been modified by the CPM. The CPM state dump shows that processing of ... This sounds very similar to a problem I have seen on MPC8247 and MPC8248. It could be a variation of the CPM bug documented by Freescale as erratum CPM98. But the key difference is that we see a persistent failure, while the erratum only mentions a problem with "the next transaction". What we see is that once the I2C engine gets confused by some anomaly on the SCL signal, it stops processing buffer descriptors entirely until it is turned off and back on. You can observe this bug by momentarily grounding the SCL line (it's an open-collector bus) with a jumper while you attempt an I/O. Our production equipment is using Linux 2.6 with the out-of-tree i2c-algo-8260.c by Dan Malek and Brad Parker. I modified this driver to shut off the I2C controller and turn it back on when it hits this problem, and now it can recover from this condition. However, this does not explain how the controller gets into the frozen state in the first place. We see it very rarely in production units and have not been able to identify the cause. If it is similar to erratum CPM98 then it could be noise on the SCL signal or perhaps an I2C device that doesn't conform to the protocol perfectly. Also beware, if you are using some kind of multiplexer, that you don't direct the multiplexer to switch to a different bus (segment) while a transaction is in progress. In testing with the current (2.6.27) i2c-cpm.c driver, I found that it is sufficient to #define I2C_CHIP_ERRATA to allow it to recover from the CPM I2C freeze. However, I don't know if I like this approach because it turns off the I2C engine after every transfer, even successful ones, and I don't know if this will affect reliability or performance. And I don't know if this will actually prevent the freeze from happening, although I presume that it would protect the I2C engine from getting confused by a glitch that happens while no transfer is in progress. I am not aware of any documentation for what exactly the I2C_CHIP_ERRATA conditional code in i2c-cpm.c is meant to work around. The comment mentions "earlier than rev D4" but I'm not aware of any such rev for 8260 or 8272 chip families, and if it is related to erratum CPM98, Freescale seems to say that this erratum is in all revs of these chips and has no plan to fix it, so it seems that the workaround should be enabled by default. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
This patch adds the ability to enable the digital filter in the device
tree (with the "clock-filter" boolean property) and automates the
predivider selection according to the clock-frequency and clock-filter
properties. This allows use of a wider range of I2C bus frequencies.
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
This patch is against 2.6.27.
To use the full range of I2C clock frequencies supported by the
Freescale CPM I2C controller, it is necessary to choose an appropriate
predivider value. The choice is affected by whether the SCL signal
digital filter is enabled.
The existing code computes the final divider (i2brg) but always uses
a predivider of 32. This can set illegal values in i2brg - for example
on a machine with 25 MHz BRG_CLK, when selecting I2C clock-frequency
97656 Hz, the code was loading i2brg with the value 1, which does not
work (the CPM requires a minimum value of 3 when the digital filter is
not enabled). Additionally, the calculation did not work when the
filter is enabled (and the driver did not provide a way to enable it).
Index: linux/drivers/i2c/busses/i2c-cpm.c
===
diff -u -p -r1.1.1.1 i2c-cpm.c
--- linux/drivers/i2c/busses/i2c-cpm.c 11 Oct 2008 02:53:07 - 1.1.1.1
+++ linux/drivers/i2c/busses/i2c-cpm.c 6 Nov 2008 01:45:15 -
@@ -87,6 +87,11 @@ struct i2c_ram {
#define I2CER_TXB 0x02
#define I2CER_RXB 0x01
#define I2MOD_EN 0x01
+#define I2MOD_PDIV_32 0x00/* BRGCLK/32 */
+#define I2MOD_PDIV_16 0x02/* BRGCLK/16 */
+#define I2MOD_PDIV_8 0x04/* BRGCLK/8 */
+#define I2MOD_PDIV_4 0x06/* BRGCLK/4 */
+#define I2MOD_FLT 0x08
/* I2C Registers */
struct i2c_reg {
@@ -111,7 +116,6 @@ struct cpm_i2c {
int version; /* CPM1=1, CPM2=2 */
int irq;
int cp_command;
- int freq;
struct i2c_reg __iomem *i2c_reg;
struct i2c_ram __iomem *i2c_ram;
u16 i2c_addr;
@@ -365,6 +369,7 @@ static int cpm_i2c_xfer(struct i2c_adapt
pmsg = &msgs[tptr];
if (pmsg->flags & I2C_M_RD)
ret = wait_event_interruptible_timeout(cpm->i2c_wait,
+ (in_be16(&tbdf[tptr].cbd_sc) & BD_SC_NAK) ||
!(in_be16(&rbdf[rptr].cbd_sc) & BD_SC_EMPTY),
1 * HZ);
else
@@ -434,7 +439,8 @@ static int __devinit cpm_i2c_setup(struc
void __iomem *i2c_base;
cbd_t __iomem *tbdf;
cbd_t __iomem *rbdf;
- unsigned char brg;
+ uint freq, maxfreq, prediv;
+ unsigned char mod, brg;
dev_dbg(&cpm->ofdev->dev, "cpm_i2c_setup()\n");
@@ -508,9 +514,15 @@ static int __devinit cpm_i2c_setup(struc
data = of_get_property(ofdev->node, "clock-frequency", &len);
if (data && len == 4)
- cpm->freq = *data;
+ freq = *data;
else
- cpm->freq = 6; /* use 60kHz i2c clock by default */
+ freq = 6; /* use 60kHz I2C clock by default */
+
+ data = of_get_property(ofdev->node, "clock-filter", &len);
+ if (data && len == 0)
+ mod = I2MOD_FLT;
+ else
+ mod = 0;
/*
* Allocate space for CPM_MAXBD transmit and receive buffer
@@ -552,8 +564,8 @@ static int __devinit cpm_i2c_setup(struc
cpm_reset_i2c_params(cpm);
- dev_dbg(&cpm->ofdev->dev, "i2c_ram 0x%p, i2c_addr 0x%04x, freq %d\n",
- cpm->i2c_ram, cpm->i2c_addr, cpm->freq);
+ dev_dbg(&cpm->ofdev->dev, "i2c_ram 0x%p, i2c_addr 0x%04x, freq %u\n",
+ cpm->i2c_ram, cpm->i2c_addr, freq);
dev_dbg(&cpm->ofdev->dev, "tbase 0x%04x, rbase 0x%04x\n",
(u8 __iomem *)cpm->tbase - DPRAM_BASE,
(u8 __iomem *)cpm->rbase - DPRAM_BASE);
@@ -566,14 +578,48 @@ static int __devinit cpm_i2c_setup(struc
out_8(&cpm->i2c_reg->i2add, 0x7f << 1);
/*
-* PDIV is set to 00 in i2mod, so brgclk/32 is used as input to the
-* i2c baud rate generator. This is divided by 2 x (DIV + 3) to get
-* the actual i2c bus frequency.
+* Compute the clock predivider and final divider to generate something
+* close to the desired I2C clock frequency. Use the largest predivider
+* possible. i2brg must be >= 6 when using I2MOD_FLT, otherwise >= 3.
+* So compute the highest frequency that will work with I2MOD_PDIV_32;
+* if that isn't high enough, see if we can use I2MOD_PDIV_16, etc.
+* Then choose a final divider that will generate at least the desired
+* clock frequency. Note the "at least" -- this round
Re: [PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
[including extra context because some of the thread went to the wrong I2C list] David Gibson wrote: > On Wed, Nov 05, 2008 at 04:35:03PM -0800, Mike Ditto wrote: >> David Gibson wrote: >> > What does worry me, however, is the description says it's about >> > whether the driver "should" enable the filter. Generally the device >> > tree doesn't attempt to say what users "should" do with the hardware, >> > just what the characteristics of the hardware are. >> > >> > What's the underlying difference here that affects the driver's choice >> > to enable the filter or not? >> >> I think it's a hardware/environment design parameter - e.g. if the I2C >> bus has hot-pluggable devices, long PCB traces, or a hierarchy of >> multiplexed bus segments, these can result in a noisy SCL signal that >> needs to be filtered. It's also a recommended mitigation for errata >> in certain CPU revs. > > Ah, ok. Well the CPU revision thing could be selected based on the > CPU revision, but the other conditions are a property of the board > wiring. Obviously it's hard to precisely characterize what it says > about the hardware, which is usually best avoided for devtree > properties, but I can see why this is more-or-less unavoidable in this > case. > > Ok. This property name and meaning looks ok to me. I would suggest a > note in the binding roughly explaining what leads to the property > being set (basically what you just told me in the paragraph above). Will do. I'll send a revised patch shortly. Thanks, -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
David Gibson wrote: > What does worry me, however, is the description says it's about > whether the driver "should" enable the filter. Generally the device > tree doesn't attempt to say what users "should" do with the hardware, > just what the characteristics of the hardware are. > > What's the underlying difference here that affects the driver's choice > to enable the filter or not? I think it's a hardware/environment design parameter - e.g. if the I2C bus has hot-pluggable devices, long PCB traces, or a hierarchy of multiplexed bus segments, these can result in a noisy SCL signal that needs to be filtered. It's also a recommended mitigation for errata in certain CPU revs. -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-cpm: Detect and report NAK right away instead of timing out.
Ben Dooks wrote: When reading from a device that is not present or declines to respond to, e.g., a non-existent register address, CPM immediately reports a NAK condition in the TxBD, but the driver kept waiting until a timeout, which takes 1 second and causes an ugly console error message. hmm, that block of text could probably go into the patch description. It's certainly fine with me if you want to include it. I was just trying, perhaps inappropriately, to separate the "bug report" from the "description of the change". -=] Mike [=- -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c-cpm: Detect and report NAK right away instead of timing out.
Make the driver report an ENXIO error immediately upon NAK instead of waiting for another interrupt and getting a timeout. Signed-off-by: Mike Ditto <[EMAIL PROTECTED]> --- When reading from a device that is not present or declines to respond to, e.g., a non-existent register address, CPM immediately reports a NAK condition in the TxBD, but the driver kept waiting until a timeout, which takes 1 second and causes an ugly console error message. Index: linux/drivers/i2c/busses/i2c-cpm.c === retrieving revision 1.3 diff -u -p -r1.3 i2c-cpm.c --- linux/drivers/i2c/busses/i2c-cpm.c 31 Oct 2008 06:36:08 - 1.3 +++ linux/drivers/i2c/busses/i2c-cpm.c 1 Nov 2008 00:12:45 - @@ -369,6 +369,7 @@ static int cpm_i2c_xfer(struct i2c_adapt pmsg = &msgs[tptr]; if (pmsg->flags & I2C_M_RD) ret = wait_event_interruptible_timeout(cpm->i2c_wait, + (in_be16(&tbdf[tptr].cbd_sc) & BD_SC_NAK) || !(in_be16(&rbdf[rptr].cbd_sc) & BD_SC_EMPTY), 1 * HZ); else ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
[redirecting again to the new i2c mailing list] Jochen Friedrich wrote: > What needs to be done though is to document this change in > Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt. How about the below? I'll wait for David to comment on the property name and for any suggestions on the documentation below, then I'll submit a new patch. -=] Mike [=- --- linux/Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt 11 Oct 2008 02:49:31 - 1.1.1.1 +++ linux/Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt 31 Oct 2008 21:15:06 - @@ -10,8 +10,12 @@ - #address-cells : Should be one. The cell is the i2c device address with the r/w bit set to zero. - #size-cells : Should be zero. -- clock-frequency : Can be used to set the i2c clock frequency. If - unspecified, a default frequency of 60kHz is being used. +- clock-frequency : Can be used to set the desired i2c clock frequency (in Hz). + If unspecified, a default of 60 kHz is being used. The actual frequency may + be somewhat higher than this value, depending on how the BRG_CLK and dividers + work out. +- clock-filter : boolean; if defined, indicates that this controller + should enable the SCL digital filter. The following two properties are deprecated. They are only used by legacy i2c drivers to find the bus to probe: - linux,i2c-index : Can be used to hard code an i2c bus number. By default, ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
Jochen Friedrich wrote: > What needs to be done though is to document this change in > Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt. How about the below? I'll wait for David to comment on the property name and for any suggestions on the documentation below, then I'll submit a new patch. --- linux/Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt 11 Oct 2008 02:49:31 - 1.1.1.1 +++ linux/Documentation/powerpc/dts-bindings/fsl/cpm_qe/cpm/i2c.txt 31 Oct 2008 21:15:06 - @@ -10,8 +10,12 @@ - #address-cells : Should be one. The cell is the i2c device address with the r/w bit set to zero. - #size-cells : Should be zero. -- clock-frequency : Can be used to set the i2c clock frequency. If - unspecified, a default frequency of 60kHz is being used. +- clock-frequency : Can be used to set the desired i2c clock frequency (in Hz). + If unspecified, a default of 60 kHz is being used. The actual frequency may + be somewhat higher than this value, depending on how the BRG_CLK and dividers + work out. +- clock-filter : boolean; if defined, indicates that this controller + should enable the SCL digital filter. The following two properties are deprecated. They are only used by legacy i2c drivers to find the bus to probe: - linux,i2c-index : Can be used to hard code an i2c bus number. By default, ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
This patch adds the ability to enable the digital filter in the device
tree (with the "clock-filter" boolean property) and automates the
predivider selection according to the clock-frequency and clock-filter
properties.
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
Resending to moved i2c mailing list.
This patch is against 2.6.27.
To use the full range of I2C clock frequencies supported by the
Freescale CPM I2C controller, it is necessary to choose an appropriate
predivider value. The choice is affected by whether the SCL signal
digital filter is enabled.
The existing code computes the final divider (i2brg) but always uses
a predivider of 32. This can set illegal values in i2brg - for example
on a machine with 25 MHz BRG_CLK, when selecting I2C clock-frequency
97656 Hz, the code was loading i2brg with the value 1, which does not
work (the CPM requires a minimum value of 3 when the digital filter is
not enabled). Additionally, the calculation did not work when the
filter is enabled (and the driver did not provide a way to enable it).
And by the way, thanks to Jochen Friedrich for integrating this driver
on the very day that I went looking for it. :-)
Index: linux/drivers/i2c/busses/i2c-cpm.c
===
RCS file: /n/home2/mditto/ws/myrepo/kernel/linux/drivers/i2c/busses/i2c-cpm.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 i2c-cpm.c
--- linux/drivers/i2c/busses/i2c-cpm.c 11 Oct 2008 02:53:07 - 1.1.1.1
+++ linux/drivers/i2c/busses/i2c-cpm.c 31 Oct 2008 04:05:41 -
@@ -87,6 +87,11 @@ struct i2c_ram {
#define I2CER_TXB 0x02
#define I2CER_RXB 0x01
#define I2MOD_EN 0x01
+#define I2MOD_PDIV_32 0x00/* BRGCLK/32 */
+#define I2MOD_PDIV_16 0x02/* BRGCLK/16 */
+#define I2MOD_PDIV_8 0x04/* BRGCLK/8 */
+#define I2MOD_PDIV_4 0x06/* BRGCLK/4 */
+#define I2MOD_FLT 0x08
/* I2C Registers */
struct i2c_reg {
@@ -111,7 +116,6 @@ struct cpm_i2c {
int version; /* CPM1=1, CPM2=2 */
int irq;
int cp_command;
- int freq;
struct i2c_reg __iomem *i2c_reg;
struct i2c_ram __iomem *i2c_ram;
u16 i2c_addr;
@@ -434,7 +438,8 @@ static int __devinit cpm_i2c_setup(struc
void __iomem *i2c_base;
cbd_t __iomem *tbdf;
cbd_t __iomem *rbdf;
- unsigned char brg;
+ uint freq, maxfreq, prediv;
+ unsigned char mod, brg;
dev_dbg(&cpm->ofdev->dev, "cpm_i2c_setup()\n");
@@ -508,9 +513,15 @@ static int __devinit cpm_i2c_setup(struc
data = of_get_property(ofdev->node, "clock-frequency", &len);
if (data && len == 4)
- cpm->freq = *data;
+ freq = *data;
else
- cpm->freq = 6; /* use 60kHz i2c clock by default */
+ freq = 6; /* use 60kHz I2C clock by default */
+
+ data = of_get_property(ofdev->node, "clock-filter", &len);
+ if (data && len == 0)
+ mod = I2MOD_FLT;
+ else
+ mod = 0;
/*
* Allocate space for CPM_MAXBD transmit and receive buffer
@@ -552,8 +563,8 @@ static int __devinit cpm_i2c_setup(struc
cpm_reset_i2c_params(cpm);
- dev_dbg(&cpm->ofdev->dev, "i2c_ram 0x%p, i2c_addr 0x%04x, freq %d\n",
- cpm->i2c_ram, cpm->i2c_addr, cpm->freq);
+ dev_dbg(&cpm->ofdev->dev, "i2c_ram 0x%p, i2c_addr 0x%04x, freq %u\n",
+ cpm->i2c_ram, cpm->i2c_addr, freq);
dev_dbg(&cpm->ofdev->dev, "tbase 0x%04x, rbase 0x%04x\n",
(u8 __iomem *)cpm->tbase - DPRAM_BASE,
(u8 __iomem *)cpm->rbase - DPRAM_BASE);
@@ -566,20 +577,53 @@ static int __devinit cpm_i2c_setup(struc
out_8(&cpm->i2c_reg->i2add, 0x7f << 1);
/*
-* PDIV is set to 00 in i2mod, so brgclk/32 is used as input to the
-* i2c baud rate generator. This is divided by 2 x (DIV + 3) to get
-* the actual i2c bus frequency.
+* Compute the clock predivider and final divider to generate something
+* close to the desired I2C clock frequency. Use the largest predivider
+* possible. i2brg must be >= 6 when using I2MOD_FLT, otherwise >= 3.
+* So compute the highest frequency that will work with I2MOD_PDIV_32;
+* if that isn't high enough, see if we can use I2MOD_PDIV_16, etc.
+* Then choose a final divider that will generate at least the desired
+* clock frequency. Note the "at least" -- this rounds upward, not
+* toward the nearest available frequency.
+*
+* I2C frequency for a given predivider and i2brg value is:
+* i2cfreq = brgfreq / prediv / 2 / (i2brg + 3 + 2 * flt)
+* i2brg value for a given
[PATCH] i2c-cpm: Add flexibility for I2C clock frequency and filter.
This patch adds the ability to enable the digital filter in the device
tree (with the "clock-filter" boolean property) and automates the
predivider selection according to the clock-frequency and clock-filter
properties.
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
This patch is against 2.6.27.
To use the full range of I2C clock frequencies supported by the
Freescale CPM I2C controller, it is necessary to choose an appropriate
predivider value. The choice is affected by whether the SCL signal
digital filter is enabled.
The existing code computes the final divider (i2brg) but always uses
a predivider of 32. This can set illegal values in i2brg - for example
on a machine with 25 MHz BRG_CLK, when selecting I2C clock-frequency
97656 Hz, the code was loading i2brg with the value 1, which does not
work (the CPM requires a minimum value of 3 when the digital filter is
not enabled). Additionally, the calculation did not work when the
filter is enabled (and the driver did not provide a way to enable it).
And by the way, thanks to Jochen Friedrich for integrating this driver
on the very day that I went looking for it. :-)
Index: linux/drivers/i2c/busses/i2c-cpm.c
===
RCS file: /n/home2/mditto/ws/myrepo/kernel/linux/drivers/i2c/busses/i2c-cpm.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 i2c-cpm.c
--- linux/drivers/i2c/busses/i2c-cpm.c 11 Oct 2008 02:53:07 - 1.1.1.1
+++ linux/drivers/i2c/busses/i2c-cpm.c 31 Oct 2008 04:05:41 -
@@ -87,6 +87,11 @@ struct i2c_ram {
#define I2CER_TXB 0x02
#define I2CER_RXB 0x01
#define I2MOD_EN 0x01
+#define I2MOD_PDIV_32 0x00/* BRGCLK/32 */
+#define I2MOD_PDIV_16 0x02/* BRGCLK/16 */
+#define I2MOD_PDIV_8 0x04/* BRGCLK/8 */
+#define I2MOD_PDIV_4 0x06/* BRGCLK/4 */
+#define I2MOD_FLT 0x08
/* I2C Registers */
struct i2c_reg {
@@ -111,7 +116,6 @@ struct cpm_i2c {
int version; /* CPM1=1, CPM2=2 */
int irq;
int cp_command;
- int freq;
struct i2c_reg __iomem *i2c_reg;
struct i2c_ram __iomem *i2c_ram;
u16 i2c_addr;
@@ -434,7 +438,8 @@ static int __devinit cpm_i2c_setup(struc
void __iomem *i2c_base;
cbd_t __iomem *tbdf;
cbd_t __iomem *rbdf;
- unsigned char brg;
+ uint freq, maxfreq, prediv;
+ unsigned char mod, brg;
dev_dbg(&cpm->ofdev->dev, "cpm_i2c_setup()\n");
@@ -508,9 +513,15 @@ static int __devinit cpm_i2c_setup(struc
data = of_get_property(ofdev->node, "clock-frequency", &len);
if (data && len == 4)
- cpm->freq = *data;
+ freq = *data;
else
- cpm->freq = 6; /* use 60kHz i2c clock by default */
+ freq = 6; /* use 60kHz I2C clock by default */
+
+ data = of_get_property(ofdev->node, "clock-filter", &len);
+ if (data && len == 0)
+ mod = I2MOD_FLT;
+ else
+ mod = 0;
/*
* Allocate space for CPM_MAXBD transmit and receive buffer
@@ -552,8 +563,8 @@ static int __devinit cpm_i2c_setup(struc
cpm_reset_i2c_params(cpm);
- dev_dbg(&cpm->ofdev->dev, "i2c_ram 0x%p, i2c_addr 0x%04x, freq %d\n",
- cpm->i2c_ram, cpm->i2c_addr, cpm->freq);
+ dev_dbg(&cpm->ofdev->dev, "i2c_ram 0x%p, i2c_addr 0x%04x, freq %u\n",
+ cpm->i2c_ram, cpm->i2c_addr, freq);
dev_dbg(&cpm->ofdev->dev, "tbase 0x%04x, rbase 0x%04x\n",
(u8 __iomem *)cpm->tbase - DPRAM_BASE,
(u8 __iomem *)cpm->rbase - DPRAM_BASE);
@@ -566,20 +577,53 @@ static int __devinit cpm_i2c_setup(struc
out_8(&cpm->i2c_reg->i2add, 0x7f << 1);
/*
-* PDIV is set to 00 in i2mod, so brgclk/32 is used as input to the
-* i2c baud rate generator. This is divided by 2 x (DIV + 3) to get
-* the actual i2c bus frequency.
+* Compute the clock predivider and final divider to generate something
+* close to the desired I2C clock frequency. Use the largest predivider
+* possible. i2brg must be >= 6 when using I2MOD_FLT, otherwise >= 3.
+* So compute the highest frequency that will work with I2MOD_PDIV_32;
+* if that isn't high enough, see if we can use I2MOD_PDIV_16, etc.
+* Then choose a final divider that will generate at least the desired
+* clock frequency. Note the "at least" -- this rounds upward, not
+* toward the nearest available frequency.
+*
+* I2C frequency for a given predivider and i2brg value is:
+* i2cfreq = brgfreq / prediv / 2 / (i2brg + 3 + 2 * flt)
+* i2brg value for a given predivider and I2C frequency is:
+*
Re: [PATCH v2] powerpc/fs_enet: Add missing irq free in error path.
If something goes wrong attaching to phy driver, we weren't freeing
the IRQ.
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
Reposting to CC netdev list. (Thanks, Kumar)
diff -r -upN 2.6.28-rc1/drivers/net/fs_enet/fs_enet-main.c
NEW/drivers/net/fs_enet/fs_enet-main.c
--- 2.6.28-rc1/drivers/net/fs_enet/fs_enet-main.c 2008-10-24
17:54:31.0 -0700
+++ NEW/drivers/net/fs_enet/fs_enet-main.c 2008-10-24 17:57:03.0
-0700
@@ -795,6 +795,7 @@ static int fs_enet_open(struct net_devic
err = fs_init_phy(dev);
if (err) {
+ free_irq(fep->interrupt, dev);
if (fep->fpi->use_napi)
napi_disable(&fep->napi);
return err;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2] powerpc: Add missing irq free in fs_enet error path.
If something goes wrong attaching to phy driver, we weren't freeing the IRQ.
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
I just noticed this file has already been touched in -rc1 so here is the
patch again, adjusted accordingly.
diff -r -upN 2.6.28-rc1/drivers/net/fs_enet/fs_enet-main.c
NEW/drivers/net/fs_enet/fs_enet-main.c
--- 2.6.28-rc1/drivers/net/fs_enet/fs_enet-main.c 2008-10-24
17:54:31.0 -0700
+++ NEW/drivers/net/fs_enet/fs_enet-main.c 2008-10-24 17:57:03.0
-0700
@@ -795,6 +795,7 @@ static int fs_enet_open(struct net_devic
err = fs_init_phy(dev);
if (err) {
+ free_irq(fep->interrupt, dev);
if (fep->fpi->use_napi)
napi_disable(&fep->napi);
return err;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Add missing irq free in fs_enet error path.
If something goes wrong attaching to phy driver, we weren't freeing the IRQ.
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
cvs diff -r linux-2_6_27 -upN linux/drivers/net/fs_enet/fs_enet-main.c
Index: linux/drivers/net/fs_enet/fs_enet-main.c
===
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 fs_enet-main.c
--- linux/drivers/net/fs_enet/fs_enet-main.c11 Oct 2008 02:53:59 -
1.1.1.1
+++ linux/drivers/net/fs_enet/fs_enet-main.c24 Oct 2008 22:19:47 -
@@ -811,6 +811,7 @@ static int fs_enet_open(struct net_devic
err = fs_init_phy(dev);
if (err) {
+ fs_free_irq(dev, fep->interrupt);
if (fep->fpi->use_napi)
napi_disable(&fep->napi);
return err;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Add del_node function to allow early boot code to prune inapplicable devices.
Benjamin Herrenschmidt wrote: > I've fixed it up manually so no need to re-submit, but you'll know next > time :-) Thanks! -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Add del_node function to allow early boot code to prune inapplicable devices.
Reposting in proper format...
Some platforms have variants that can share most of a flat device tree but need
a few devices selectively pruned at boot time. This adds del_node() to ops.h
to allow access to the existing fdt_del_node().
Signed-off-by: Mike Ditto <[EMAIL PROTECTED]>
---
Index: arch/powerpc/boot/ops.h
===
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ops.h
--- arch/powerpc/boot/ops.h 11 Oct 2008 02:51:35 - 1.1.1.1
+++ arch/powerpc/boot/ops.h 18 Oct 2008 02:06:45 -
@@ -40,6 +40,7 @@
const int buflen);
int (*setprop)(const void *phandle, const char *name,
const void *buf, const int buflen);
+ int (*del_node)(const void *phandle);
void *(*get_parent)(const void *phandle);
/* The node must not already exist. */
void *(*create_node)(const void *parent, const char *name);
@@ -124,6 +125,11 @@
return dt_ops.setprop(devp, name, buf, strlen(buf) + 1);
return -1;
+}
+
+static inline int del_node(const void *devp)
+{
+ return dt_ops.del_node ? dt_ops.del_node(devp) : -1;
}
static inline void *get_parent(const char *devp)
Index: arch/powerpc/boot/libfdt-wrapper.c
===
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 libfdt-wrapper.c
--- arch/powerpc/boot/libfdt-wrapper.c 11 Oct 2008 02:51:35 - 1.1.1.1
+++ arch/powerpc/boot/libfdt-wrapper.c 17 Oct 2008 22:08:44 -
@@ -105,6 +105,11 @@
return check_err(rc);
}
+static int fdt_wrapper_del_node(const void *devp)
+{
+ return fdt_del_node(fdt, devp_offset(devp));
+}
+
static void *fdt_wrapper_get_parent(const void *devp)
{
return offset_devp(fdt_parent_offset(fdt, devp_offset(devp)));
@@ -173,6 +178,7 @@
dt_ops.create_node = fdt_wrapper_create_node;
dt_ops.find_node_by_prop_value = fdt_wrapper_find_node_by_prop_value;
dt_ops.find_node_by_compatible = fdt_wrapper_find_node_by_compatible;
+ dt_ops.del_node = fdt_wrapper_del_node;
dt_ops.get_path = fdt_wrapper_get_path;
dt_ops.finalize = fdt_wrapper_finalize;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: device tree variations
David Gibson wrote:
> Deleting the irrelevant parts or picking a device tree to pass to
> fdt_init() are both reasonable solutions. libfdt which is included in
> the bootwrapper has functions for removing unwanted nodes: either
> fdt_nop_node() or fdt_del_node() will suffice. There isn't currently
> a dt_ops hook to call though to those functions though. You could
> either add one, or (knowing that your platform always has a flat dt)
> bypass the dt_ops hooks and call libfdt directly.
Thanks. The fdt_del_node approach works pretty nicely. I added a
dt_ops hook since fdt is static in libfdt-wrapper.c.
At first I tried fdt_nop_node, fearing that find_node_by_prop_value()
and fdt_del_node() would interact badly when deleting in mid-traversal,
but it turns out that fdt_nop_node() upsets find_node_by_prop_value()
anyway. Plus, the kernel prints harmless but strange messages when
there are NOPs in the tree. So I use fdt_del_node() and rescan from
the top each time I delete a node and it works fine.
/* Find all product-dependent nodes and delete inapplicable ones. */
dev = NULL;
while ((dev = find_node_by_prop_value(dev,
"product-dependent",
"", 0)) != NULL) {
u32 mask;
int len;
len = getprop(dev, "productmask", &mask, sizeof mask);
if (len == sizeof mask) {
if ((mask & (1 << product_id)) == 0) {
del_node(dev);
/* Have to restart from the beginning. */
dev = NULL;
}
}
}
I had to fix the memcmp bug I mentioned in an earlier posting to allow
searching for a boolean (empty) property.
If anyone cares, below is a trivial patch to expose the del_node()
operation via dt_ops.
Thanks again,
-=] Mike [=-
Index: arch/powerpc/boot/ops.h
===
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ops.h
--- arch/powerpc/boot/ops.h 11 Oct 2008 02:51:35 - 1.1.1.1
+++ arch/powerpc/boot/ops.h 18 Oct 2008 02:06:45 -
@@ -40,6 +40,7 @@
const int buflen);
int (*setprop)(const void *phandle, const char *name,
const void *buf, const int buflen);
+ int (*del_node)(const void *phandle);
void *(*get_parent)(const void *phandle);
/* The node must not already exist. */
void *(*create_node)(const void *parent, const char *name);
@@ -124,6 +125,11 @@
return dt_ops.setprop(devp, name, buf, strlen(buf) + 1);
return -1;
+}
+
+static inline int del_node(const void *devp)
+{
+ return dt_ops.del_node ? dt_ops.del_node(devp) : -1;
}
static inline void *get_parent(const char *devp)
Index: arch/powerpc/boot/libfdt-wrapper.c
===
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 libfdt-wrapper.c
--- arch/powerpc/boot/libfdt-wrapper.c 11 Oct 2008 02:51:35 - 1.1.1.1
+++ arch/powerpc/boot/libfdt-wrapper.c 17 Oct 2008 22:08:44 -
@@ -105,6 +105,11 @@
return check_err(rc);
}
+static int fdt_wrapper_del_node(const void *devp)
+{
+ return fdt_del_node(fdt, devp_offset(devp));
+}
+
static void *fdt_wrapper_get_parent(const void *devp)
{
return offset_devp(fdt_parent_offset(fdt, devp_offset(devp)));
@@ -173,6 +178,7 @@
dt_ops.create_node = fdt_wrapper_create_node;
dt_ops.find_node_by_prop_value = fdt_wrapper_find_node_by_prop_value;
dt_ops.find_node_by_compatible = fdt_wrapper_find_node_by_compatible;
+ dt_ops.del_node = fdt_wrapper_del_node;
dt_ops.get_path = fdt_wrapper_get_path;
dt_ops.finalize = fdt_wrapper_finalize;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Bug in boot code memcmp with zero length
I noticed, when trying to use, e.g., node = find_node_by_prop_value(prev, "booleanprop", "", 0)) to search for all nodes with a certain boolean property, that memcmp() returns garbage when comparing zero bytes. It should return zero. Index: arch/powerpc/boot/string.S === retrieving revision 1.1.1.1 diff -u -r1.1.1.1 string.S --- arch/powerpc/boot/string.S 11 Oct 2008 02:51:35 - 1.1.1.1 +++ arch/powerpc/boot/string.S 17 Oct 2008 19:11:18 - @@ -235,7 +235,7 @@ .globl memcmp memcmp: cmpwi 0,r5,0 - blelr + ble 2f mtctr r5 addir6,r3,-1 addir4,r4,-1 @@ -243,6 +243,8 @@ lbzur0,1(r4) subf. r3,r0,r3 bdnzt 2,1b + blr +2: li r3,0 blr ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
device tree variations
I'm building a kernel that can run on a handful of hardware configurations, all using OF-unaware U-Boot. I know how to make a static device tree (dts file) that works on one of these hardware variations, and how to add nodes and modify properties in the platform init code. But I don't see a nice way to deal with nodes that should be present on only some hardware configurations. I could have the dts file contain only the common elements, and add all the variable elements in the startup code. But that means the bulk of the device tree will be expressed as relatively ugly C source instead of the much more readable and maintainable dts notation. I would much rather have the dts file contain the union of all platforms and have the platform init code delete the nodes that are not applicable, but I don't see an API to do those deletions. I suppose I could instead compile N different dts files and have the platform init code pick the appropriate dtb blob to pass to fdt_init(). Any suggestions for the best way to do this? -=] Mike [=- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
