From: Sylvain Munaut <tnt@246tnt•com>
To: Adrian Cox <adrian@humboldt•co.uk>
Cc: mcclintock@freescale•com,
Embedded Linux PPC list <linuxppc-embedded@lists•linuxppc.org>,
"Kumar K. Gala" <kumar.gala@motorola•com>
Subject: Re: [PATCH][RFC]Updated MPC I2C driver
Date: Fri, 02 Jul 2004 17:11:04 +0200 [thread overview]
Message-ID: <40E57B08.3010204@246tNt.com> (raw)
In-Reply-To: <1088775864.933.60.camel@localhost>
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/
next prev parent reply other threads:[~2004-07-02 15:11 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-07-01 18:19 [PATCH][RFC]Updated MPC I2C driver Adrian Cox
2004-07-01 14:59 ` Matthew McClintock
2004-07-01 21:25 ` Adrian Cox
2004-07-01 22:32 ` Sylvain Munaut
2004-07-02 9:05 ` Adrian Cox
2004-07-02 11:01 ` Sylvain Munaut
2004-07-02 13:44 ` Adrian Cox
2004-07-02 15:11 ` Sylvain Munaut [this message]
2004-07-01 18:59 ` Eugene Surovegin
2004-07-01 19:20 ` Sylvain Munaut
2004-07-01 15:48 ` Matthew McClintock
2004-07-01 21:07 ` Sylvain Munaut
2004-07-01 20:54 ` Adrian Cox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40E57B08.3010204@246tNt.com \
--to=tnt@246tnt$(echo .)com \
--cc=adrian@humboldt$(echo .)co.uk \
--cc=kumar.gala@motorola$(echo .)com \
--cc=linuxppc-embedded@lists$(echo .)linuxppc.org \
--cc=mcclintock@freescale$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox