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

[Wolfgang Grandegger <wg@grandegger.com>]

Grant Likely wrote:
> On Thu, Jul 31, 2008 at 5:51 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Grant Likely wrote:
>>> On Fri, Jul 25, 2008 at 11:19:41AM -0500, Timur Tabi wrote:
>>>> Wolfgang Grandegger wrote:
>>>>
>>>>> I know but we still need an algorithm for MPC52xx and MPC82xx as well.
>>>> That's true, but I still think hard-coding values of DFSR and FDR in the
>>>> device
>>>> tree is not a good way to do this.
>>> I agree, it should encode real frequencies, not raw register values.
>> Digging deeper I'm frightened by plenty of platform specific code. We would
>> need:
>>
>> - one table of divider,fdr,dfsr values for the MPC82/3/5/6xx processors
>>  (already available from Timur's U-Boot implementation)
>>
>> - one table of divider,fdr values for the MPC5200 rev A.
>>
>> - one table of divider,fdr values for the MPC5200 rev B.
>>  (the Rev. B has two more pre-scaler bits).
>>
>> - furthermore, there are various mpc-specific I2C clock sources:
>>
>>  MPC82xx                     : fsl_get_sys_freq()
>>  MPC5200                     : IPB
>>  MPC83xx                     : fsl_get_sys_freq()
>>  MPC8540/41/60/55,MPC8610    : fsl_get_sys_freq()
>>  MPC8543/45/47/48/68, MPC8641: fsl_get_sys_freq()/2
>>  MPC8544                     : fsl_get_sys_freq()/2 or /3
>>
>>  It would make sense to hand-over the I2C frequency from U-Boot to
>>  Linux.
> 
> U-Boot isn't always available and there are plenty of 5200 and 8xxx
> boards out there which will never have U-Boot reflashed to provide
> this data.  Also, there are boards that don't even use U-Boot.  I
> don't want to go down this path.  It is the drivers *job* to
> understand how to set these registers.
> 
> 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.

Yep, that makes sense.

Wolfgang.