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

[Timur Tabi <timur@freescale.com>]

Jon Smirl wrote:

> The existence of the dfsrr and fsl,mpc5200-i2c attributes imply to me
> that the compatible strings were not done correctly. If these devices
> really were compatible we wouldn't need extra attributes to tell them
> apart.

I agree with that.

> So I'm with Grant on adding compatible strings sufficient to remove
> the need for dfsrr and fsl,mpc5200-i2c.

Let's just make sure we don't break backwards compatibility.

> Something like...
> static const struct of_device_id mpc_i2c_of_match[] = {
>        {.compatible = "fsl,mpc5200b-i2c", .data = fsl_i2c_mpc5200, },
>        {.compatible = "fsl,mpc5200-i2c", .data = fsl_i2c_mpc5200, },
>        {.compatible = "fsl,mpc8xxx-i2c", .data = fsl_i2c_dfsrr, },

That seems ok.

> As for the source clock, how about creating a new global like
> ppc_proc_freq called ppc_ipb_freq. The platform code can then set the
> right clock value into the variable. For mpc8xxxx get it from uboot.
> mpc5200 can easily compute it from ppc_proc_freq and checking how the
> ipb divider is set. That will move the clock problem out of the i2c
> driver.

I don't want to differentiate between 52xx and 8xxx any more than we have to.
If 8xxx has the clock frequency in the device tree, then 52xx should have it
there, too.

For backwards compatibility purposes, we can have the driver provide a
hard-coded value of some kind if the property does not exist.

> I'd like to move towards a more generic uboot that gets the processor
> minimally running and then use the device tree to customize as many
> things as possible. But that's just my opinion.

U-Boot needs to configure the I2C bus speed because U-Boot uses the I2C.  We
should capitalize on that by taking the information that U-Boot has calculated
and re-use it.

-- 
Timur Tabi
Linux kernel developer at Freescale