Hi Adrian Cox wrote:
>On Fri, 2004-07-02 at 12:01, Sylvain Munaut wrote: > > > >>#include <asm/ppcboot.h> >>extern bd_t __res; >>u32 ipbfreq = __res.bi_ipbfreq; >> >>But this field will only exists when : >> - CONFIG_PPC_MPC52xx symbol is defined. >> - When the MPC52xx patch is applied to the kernel >> >> > >Maybe we should define two more fields in the ocp_fs_i2c_data structure: >one for base clock, and one for i2c clock. Then platform code could fill >in the clocks as necessary. > > Yes, that's a good idea. It would need a flag to tell which clock computation to take though. Maybe instead of passing the baseclock, giving a pointer to a u32 that contains the clock would be more appropriate. Since all board have probably an int somewhere holding that value, you can put de definition in the ocp_def without any further modifications. >>Also, there is no DFSRR register on the 5200. >> >> > >I noticed that. I don't think anybody ever used anything but the default >value on the other chips. > > Yes. I've seen that the DFSRR is not as the same place. So maybe we would need a two bits flag like #define FS_I2C_NO_DFSRR 0x00 #define FS_I2C_PACKED_DFSRR 0x02 #define FS_I2C_SEPARATE_DFSRR 0x04 >>Are you sure ? If I don't set the BNBE (Bus Not Busy Enable) bit, I just >>get timeouts. >> >> > >>From the manual it looks as if setting BNBE might cause extra >interrupts, which the driver has no way to handle. Could you try >enabling the interrupts, and see if this happens? > > > >>Yes sure, that's the easiest way. It's just that I'd like to avoid it. >>Especially when it's content is dependent on if the user has choosed to >>use irq or not. >>But It's sure is a pity that the register is shared between the two I2C >>... Because even with a flag, the driver should be passed the address of >>this register, and what bits to use. >> >> > >How about putting a function pointer for platform interrupt enabling and >disabling into the ocp_fs_i2c_data? > > Well, you're right setting the BNBE is not right, it just hang because interrupts are fired and not handled. Now, here is the interesting part : If I set the ocp_def to use both I2C controller, defining the I2C1 before I2C2, it works fine. Now if I just invert the entry, it does not. Interrupts for I2C2 are never fired. If I only put I2C1, it seems to work ( no timout). With only I2C2, interrupts are never fired ... I have no clue why ! The best option is just to set IRQ to OCP_IRQ_NA, and it works fine, whatever the value of the shared interrupt registers is. >It looks to me that supporting the 5200 will require a lot of small >changes. > > Well, for the interrupt yes that a quirk. The solution of dropping interrupt completly for it is the best option for now. For the DFSRR register, the solution mentionned above is clean, the 3 chips requires different implementation anyway. For the clock thing, I think it's good to be able to set the I2C clock. Sylvain Munaut ** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/
