Re: [PATCH take2] [POWERPC] i2c: adds support for i2c bus on 8xx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jochen Friedrich wrote:
diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
index 8848e63..a526c02 100644
--- a/arch/powerpc/boot/dts/mpc885ads.dts
+++ b/arch/powerpc/boot/dts/mpc885ads.dts
@@ -213,6 +213,15 @@
                 fsl,cpm-command = <0080>;
                 linux,network-index = <2>;
             };
+
+            i2c@860 {
+                device_type = "i2c";

No device_type.

Why? Documentation/powerpc/booting-without-of.txt says for I2C interfaces
device_type is required and should be "i2c". Is this no longer true?

booting-without-of.txt should be changed.

Should be fsl,cpm-i2c.  Is cpm2 i2c the same?  If not, it should be
fsl,cpm1-i2c.  It's probably best to specify it anyway, along with
fsl,mpc885-i2c.

CPM2 i2c seems to be the same. However, i have no way to test this.

OK, let's make the compatible "fsl,mpc885-i2c", "fsl,cpm1-i2c", "fsl,cpm-i2c".

For now, match on the last one, but if any differences pop up, we have the more specific ones.

I noticed cpm1_set_pin32, but this function don't seem to set the
odr register. Will this be added? Then it would be:

    {CPM_PORTB, 26, CPM_PIN_OUTPUT | CPM_PIN_OPENDRAIN},
    {CPM_PORTB, 27, CPM_PIN_OUTPUT | CPM_PIN_OPENDRAIN},


Ah, missed that -- there's opendrain support for port E, but I missed that port B had it as well. Feel free to add it.

It's a 7-bit address...  and are you sure that 0x7e is unique?  Does this
driver even support slave operation?

It's in fact 0x7F << 1.

Gah, I hate powerpc bit numbering, especially without the numbered-as-if-64-bit hack. I specifically looked at the manual before to see if it was shifted, saw "0-6", and concluded it wasn't. :-P

The same value is used in the 2.4 driver and
in u-boot, as well.

That doesn't mean that this isn't a good time to review what the code is doing. :-)

Slave operation is not supported.

If slave operation isn't supported, how is this value used?

Why is an 8xx driver matching all i2c cpm (i.e. what about cpm2)?

With the suggested change to use fsl,cpm-command, the driver should
be able to use  both cpm1 and cpm2. The operation and structs for i2c
are identical. The only difference might be the hack^wsupport for
relocation.

OK. Would that allow it to become one driver, rather than a wrapper and an algorithm?

-Scott
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux