* [PATCH] powerpc/mm: Implement _PAGE_SPECIAL & pte_special() for 32-bit
From: Kumar Gala @ 2008-07-31 18:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin, linux-kernel
Implement _PAGE_SPECIAL and pte_special() for 32-bit powerpc. This bit will
be used by the fast get_user_pages() to differenciate PTEs that correspond
to a valid struct page from special mappings that don't such as IO mappings
obtained via io_remap_pfn_ranges().
We currently only implement this on sub-arch that support SMP or will so
in the future (6xx, 44x, FSL-BookE) and not (8xx, 40x).
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
Nick, do you forsee using _PAGE_SPECIAL for other applications that would
be of interested to non-SMP hw?
We can look at adding it into 8xx and 40x, but was being lazy as I assume
there is no point.
- k
include/asm-powerpc/pgtable-ppc32.h | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/asm-powerpc/pgtable-ppc32.h b/include/asm-powerpc/pgtable-ppc32.h
index 6fe39e3..58afaee 100644
--- a/include/asm-powerpc/pgtable-ppc32.h
+++ b/include/asm-powerpc/pgtable-ppc32.h
@@ -261,6 +261,7 @@ extern int icache_44x_need_flush;
#define _PAGE_HWEXEC 0x00000004 /* H: Execute permission */
#define _PAGE_ACCESSED 0x00000008 /* S: Page referenced */
#define _PAGE_DIRTY 0x00000010 /* S: Page dirty */
+#define _PAGE_SPECIAL 0x00000020 /* S: Special page */
#define _PAGE_USER 0x00000040 /* S: User page */
#define _PAGE_ENDIAN 0x00000080 /* H: E bit */
#define _PAGE_GUARDED 0x00000100 /* H: G bit */
@@ -276,6 +277,7 @@ extern int icache_44x_need_flush;
/* ERPN in a PTE never gets cleared, ignore it */
#define _PTE_NONE_MASK 0xffffffff00000000ULL
+#define __HAVE_ARCH_PTE_SPECIAL
#elif defined(CONFIG_FSL_BOOKE)
/*
@@ -305,6 +307,7 @@ extern int icache_44x_need_flush;
#define _PAGE_COHERENT 0x00100 /* H: M bit */
#define _PAGE_NO_CACHE 0x00200 /* H: I bit */
#define _PAGE_WRITETHRU 0x00400 /* H: W bit */
+#define _PAGE_SPECIAL 0x00800 /* S: Special page */
#ifdef CONFIG_PTE_64BIT
/* ERPN in a PTE never gets cleared, ignore it */
@@ -315,6 +318,8 @@ extern int icache_44x_need_flush;
#define _PMD_PRESENT_MASK (PAGE_MASK)
#define _PMD_BAD (~PAGE_MASK)
+#define __HAVE_ARCH_PTE_SPECIAL
+
#elif defined(CONFIG_8xx)
/* Definitions for 8xx embedded chips. */
#define _PAGE_PRESENT 0x0001 /* Page is valid */
@@ -362,6 +367,7 @@ extern int icache_44x_need_flush;
#define _PAGE_ACCESSED 0x100 /* R: page referenced */
#define _PAGE_EXEC 0x200 /* software: i-cache coherency required */
#define _PAGE_RW 0x400 /* software: user write access allowed */
+#define _PAGE_SPECIAL 0x800 /* software: Special page */
#define _PTE_NONE_MASK _PAGE_HASHPTE
@@ -372,6 +378,8 @@ extern int icache_44x_need_flush;
/* Hash table based platforms need atomic updates of the linux PTE */
#define PTE_ATOMIC_UPDATES 1
+#define __HAVE_ARCH_PTE_SPECIAL
+
#endif
/*
@@ -404,6 +412,9 @@ extern int icache_44x_need_flush;
#ifndef _PAGE_WRITETHRU
#define _PAGE_WRITETHRU 0
#endif
+#ifndef _PAGE_SPECIAL
+#define _PAGE_SPECIAL 0
+#endif
#ifndef _PMD_PRESENT_MASK
#define _PMD_PRESENT_MASK _PMD_PRESENT
#endif
@@ -533,7 +544,7 @@ static inline int pte_write(pte_t pte) { return pte_val(pte) & _PAGE_RW; }
static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; }
static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; }
static inline int pte_file(pte_t pte) { return pte_val(pte) & _PAGE_FILE; }
-static inline int pte_special(pte_t pte) { return 0; }
+static inline int pte_special(pte_t pte) { return pte_val(pte) & _PAGE_SPECIAL; }
static inline void pte_uncache(pte_t pte) { pte_val(pte) |= _PAGE_NO_CACHE; }
static inline void pte_cache(pte_t pte) { pte_val(pte) &= ~_PAGE_NO_CACHE; }
@@ -552,7 +563,7 @@ static inline pte_t pte_mkdirty(pte_t pte) {
static inline pte_t pte_mkyoung(pte_t pte) {
pte_val(pte) |= _PAGE_ACCESSED; return pte; }
static inline pte_t pte_mkspecial(pte_t pte) {
- return pte; }
+ pte_val(pte) |= _PAGE_SPECIAL; return pte; }
static inline unsigned long pte_pgprot(pte_t pte)
{
return __pgprot(pte_val(pte)) & PAGE_PROT_BITS;
--
1.5.5.1
^ permalink raw reply related
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 18:35 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <20080731182810.GB29097@secretlab.ca>
Grant Likely wrote:
> No it doesn't, it depends on the register interface to decide
> compatibility. Clock interface is part of that.
I don't think so. The interface for programming the clock registers is
identical on all 8[356]xx parts. The only thing that matters is what specific
values to put in the FDR and DFSR registers to get a desired I2C bus speed.
That answer is dependent on the actual clock input to the device, which is
external to the device. I wouldn't call the input frequency a property of the
I2C device.
> I suggested encoding
> the clock divider directly in compatible (implicit in the SoC version),
> but it doesn't have to be that way. If clock freq is obtained from
> another property or some other method then that is okay too.
I think we agree on that.
> There is nothing wrong with it (as long as we agree and it gets
> documented). I certainly don't have a problem with doing it this way.
I propose the property "clock-frequency", like this:
i2c@3000 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <0>;
compatible = "fsl-i2c";
reg = <0x3000 0x100>;
interrupts = <14 0x8>;
interrupt-parent = <&ipic>;
dfsrr;
clock-frequency = <0xblablabla>; <-- added by U-Boot
};
Note that the dfsrr property already differentiates between 8xxx and 52xx, so
maybe we don't need any other device tree changes.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 18:28 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <489200B6.9060906@freescale.com>
On Thu, Jul 31, 2008 at 01:13:10PM -0500, Timur Tabi wrote:
> Grant Likely wrote:
>
> > This is a solved problem. The device tree simple claims compatibility
> > with an older part that has the identical register-level interface.
>
> That would assume that the clock frequency is the only thing that decides
> compatibility, which may technically be true now, but I don't think it's a good
> idea.
No it doesn't, it depends on the register interface to decide
compatibility. Clock interface is part of that. I suggested encoding
the clock divider directly in compatible (implicit in the SoC version),
but it doesn't have to be that way. If clock freq is obtained from
another property or some other method then that is okay too.
What is important is that if compatibility is claimed, then it must
really be compatible! If some new part appears in the future that
breaks our assumptions, then we'll need to rework the driver anyway and
the new part will *not* claim compatibility with the old.
> I don't understand what's wrong with simply specifying the actual clock
> frequency that the device uses? It varies from SOC to SOC, but U-Boot
> calculates today already:
There is nothing wrong with it (as long as we agree and it gets
documented). I certainly don't have a problem with doing it this way.
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 18:21 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <48920016.5000506@freescale.com>
On Thu, Jul 31, 2008 at 01:10:30PM -0500, Timur Tabi wrote:
> Grant Likely wrote:
> > On Thu, Jul 31, 2008 at 12:07 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> >> We could add a property "source-clock-divider = <2/3>" if it's needed!?
> >
> > fsl,source-clock-divider
> >
> > But, yes, this is a good solution (assuming that it is a board or SoC
> > characteristic, and not just a choice that the driver has the option
> > of making on it's own).
>
> I was going to suggest the actual clock frequency of the I2C device. It would
> be value of gd->i2c1_clk or gd->i2c2_clk from U-Boot. The actual divider value
> is meaningless.
I'm cool with that too, as long as it gets documented
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 18:13 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <20080731180959.GA29057@secretlab.ca>
Grant Likely wrote:
> This is a solved problem. The device tree simple claims compatibility
> with an older part that has the identical register-level interface.
That would assume that the clock frequency is the only thing that decides
compatibility, which may technically be true now, but I don't think it's a good
idea.
I don't understand what's wrong with simply specifying the actual clock
frequency that the device uses? It varies from SOC to SOC, but U-Boot
calculates today already:
#if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
defined(CONFIG_MPC8560) || defined(CONFIG_MPC8555)
gd->i2c1_clk = sys_info.freqSystemBus;
#elif defined(CONFIG_MPC8544)
/*
* On the 8544, the I2C clock is the same as the SEC clock. This can be
* either CCB/2 or CCB/3, depending on the value of cfg_sec_freq. See
* 4.4.3.3 of the 8544 RM. Note that this might actually work for all
* 85xx, but only the 8544 has cfg_sec_freq, so it's unknown if the
* PORDEVSR2_SEC_CFG bit is 0 on all 85xx boards that are not an 8544.
*/
if (gur->pordevsr2 & MPC85xx_PORDEVSR2_SEC_CFG)
gd->i2c1_clk = sys_info.freqSystemBus / 3;
else
gd->i2c1_clk = sys_info.freqSystemBus / 2;
#else
/* Most 85xx SOCs use CCB/2, so this is the default behavior. */
gd->i2c1_clk = sys_info.freqSystemBus / 2;
#endif
gd->i2c2_clk = gd->i2c1_clk;
We need this ugliness in U-Boot. Let's take advantage of this and do something
clean and elegant in the device tree.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 18:10 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <fa686aa40807311107m44ce7fbk35e94d1a24b992fb@mail.gmail.com>
Grant Likely wrote:
> On Thu, Jul 31, 2008 at 12:07 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> We could add a property "source-clock-divider = <2/3>" if it's needed!?
>
> fsl,source-clock-divider
>
> But, yes, this is a good solution (assuming that it is a board or SoC
> characteristic, and not just a choice that the driver has the option
> of making on it's own).
I was going to suggest the actual clock frequency of the I2C device. It would
be value of gd->i2c1_clk or gd->i2c2_clk from U-Boot. The actual divider value
is meaningless.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 18:09 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <4891FC3A.7040609@freescale.com>
On Thu, Jul 31, 2008 at 12:54:02PM -0500, Timur Tabi wrote:
> Grant Likely wrote:
>
> > static const struct of_device_id mpc_i2c_of_match[] = {
> > {.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, },
>
> So we need to update this table every time there's a new SOC? All 83xx, 85xx,
> and 86xx SOCs use the same table. I'd prefer an implementation that does need a
> specific entry for each variant of 8[356]xx.
This is a solved problem. The device tree simple claims compatibility
with an older part that has the identical register-level interface.
For example; an mpc8540 based board would claim:
compatible = "fsl,mpc8540-i2c"
MPC8541/60/55,MPC8610 would all then claim:
compatible = "fsl,<exact-soc>-i2c", "fsl,mpc8540-i2c"
Cheers,
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 18:07 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <4891FF51.4020701@grandegger.com>
On Thu, Jul 31, 2008 at 12:07 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>
> We could add a property "source-clock-divider = <2/3>" if it's needed!?
fsl,source-clock-divider
But, yes, this is a good solution (assuming that it is a board or SoC
characteristic, and not just a choice that the driver has the option
of making on it's own).
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 18:07 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <4891FC3A.7040609@freescale.com>
Timur Tabi wrote:
> Grant Likely wrote:
>
>> static const struct of_device_id mpc_i2c_of_match[] = {
>> {.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, },
>
> So we need to update this table every time there's a new SOC? All 83xx, 85xx,
> and 86xx SOCs use the same table. I'd prefer an implementation that does need a
> specific entry for each variant of 8[356]xx.
We could add a property "source-clock-divider = <2/3>" if it's needed!?
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 18:06 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <4891FF51.4020701@grandegger.com>
Wolfgang Grandegger wrote:
> We could add a property "source-clock-divider = <2/3>" if it's needed!?
How about we just get U-Boot to put the core frequency in the device tree?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 17:54 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <fa686aa40807311031r66f6451aw206faf54509c14d9@mail.gmail.com>
Grant Likely wrote:
> static const struct of_device_id mpc_i2c_of_match[] = {
> {.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, },
So we need to update this table every time there's a new SOC? All 83xx, 85xx,
and 86xx SOCs use the same table. I'd prefer an implementation that does need a
specific entry for each variant of 8[356]xx.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 17:51 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C, Timur Tabi
In-Reply-To: <fa686aa40807311031r66f6451aw206faf54509c14d9@mail.gmail.com>
Grant Likely wrote:
> On Thu, Jul 31, 2008 at 11:22 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Jon Smirl wrote:
>>> On 7/31/08, 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).
>>> Aren't the tables in the manual there just to make it easy for a human
>>> to pick out the line they want? For a computer you'd program the
>>> formula that was used to create the tables.
>> I have the formulas to create the tables, also for the MPC5200 Rev. A and B.
>
> Oh, hey, even better.
>
>> That was not my point. I'm worried about arch specific code in i2c-mpc.c. It
>> should go somewhere to arch/powerpc.
>
> i2c-mpc *is* arch specific. I really don't think you need to worry
> about adding a block of code for each supported SoC family. Just
> change the of_match table to look something like this:
>
> static const struct of_device_id mpc_i2c_of_match[] = {
> {.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, },
>
> /* keep this only for older device trees with some support
> code to figure out
> what .data should have pointed to. */
> {.compatible = "fsl-i2c", },
> {},
> };
> MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
Cool, this would also make the "dfsrr" property obsolete. Just the
MPC8544 needs more attention because the I2C clock can be programmed to
be freq/2 or freq/3.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 17:47 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi
In-Reply-To: <fa686aa40807311036l1f081780g667129bbd4df3d9c@mail.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
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 17:36 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi
In-Reply-To: <9e4733910807311006t49372fa7m37bee031130cb4ac@mail.gmail.com>
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.
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.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 17:35 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Timur Tabi, Linux I2C, Linuxppc-dev
In-Reply-To: <4891F4D8.9090905@grandegger.com>
On 7/31/08, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Jon Smirl wrote:
> > There appears to be one for i2x8xxx but not the other CPUs.
> >
> > /* I2C
> > */
> > typedef struct i2c {
> > u_char i2c_i2mod;
> > char res1[3];
> > u_char i2c_i2add;
> > char res2[3];
> > u_char i2c_i2brg;
> > char res3[3];
> > u_char i2c_i2com;
> > char res4[3];
> > u_char i2c_i2cer;
> > char res5[3];
> > u_char i2c_i2cmr;
> > char res6[0x8b];
> > } i2c8xx_t;
> >
>
> The I2C interface for the MPC5200 is not compatible with the one for the
> MPC83/4/5/6xx, AFAIK.
Ignore that table, I mistook MPC8xx for MPC8xxx. That is a MPC8xx table.
>
> Wolfgang.
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 17:31 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linuxppc-dev, Linux I2C, Timur Tabi
In-Reply-To: <4891F4D8.9090905@grandegger.com>
On Thu, Jul 31, 2008 at 11:22 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Jon Smirl wrote:
>>
>> On 7/31/08, 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).
>>
>> Aren't the tables in the manual there just to make it easy for a human
>> to pick out the line they want? For a computer you'd program the
>> formula that was used to create the tables.
>
> I have the formulas to create the tables, also for the MPC5200 Rev. A and B.
Oh, hey, even better.
> That was not my point. I'm worried about arch specific code in i2c-mpc.c. It
> should go somewhere to arch/powerpc.
i2c-mpc *is* arch specific. I really don't think you need to worry
about adding a block of code for each supported SoC family. Just
change the of_match table to look something like this:
static const struct of_device_id mpc_i2c_of_match[] = {
{.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, },
/* keep this only for older device trees with some support
code to figure out
what .data should have pointed to. */
{.compatible = "fsl-i2c", },
{},
};
MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 17:24 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi
In-Reply-To: <fa686aa40807310951l48e2af8ep11ef83480397669a@mail.gmail.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.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 17:22 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Timur Tabi, Linux I2C, Linuxppc-dev
In-Reply-To: <9e4733910807310849g7e5612dbk9536733e061af8ad@mail.gmail.com>
Jon Smirl wrote:
> On 7/31/08, 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).
>
> Aren't the tables in the manual there just to make it easy for a human
> to pick out the line they want? For a computer you'd program the
> formula that was used to create the tables.
I have the formulas to create the tables, also for the MPC5200 Rev. A
and B. That was not my point. I'm worried about arch specific code in
i2c-mpc.c. It should go somewhere to arch/powerpc.
> I agree that it took me half an hour to figure out the formula that
> was needed to compute the i2s clocks, but once you figure out the
> formula it solves all of the cases and no one needs to read the manual
> any more. The i2c formula may even need a small loop which compares
> different solutions looking for the smallest error term. But it's a
> small space to search.
>
> These device tree flags should be removed, the driver can ask the
> platform code what CPU it is running on.
>
> if (of_get_property(op->node, "dfsrr", NULL))
> i2c->flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
>
> if (of_device_is_compatible(op->node, "fsl,mpc5200-i2c") ||
> of_device_is_compatible(op->node, "mpc5200-i2c"))
> i2c->flags |= FSL_I2C_DEV_CLOCK_5200;
>
> static void mpc_i2c_setclock(struct mpc_i2c *i2c)
> {
> /* Set clock and filters */
> if (i2c->flags & FSL_I2C_DEV_SEPARATE_DFSRR) {
> writeb(0x31, i2c->base + MPC_I2C_FDR);
> writeb(0x10, i2c->base + MPC_I2C_DFSRR);
> } else if (i2c->flags & FSL_I2C_DEV_CLOCK_5200)
> writeb(0x3f, i2c->base + MPC_I2C_FDR);
> else
> writel(0x1031, i2c->base + MPC_I2C_FDR);
> }
>
> These defines shouldn't be here, they should get the offset from the
> right header file for the CPU. But it appears that structures for the
> i2c memory map haven't been done for the various CPUs.
>
> #define MPC_I2C_FDR 0x04
> #define MPC_I2C_CR 0x08
> #define MPC_I2C_SR 0x0c
> #define MPC_I2C_DR 0x10
> #define MPC_I2C_DFSRR 0x14
>
> There appears to be one for i2x8xxx but not the other CPUs.
>
> /* I2C
> */
> typedef struct i2c {
> u_char i2c_i2mod;
> char res1[3];
> u_char i2c_i2add;
> char res2[3];
> u_char i2c_i2brg;
> char res3[3];
> u_char i2c_i2com;
> char res4[3];
> u_char i2c_i2cer;
> char res5[3];
> u_char i2c_i2cmr;
> char res6[0x8b];
> } i2c8xx_t;
The I2C interface for the MPC5200 is not compatible with the one for the
MPC83/4/5/6xx, AFAIK.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 17:06 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi
In-Reply-To: <fa686aa40807310951l48e2af8ep11ef83480397669a@mail.gmail.com>
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 */
};
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 16:51 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi
In-Reply-To: <4891A744.6060005@grandegger.com>
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.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Lost time?
From: Scott Wood @ 2008-07-31 16:49 UTC (permalink / raw)
To: - Reyneke; +Cc: linuxppc-embedded
In-Reply-To: <BAY101-W540C823A709A6C5F2E815FBE7C0@phx.gbl>
- Reyneke wrote:
> Anyone have any ideas, obvious suspects, or pointers where to go and look?
Check the kernel's idea of what the timebase frequency is on both kernels.
-Scott
^ permalink raw reply
* [2.6 patch] Add cuImage.mpc866ads to the bootwrapper as a cuboot-8xx target
From: Adrian Bunk @ 2008-07-31 16:10 UTC (permalink / raw)
To: paulus, benh; +Cc: Scott Wood, linuxppc-dev
From: Scott Wood <scottwood@freescale.com>
This patch fixes the following build error with mpc866_ads_defconfig:
<-- snip -->
...
WRAP arch/powerpc/boot/cuImage.mpc866ads
powerpc64-linux-ld: arch/powerpc/boot/cuboot-mpc866ads.o: No such file: No such file or directory
make[2]: *** [arch/powerpc/boot/cuImage.mpc866ads] Error 1
<-- snip -->
Reported-by: Adrian Bunk <bunk@kernel.org>
Signed-off-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Adrian Bunk <bunk@kernel.org>
---
This patch was sent by Scott Wood on:
- 21 May 2008
arch/powerpc/boot/wrapper | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index d6c96d9..3b59e33 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -159,7 +159,7 @@ cuboot*)
binary=y
gzip=
case "$platform" in
- *-mpc885ads|*-adder875*|*-ep88xc)
+ *-mpc866ads|*-mpc885ads|*-adder875*|*-ep88xc)
platformo=$object/cuboot-8xx.o
;;
*5200*|*-motionpro)
^ permalink raw reply related
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 15:55 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <9e4733910807310849g7e5612dbk9536733e061af8ad@mail.gmail.com>
Jon Smirl wrote:
> Aren't the tables in the manual there just to make it easy for a human
> to pick out the line they want? For a computer you'd program the
> formula that was used to create the tables.
Actually, the tables in the manuals are just an example of what can be
programmed. They don't cover all cases. The tables assume a specific value of
DFSR.
For 8xxx, you want to use AN2919 to figure out how to really program the device.
The table I generated for U-Boot is based on the calculations outlined in
AN2919. I debated trying to implement the actual algorithm, but decided that
pre-calculating the values was better. The algorithm is very convoluted.
> I agree that it took me half an hour to figure out the formula that
> was needed to compute the i2s clocks, but once you figure out the
> formula it solves all of the cases and no one needs to read the manual
> any more. The i2c formula may even need a small loop which compares
> different solutions looking for the smallest error term. But it's a
> small space to search.
My understanding is that the algorithm itself is different on 8xxx vs. 52xx. It
might be possible to combine 5200A and 5200B into one table/algorithm, but I
don't think you can combine it with the 8xxx table.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 15:49 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Timur Tabi, Linux I2C, Linuxppc-dev
In-Reply-To: <4891A744.6060005@grandegger.com>
On 7/31/08, 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).
Aren't the tables in the manual there just to make it easy for a human
to pick out the line they want? For a computer you'd program the
formula that was used to create the tables.
I agree that it took me half an hour to figure out the formula that
was needed to compute the i2s clocks, but once you figure out the
formula it solves all of the cases and no one needs to read the manual
any more. The i2c formula may even need a small loop which compares
different solutions looking for the smallest error term. But it's a
small space to search.
These device tree flags should be removed, the driver can ask the
platform code what CPU it is running on.
if (of_get_property(op->node, "dfsrr", NULL))
i2c->flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
if (of_device_is_compatible(op->node, "fsl,mpc5200-i2c") ||
of_device_is_compatible(op->node, "mpc5200-i2c"))
i2c->flags |= FSL_I2C_DEV_CLOCK_5200;
static void mpc_i2c_setclock(struct mpc_i2c *i2c)
{
/* Set clock and filters */
if (i2c->flags & FSL_I2C_DEV_SEPARATE_DFSRR) {
writeb(0x31, i2c->base + MPC_I2C_FDR);
writeb(0x10, i2c->base + MPC_I2C_DFSRR);
} else if (i2c->flags & FSL_I2C_DEV_CLOCK_5200)
writeb(0x3f, i2c->base + MPC_I2C_FDR);
else
writel(0x1031, i2c->base + MPC_I2C_FDR);
}
These defines shouldn't be here, they should get the offset from the
right header file for the CPU. But it appears that structures for the
i2c memory map haven't been done for the various CPUs.
#define MPC_I2C_FDR 0x04
#define MPC_I2C_CR 0x08
#define MPC_I2C_SR 0x0c
#define MPC_I2C_DR 0x10
#define MPC_I2C_DFSRR 0x14
There appears to be one for i2x8xxx but not the other CPUs.
/* I2C
*/
typedef struct i2c {
u_char i2c_i2mod;
char res1[3];
u_char i2c_i2add;
char res2[3];
u_char i2c_i2brg;
char res3[3];
u_char i2c_i2com;
char res4[3];
u_char i2c_i2cer;
char res5[3];
u_char i2c_i2cmr;
char res6[0x8b];
} i2c8xx_t;
> - 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.
>
> Wolfgang.
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] dtc: give advance warning that "-S" is going away.
From: Jon Loeliger @ 2008-07-31 15:43 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: linuxppc-dev, devicetree-discuss
In-Reply-To: <1217426356-1248-1-git-send-email-paul.gortmaker@windriver.com>
> The "-S" option allowed the specification of a minimum size for
> the blob, however the main reason for caring about the size is
> so there is enough padding to add a chosen node by u-boot or
> whoever. In which case, folks don't really care about the absolute
> size, but rather the size of the padding added for this -- which
> is what the "-p" option does. Having the "-S" just confuses people.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Applied.
jdl
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox