Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

["Jon Smirl" <jonsmirl@gmail.com>]

On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Jul 31, 2008 at 11:06 AM, Jon Smirl <jonsmirl@gmail.com> wrote:
>  > On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
>  >>  If you're careful, the table doesn't need to be huge.  It can be
>  >>  marked as initdata and conditionally compiled depending on which
>  >>  architectures are compiled in.  You should use .data in the driver's
>  >>  of_device_id table to provide machine specific ops for setting
>  >>  clocking to avoid a maze of if/else statements.
>  >
>  > Does this look ok for the mpc5200 i2c struct?
>  >
>  > /* I2C Registers */
>  > struct mpc52xx_i2c {
>  >        u8 madr;                        /* I2C + 0x00 */
>  >        u8 reserved1[3];                /* I2C + 0x01 */
>  >        u8 mfdr;                        /* I2C + 0x04 */
>  >        u8 reserved2[3];                /* I2C + 0x05 */
>  >        u8 mcr;                         /* I2C + 0x08 */
>  >        u8 reserved3[3];                /* I2C + 0x09 */
>  >        u8 msr;                 /* I2C + 0x0c */
>  >        u8 reserved4[3];                /* I2C + 0x0d */
>  >        u8 mdr;                 /* I2C + 0x10 */
>  >        u8 reserved5[15];       /* I2C + 0x11 */
>  >        u8 interrupt;           /* I2C + 0x20 */
>  >        u8 reserved6[3];                /* I2C + 0x21 */
>  >        u8 mifr;                        /* I2C + 0x24 */
>  > };
>
>
> Ugh.  I hate all the registers defined in structures thing done for
>  5200, but I guess it is better to stick with established convention
>  than do it differently.

Isn't it better than adding random numbers to a pointer and having to
worry about what the pointer is pointing at so that it will multiply
by the right word size? That's a mess when the registers are different
lengths.

A common i2c struct might be a better idea that adds in the dfsrr
register might be better.

You can set the flags for dfsrr use in these set_freq routines too
since they select on CPU type.
       {.compatible = "fsl,mpc5200b-i2c", .data = fsl_i2c_mpc5200b_set_freq, },
       {.compatible = "fsl,mpc5200-i2c", .data = fsl_i2c_mpc5200_set_freq, },
       {.compatible = "fsl,mpc8260-i2c", .data = fsl_i2c_mpc8xxx_set_freq, },
       {.compatible = "fsl,mpc8349-i2c", .data = fsl_i2c_mpc8xxx_set_freq, },
       {.compatible = "fsl,mpc8540-i2c", .data = fsl_i2c_mpc8xxx_set_freq, },
       {.compatible = "fsl,mpc8543-i2c", .data =
fsl_i2c_mpc8xxx_div2_set_freq, },
       {.compatible = "fsl,mpc8544-i2c", .data =
fsl_i2c_mpc8xxx_div3_set_freq, },



>
>  Yes, I think this is okay (but I haven't double checked the values; I
>  trust you).
>
>
>  g.
>
>
>  --
>  Grant Likely, B.Sc., P.Eng.
>  Secret Lab Technologies Ltd.
>


-- 
Jon Smirl
jonsmirl@gmail.com