* Re: Board level compatibility matching
From: Grant Likely @ 2008-07-31 21:09 UTC (permalink / raw)
To: Scott Wood; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <20080731205906.GB17718@ld0162-tx32.am.freescale.net>
On Thu, Jul 31, 2008 at 03:59:06PM -0500, Scott Wood wrote:
> On Thu, Jul 31, 2008 at 02:19:57PM -0600, Grant Likely wrote:
> > - Add a property to the device tree that explicitly specifies the SoC
> > that the board is based on. Something like 'soc-model =
> > "fsl,mpc5200b"' would be appropriate.
>
> Shouldn't that go in the compatible property of the soc node?
Not all SoC's (in particular 4xx SoCs) use an soc node. Some of them
instead just describe the internal busses on the SoC.
> > - Prioritize board ports in the arch/powerpc/platforms directory to
> > identify level-1 machines support from the level-2 ones. Make sure
> > that level-1 stuff always gets probed before level-2 stuff within each
> > SoC family. In all likelyhood, this would probably just involve
> > making sure that board specific machines get linked in before the
> > catchall machine.
>
> I don't think we're too far away from being able to have a catch-all that
> isn't even soc-type-specific -- the main things that come to mind are
> instantiating PCI controllers (should be easily fixed) and finding the
> root IRQ controller (I suppose we could pick a random interrupt
> controller and follow the interrupt tree to its root, though it'd be more
> robust if it were explicitly identified in the device tree).
Perhaps a root-interrupt-controller property?
g.
^ permalink raw reply
* Re: libata badness
From: Anton Vorontsov @ 2008-07-31 21:04 UTC (permalink / raw)
To: Kumar Gala; +Cc: linux-ide, Jeff Garzik, Linux Kernel list, linuxppc-dev list
In-Reply-To: <95646AC3-5385-4F59-8EBA-20F811168A87@kernel.crashing.org>
On Thu, Jul 31, 2008 at 01:53:45PM -0500, Kumar Gala wrote:
> I'm getting the following badness with top of tree on a embedded PowerPC
> w/a ULI 1575 bridge (M5229 IDE):
>
> 02:1f.0 IDE interface: Acer Laboratories Inc. [ALi] M5229 IDE (rev c8)
Just to be sure that I didn't break the ULi for your board, could
you try locally revert this patch:
commit 52ddd1cdc923802b224b15ba75000c6c5668227f
Author: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Thu Jun 12 03:04:17 2008 +0400
powerpc/85xx/86xx: some refactoring for fsl_uli1575 code
And see if this helps?
> If you need more info let me know.
>
> - k
>
> ahci 0000:02:1f.1: AHCI 0001.0000 32 slots 4 ports 3 Gbps 0xf impl SATA
> mode
> ahci 0000:02:1f.1: flags: ncq sntf ilck pm led clo pmp pio slum part
> ------------[ cut here ]------------
> Badness at c021e700 [verbose debug info unavailable]
> NIP: c021e700 LR: c021e6e8 CTR: c022ce90
> REGS: ef82bca0 TRAP: 0700 Not tainted (2.6.27-rc1-00495-g33bd9fe-
> dirty)
> MSR: 00029032 <EE,ME,IR,DR> CR: 22044022 XER: 20000000
> TASK = ef830000[1] 'swapper' THREAD: ef82a000 CPU: 1
> GPR00: 00000001 ef82bd50 ef830000 00000000 00009032 00000000 ef0b64fc
> 00000000
> GPR08: ef937c10 c022f93f efbfc8e0 ef900b60 22044028 fffdd3ff 0ffe8700
> c044c17c
> GPR16: c04d52ec ef82f458 ef82f4f4 ef82f4f4 c044c234 c044c5d8 c044c5d8
> c044c220
> GPR24: c044c218 c04d52f0 00000080 c022f940 00000000 c044c244 efbec490
> 00000000
> NIP [c021e700] ata_host_activate+0x40/0x10c
> LR [c021e6e8] ata_host_activate+0x28/0x10c
> Call Trace:
> [ef82bd50] [c021e6e8] ata_host_activate+0x28/0x10c (unreliable)
> [ef82bd80] [c022f48c] ahci_init_one+0x8b4/0xd68
> [ef82be30] [c01b69c0] pci_device_probe+0x84/0xa8
> [ef82be50] [c01e5438] driver_probe_device+0xb4/0x1e8
> [ef82be70] [c01e55f0] __driver_attach+0x84/0x88
> [ef82be90] [c01e4ba0] bus_for_each_dev+0x5c/0xa4
> [ef82bec0] [c01e5254] driver_attach+0x24/0x34
> [ef82bed0] [c01e44b8] bus_add_driver+0x1d8/0x24c
> [ef82bef0] [c01e5810] driver_register+0x70/0x160
> [ef82bf10] [c01b6c54] __pci_register_driver+0x64/0xc4
> [ef82bf30] [c04aaa60] ahci_init+0x28/0x38
> [ef82bf40] [c048717c] do_one_initcall+0x38/0x1ac
> [ef82bfb0] [c04874e0] kernel_init+0x1f0/0x1fc
> [ef82bff0] [c0013b04] kernel_thread+0x44/0x60
> Instruction dump:
> 7cbb2b78 90010034 7cda3378 7cf93b78 7c7e1b78 4bff9dbd 7c7f1b79 408200c8
> 2f9c0000 409e002c 313bffff 7c09d910 <0f000000> 80010034 7fc3f378
> 7f24cb78
> scsi0 : ahci
> scsi1 : ahci
> scsi2 : ahci
> scsi3 : ahci
> ata1: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006100
> ata2: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006180
> ata3: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006200
> ata4: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006280
> ata1: SATA link down (SStatus 0 SControl 300)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 21:03 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48922715.2000304@freescale.com>
On 7/31/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
> > Isn't there a single global divider that generates all the i2c source
> > clocks? You don't want to copy a global value into each i2c node.
>
>
> Why not? There are only two I2C devices, and it's theoretically possible for
> them to have different input clock frequencies. Keeping it in the I2C node
> allows the I2C driver to reference a property directly in the node that its probing.
But that's the same as saying we should copy the system clock
frequency into all of the PSC nodes because we might implement
hardware where they aren't all clocked off from the same input clock
source.
> > Aren't we talking about the /2 or /3 or /1 divider that appears to be
> > randomly implemented on various members of the mpc8xxx family?
I don't this these dividers or clocks need to be exposed at all if
you'd just put that ugly code snippet into your platform driver.
>
>
> Yes.
>
>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Board level compatibility matching
From: Scott Wood @ 2008-07-31 20:59 UTC (permalink / raw)
To: Grant Likely; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <fa686aa40807311319k44d270b0t4fa9cb0e6523339@mail.gmail.com>
On Thu, Jul 31, 2008 at 02:19:57PM -0600, Grant Likely wrote:
> - Add a property to the device tree that explicitly specifies the SoC
> that the board is based on. Something like 'soc-model =
> "fsl,mpc5200b"' would be appropriate.
Shouldn't that go in the compatible property of the soc node?
> - Prioritize board ports in the arch/powerpc/platforms directory to
> identify level-1 machines support from the level-2 ones. Make sure
> that level-1 stuff always gets probed before level-2 stuff within each
> SoC family. In all likelyhood, this would probably just involve
> making sure that board specific machines get linked in before the
> catchall machine.
I don't think we're too far away from being able to have a catch-all that
isn't even soc-type-specific -- the main things that come to mind are
instantiating PCI controllers (should be easily fixed) and finding the
root IRQ controller (I suppose we could pick a random interrupt
controller and follow the interrupt tree to its root, though it'd be more
robust if it were explicitly identified in the device tree).
-Scott
^ permalink raw reply
* Re: Board level compatibility matching
From: Jon Smirl @ 2008-07-31 20:58 UTC (permalink / raw)
To: Grant Likely; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <20080731205251.GB29834@secretlab.ca>
On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Jul 31, 2008 at 04:49:49PM -0400, Jon Smirl wrote:
> > On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > This topic keeps coming up, so it is probably time to address it once
> > > and for all.
> > >
> > > When it comes to machine level support in arch/powerpc, there seems to
> > > me that there are two levels or machine support.
> > >
> > ......
> > >
> > > Thoughts?
> > > g.
> >
> >
> > As part of this, how can we going to solve the problem with triggering
> > the load of a board specific machine/fabric driver in a generic way?
>
>
> That really is a separate problem. We *could* do this with a board
> specific powerpc machine driver, but I don't think it is the best
> solution.
>
> I'm still thinking that the drivers module_init() function could check
> the top level board model property and decide whether or not to load
> based on that.
You're assuming the driver is compiled in.
If the drivers are on initrd selection has to happen via the normal
device/driver matching process. Search for a device in the alias table
of the drive file.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:56 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <9e4733910807311355q3394b4bfg66c37055384451f7@mail.gmail.com>
Jon Smirl wrote:
> Isn't there a single global divider that generates all the i2c source
> clocks? You don't want to copy a global value into each i2c node.
Why not? There are only two I2C devices, and it's theoretically possible for
them to have different input clock frequencies. Keeping it in the I2C node
allows the I2C driver to reference a property directly in the node that its probing.
> Aren't we talking about the /2 or /3 or /1 divider that appears to be
> randomly implemented on various members of the mpc8xxx family?
Yes.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Scott Wood @ 2008-07-31 20:56 UTC (permalink / raw)
To: Jon Smirl; +Cc: Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <9e4733910807311355q3394b4bfg66c37055384451f7@mail.gmail.com>
Jon Smirl wrote:
> Isn't there a single global divider that generates all the i2c source
> clocks? You don't want to copy a global value into each i2c node.
Why not? The "compatible" is pretty much always going to be the same
for the i2c node, but we copy that, as well.
Likewise for the clock-frequency of serial nodes.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 20:55 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <20080731204838.GA29834@secretlab.ca>
On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Jul 31, 2008 at 03:37:07PM -0500, Timur Tabi wrote:
> > Grant Likely wrote:
> >
> > > How is the divider controlled? Is it a fixed property of the SoC?
> >
> > Yes. The divider is either 1, 2, or 3, and the only way to know which one
> > it is is to look up the specific SOC model number. And depending on the
> > SOC model, there may also be a register that needs to be looked up.
> >
> > > a
> > > shared register setting? or a register setting within the i2c device?
> >
> > The I2C device itself has no idea what the divider is. It only sees the
> > result of the divider.
>
>
> Then that absolutely suggests to me that either the final clock or the
> divider should be encoded in the i2c node; not in the soc node.
Isn't there a single global divider that generates all the i2c source
clocks? You don't want to copy a global value into each i2c node.
Aren't we talking about the /2 or /3 or /1 divider that appears to be
randomly implemented on various members of the mpc8xxx family?
>
>
> g.
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Board level compatibility matching
From: Grant Likely @ 2008-07-31 20:52 UTC (permalink / raw)
To: Jon Smirl; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <9e4733910807311349l5770a17ib2ab2535bcd2868a@mail.gmail.com>
On Thu, Jul 31, 2008 at 04:49:49PM -0400, Jon Smirl wrote:
> On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> > This topic keeps coming up, so it is probably time to address it once
> > and for all.
> >
> > When it comes to machine level support in arch/powerpc, there seems to
> > me that there are two levels or machine support.
> >
> ......
> >
> > Thoughts?
> > g.
>
>
> As part of this, how can we going to solve the problem with triggering
> the load of a board specific machine/fabric driver in a generic way?
That really is a separate problem. We *could* do this with a board
specific powerpc machine driver, but I don't think it is the best
solution.
I'm still thinking that the drivers module_init() function could check
the top level board model property and decide whether or not to load
based on that.
g.
^ permalink raw reply
* Re: Board level compatibility matching
From: Jon Smirl @ 2008-07-31 20:49 UTC (permalink / raw)
To: Grant Likely; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <fa686aa40807311319k44d270b0t4fa9cb0e6523339@mail.gmail.com>
On 7/31/08, Grant Likely <grant.likely@secretlab.ca> wrote:
> This topic keeps coming up, so it is probably time to address it once
> and for all.
>
> When it comes to machine level support in arch/powerpc, there seems to
> me that there are two levels or machine support.
>
......
>
> Thoughts?
> g.
As part of this, how can we going to solve the problem with triggering
the load of a board specific machine/fabric driver in a generic way?
--
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 20:48 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <48922273.6070801@freescale.com>
On Thu, Jul 31, 2008 at 03:37:07PM -0500, Timur Tabi wrote:
> Grant Likely wrote:
>
> > How is the divider controlled? Is it a fixed property of the SoC?
>
> Yes. The divider is either 1, 2, or 3, and the only way to know which one
> it is is to look up the specific SOC model number. And depending on the
> SOC model, there may also be a register that needs to be looked up.
>
> > a
> > shared register setting? or a register setting within the i2c device?
>
> The I2C device itself has no idea what the divider is. It only sees the
> result of the divider.
Then that absolutely suggests to me that either the final clock or the
divider should be encoded in the i2c node; not in the soc node.
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:44 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <9e4733910807311343h3a1b21dbub15754090a67fac6@mail.gmail.com>
Jon Smirl wrote:
> It wouldn't go into the i2c driver, it would go into the mpc8xxx
> platform driver. Why is it bad to put it into the mpc8xxx platform
> driver? It is an accurate description of the mpc8xxx platform isn't
> it?
There's no need to put that code in the platform driver because U-Boot will put
the final result in the device tree! That way, we won't need to update the
platform driver *and* U-Boot for any new SOC.
--
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 20:43 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48922218.9020503@freescale.com>
On 7/31/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
> > For mpc5200 it is easy, mpc52xx_find_ipb_freq() already exists to get
> > the source clock for the i2c devices. Each i2c node in the device tree
> > can then specify "clock-frequency = 400000;" or let it default. You
>
>
> 400KHz is the *speed* of the I2C bus, so let's be sure to use "speed" in the
> property name. Reserve the word "bus" for the input clock to the I2C device.
>
>
> > #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;
>
>
> I think the whole point is to eliminate duplicating this code in the Linux
> driver. It's hideously ugly, so it should be limited as much as possible.
It wouldn't go into the i2c driver, it would go into the mpc8xxx
platform driver. Why is it bad to put it into the mpc8xxx platform
driver? It is an accurate description of the mpc8xxx platform isn't
it?
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Board level compatibility matching
From: Chris Friesen @ 2008-07-31 20:39 UTC (permalink / raw)
To: Grant Likely; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <fa686aa40807311319k44d270b0t4fa9cb0e6523339@mail.gmail.com>
Grant Likely wrote:
> Doing so should simplify adding new board ports. In many cases it
> would just involve dropping in a new .dts file. However, it retains
> the flexability of overriding generic code with platform specific
> fixups as the need arises.
If it makes it easier for board vendors to do a clean port, you get a
vote from me.
I've seen a large amount of vendor code from that needed a lot of
massaging before it would play nice with support for similar boards from
other vendors--not to mention how ugly it looked with all the
board-specific ifdefs.
Chris
^ permalink raw reply
* Re: libata badness
From: Scott Wood @ 2008-07-31 20:37 UTC (permalink / raw)
To: Kumar Gala
Cc: linux-ide, Jeff Garzik, Linux Kernel list, Ben Dooks,
linuxppc-dev list
In-Reply-To: <836C281D-4D3A-4547-98D9-143A690698AF@kernel.crashing.org>
On Thu, Jul 31, 2008 at 03:24:31PM -0500, Kumar Gala wrote:
> >it would be helpful to compile a kernel with verbose fault information
> >turned on.
>
> Doesn't seem to provide too much more info on ppc.
I'm pretty sure it does the same thing on ppc as on all the other
architectures.
> just says the file/ line number of the badness.
...which is a lot more useful than an address in someone else's kernel
image.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:37 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <fa686aa40807311335t1dd123d0mc385234ebe55248@mail.gmail.com>
Grant Likely wrote:
> How is the divider controlled? Is it a fixed property of the SoC?
Yes. The divider is either 1, 2, or 3, and the only way to know which one it is
is to look up the specific SOC model number. And depending on the SOC model,
there may also be a register that needs to be looked up.
> a
> shared register setting? or a register setting within the i2c device?
The I2C device itself has no idea what the divider is. It only sees the result
of the divider.
--
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 20:35 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <9e4733910807311332q611b43b3y26f64b5269ccb657@mail.gmail.com>
Jon Smirl wrote:
> For mpc5200 it is easy, mpc52xx_find_ipb_freq() already exists to get
> the source clock for the i2c devices. Each i2c node in the device tree
> can then specify "clock-frequency = 400000;" or let it default. You
400KHz is the *speed* of the I2C bus, so let's be sure to use "speed" in the
property name. Reserve the word "bus" for the input clock to the I2C device.
> #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;
I think the whole point is to eliminate duplicating this code in the Linux
driver. It's hideously ugly, so it should be limited as much as possible.
--
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 20:35 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <9e4733910807311332q611b43b3y26f64b5269ccb657@mail.gmail.com>
On Thu, Jul 31, 2008 at 2:32 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 7/31/08, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Timur Tabi wrote:
>>
>> > Wolfgang Grandegger wrote:
>> >
>> >
>> > > But clock-frequency, aka bus-frequency, is already used by
>> fsl_get_sys_freq():
>> > >
>> > >
>> http://lxr.linux.no/linux+v2.6.26/arch/powerpc/sysdev/fsl_soc.c#L80
>> > >
>> >
>> > So? clock-frequency is a per-node property. I use it in the codec node
>> on the
>> > 8610 (mpc8610_hpcd.dts). It does not mean "platform clock frequency".
>> >
>> >
>> > > U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
>> driver simply calls fsl_get_i2c_freq().
>> > >
>> >
>> > This is just more complicated than it needs to be. Why should the I2C
>> driver
>> > fetch the platform clock and the divider from the parent node, and then do
>> > additional math, when it could just get the value it needs right from the
>> node
>> > it's probing?
>> >
>>
>> I'm a bit confused. The frequency of the I2C source clock and the real I2C
>> clock frequency are two different things. The first one is common for all
>> I2C devices, the second can be different. What properties would you like to
>> use for defining both?
How is the divider controlled? Is it a fixed property of the SoC? a
shared register setting? or a register setting within the i2c device?
(My answer depends on the layout)
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 20:32 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Timur Tabi, Linux I2C, Linuxppc-dev
In-Reply-To: <48921DED.6010403@grandegger.com>
On 7/31/08, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Timur Tabi wrote:
>
> > Wolfgang Grandegger wrote:
> >
> >
> > > But clock-frequency, aka bus-frequency, is already used by
> fsl_get_sys_freq():
> > >
> > >
> http://lxr.linux.no/linux+v2.6.26/arch/powerpc/sysdev/fsl_soc.c#L80
> > >
> >
> > So? clock-frequency is a per-node property. I use it in the codec node
> on the
> > 8610 (mpc8610_hpcd.dts). It does not mean "platform clock frequency".
> >
> >
> > > U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
> driver simply calls fsl_get_i2c_freq().
> > >
> >
> > This is just more complicated than it needs to be. Why should the I2C
> driver
> > fetch the platform clock and the divider from the parent node, and then do
> > additional math, when it could just get the value it needs right from the
> node
> > it's probing?
> >
>
> I'm a bit confused. The frequency of the I2C source clock and the real I2C
> clock frequency are two different things. The first one is common for all
> I2C devices, the second can be different. What properties would you like to
> use for defining both?
For mpc5200 it is easy, mpc52xx_find_ipb_freq() already exists to get
the source clock for the i2c devices. Each i2c node in the device tree
can then specify "clock-frequency = 400000;" or let it default. You
have the input clock and the desired clock, do some math and you can
set FDR.
For the 8xxx chips there is no simple solution for obtain the input
clock for the i2c section. Maybe create something like i2c-frequency
and set it from uboot. The make another accessor function like
mpc8xxx_find_i2c_freq().
PowerPC,8xxx@0 {
timebase-frequency = <0>; /* From Bootloader */
bus-frequency = <0>; /* From Bootloader */
clock-frequency = <0>; /* From Bootloader */
i2c-frequency = <0>; /* From Bootloader */
};
Instead of creating i2c-frequency in the device tree, you could put
this piece of code in the the mpc8xxx platform driver and use it to
implement mpc8xxx_find_i2c_freq(). Read the PORDEVSR2_SEC_CFG bit back
from whatever uboot set it too.
#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;
--
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 20:30 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <48921E44.7010502@freescale.com>
On Thu, Jul 31, 2008 at 2:19 PM, Timur Tabi <timur@freescale.com> wrote:
> Wolfgang Grandegger wrote:
>
>> I'm a bit confused. The frequency of the I2C source clock and the real
>> I2C clock frequency are two different things.
>
> There are two frequencies:
>
> 1) The frequency of the input clock to the I2C device, after it has gone through
> a divider. This is what I call the "I2C clock frequency" and what I think
> belongs in the clock-frequency property. This is usually the platform clock
> divided by 1, 2, or 3.
>
> 2) The speed of the I2C bus, as seen by devices on that bus. This is usually
> 400KHz.
analogous example: serial nodes use two properties; "clock-frequency"
and "current-speed", one for clock frequency routed into the device
and one for the current baud rate. It is completely relevant to put
the effective clock frequency into the i2c device node.
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 20:28 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921E44.7010502@freescale.com>
Timur Tabi wrote:
> Wolfgang Grandegger wrote:
>
>> I'm a bit confused. The frequency of the I2C source clock and the real
>> I2C clock frequency are two different things.
>
> There are two frequencies:
>
> 1) The frequency of the input clock to the I2C device, after it has gone through
> a divider. This is what I call the "I2C clock frequency" and what I think
> belongs in the clock-frequency property. This is usually the platform clock
> divided by 1, 2, or 3.
OK.
> 2) The speed of the I2C bus, as seen by devices on that bus. This is usually
> 400KHz.
Which should be defined with the property "current-speed", right?
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:28 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48922087.4050903@grandegger.com>
Wolfgang Grandegger wrote:
>> 2) The speed of the I2C bus, as seen by devices on that bus. This is usually
>> 400KHz.
>
> Which should be defined with the property "current-speed", right?
I would use something like "bus-speed", but yes. The word "current" shouldn't
be in a property string.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: libata badness
From: Kumar Gala @ 2008-07-31 20:24 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-ide, Jeff Garzik, Linux Kernel list, linuxppc-dev list
In-Reply-To: <20080731190259.GJ26938@trinity.fluff.org>
On Jul 31, 2008, at 2:02 PM, Ben Dooks wrote:
> On Thu, Jul 31, 2008 at 01:53:45PM -0500, Kumar Gala wrote:
>> I'm getting the following badness with top of tree on a embedded
>> PowerPC w/a ULI 1575 bridge (M5229 IDE):
>>
>> 02:1f.0 IDE interface: Acer Laboratories Inc. [ALi] M5229 IDE (rev
>> c8)
>>
>> If you need more info let me know.
>>
>> - k
>>
>> ahci 0000:02:1f.1: AHCI 0001.0000 32 slots 4 ports 3 Gbps 0xf impl
>> SATA mode
>> ahci 0000:02:1f.1: flags: ncq sntf ilck pm led clo pmp pio slum part
>> ------------[ cut here ]------------
>> Badness at c021e700 [verbose debug info unavailable]
>
> it would be helpful to compile a kernel with verbose fault information
> turned on.
Doesn't seem to provide too much more info on ppc. just says the file/
line number of the badness.
>> NIP: c021e700 LR: c021e6e8 CTR: c022ce90
>> REGS: ef82bca0 TRAP: 0700 Not tainted (2.6.27-rc1-00495-g33bd9fe-
>> dirty)
>> MSR: 00029032 <EE,ME,IR,DR> CR: 22044022 XER: 20000000
>> TASK = ef830000[1] 'swapper' THREAD: ef82a000 CPU: 1
>> GPR00: 00000001 ef82bd50 ef830000 00000000 00009032 00000000 ef0b64fc
>> 00000000
>> GPR08: ef937c10 c022f93f efbfc8e0 ef900b60 22044028 fffdd3ff 0ffe8700
>> c044c17c
>> GPR16: c04d52ec ef82f458 ef82f4f4 ef82f4f4 c044c234 c044c5d8 c044c5d8
>> c044c220
>> GPR24: c044c218 c04d52f0 00000080 c022f940 00000000 c044c244 efbec490
>> 00000000
>> NIP [c021e700] ata_host_activate+0x40/0x10c
>> LR [c021e6e8] ata_host_activate+0x28/0x10c
>> Call Trace:
>> [ef82bd50] [c021e6e8] ata_host_activate+0x28/0x10c (unreliable)
>> [ef82bd80] [c022f48c] ahci_init_one+0x8b4/0xd68
>> [ef82be30] [c01b69c0] pci_device_probe+0x84/0xa8
>> [ef82be50] [c01e5438] driver_probe_device+0xb4/0x1e8
>> [ef82be70] [c01e55f0] __driver_attach+0x84/0x88
>> [ef82be90] [c01e4ba0] bus_for_each_dev+0x5c/0xa4
>> [ef82bec0] [c01e5254] driver_attach+0x24/0x34
>> [ef82bed0] [c01e44b8] bus_add_driver+0x1d8/0x24c
>> [ef82bef0] [c01e5810] driver_register+0x70/0x160
>> [ef82bf10] [c01b6c54] __pci_register_driver+0x64/0xc4
>> [ef82bf30] [c04aaa60] ahci_init+0x28/0x38
>> [ef82bf40] [c048717c] do_one_initcall+0x38/0x1ac
>> [ef82bfb0] [c04874e0] kernel_init+0x1f0/0x1fc
>> [ef82bff0] [c0013b04] kernel_thread+0x44/0x60
>> Instruction dump:
>> 7cbb2b78 90010034 7cda3378 7cf93b78 7c7e1b78 4bff9dbd 7c7f1b79
>> 408200c8
>> 2f9c0000 409e002c 313bffff 7c09d910 <0f000000> 80010034 7fc3f378
>> 7f24cb78
>> scsi0 : ahci
>> scsi1 : ahci
>> scsi2 : ahci
>> scsi3 : ahci
>> ata1: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006100
>> ata2: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006180
>> ata3: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006200
>> ata4: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006280
>> ata1: SATA link down (SStatus 0 SControl 300)
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: qc timeout (cmd 0xec)
>> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: qc timeout (cmd 0xec)
>> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata2.00: qc timeout (cmd 0xec)
>> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
>> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>> ata3: SATA link down (SStatus 0 SControl 300)
>> ata4: SATA link down (SStatus 0 SControl 300)
>> scsi4 : pata_ali
>> scsi5 : pata_ali
>> ata5: PATA max UDMA/133 cmd 0x1200 ctl 0x1208 bmdma 0x1220
>> ata6: PATA max UDMA/133 cmd 0x1210 ctl 0x1218 bmdma 0x1228
>
> Looks like you're running into the same problem as I did, with the
> fact that the M5529 is in native mode, but doesn't have any IRQ
> available. Do you know if the bridge chip in it is in routes the
> IRQs internally?
They are routed via an i8259 in the bridge.
- k
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 20:20 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <20080731195911.GA29610@secretlab.ca>
Grant Likely wrote:
> On Thu, Jul 31, 2008 at 09:54:48PM +0200, Wolfgang Grandegger wrote:
>> Thinking more about it, it would be best to add the property
>> "i2c-clock-divider" to the soc node and implement fsl_get_i2c_freq() in
>> a similar way:
>>
>> soc8541@e0000000 {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> device_type = "soc";
>> ranges = <0x0 0xe0000000 0x100000>;
>> reg = <0xe0000000 0x1000>; // CCSRBAR 1M
>> bus-frequency = <0>;
>> i2c-clock-divider = <2>;
>>
>> U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
>> driver simply calls fsl_get_i2c_freq().
>
> Ugh. This is specifically related to the i2c device, so please place
> the property in the i2c device. Remember, device tree design is not
> about what will make the implementation simplest, but rather about what
> describes the hardware in the best way.
>
> Now, if you can argue that i2c-clock-frequency is actually a separate
> clock domain defined at the SoC level, not the i2c device level, then I
> will change my opinion.
Is it not exactly that? For me it's not a per I2C device property, at least.
Wolfgang.
^ permalink raw reply
* Board level compatibility matching
From: Grant Likely @ 2008-07-31 20:19 UTC (permalink / raw)
To: Josh Boyer, linuxppc-dev, devicetree-discuss, Kumar Gala,
Benjamin Herrenschmidt, Segher Boessenkool
This topic keeps coming up, so it is probably time to address it once
and for all.
When it comes to machine level support in arch/powerpc, there seems to
me that there are two levels or machine support.
Level 1 is the board specific stuff. Board X has a, b, and c things
that need to be done for Linux to work correctly, but the fixups are
entirely board specific and will only ever be used on a single board
port. The lite5200 support in arch/powerpc/platforms/52xx is an
example of this kind of board support. It binds on a value in the top
level compatible property.
Level 2 is kind of the generic catch-all machine support for systems
that are unremarkable and don't require any special code to be run.
In most cases, new boards can be supported by this generic code
without any changes to the Linux kernel.
arch/powerpc/platforms/52xx/mpc5200_simple.c is an example here.
mpc5200_simple maintains a list of boards that are known to work with
it.
At the moment, every new board port forces a linux kernel source tree
change, even if it is just adding a single string to the match table.
I'm willing to wager that 99 times out of 100, boards based on the
mpc5200 SoC will want to use the common board support code and that
maintaining an explicit list of supported boards is completely
unnecessary. I expect that the exact same is true for 8xxx and 4xx
SoCs. So, rather than continuing to need to maintain explicit lists,
I propose the following:
- Add a property to the device tree that explicitly specifies the SoC
that the board is based on. Something like 'soc-model =
"fsl,mpc5200b"' would be appropriate. This in and of itself does not
change the usage conventions, it just provides more information about
the hardware. (Another idea is to add a string to the top level
compatible property, but there are still arguments about what
compatible really means in the root node.)
- Prioritize board ports in the arch/powerpc/platforms directory to
identify level-1 machines support from the level-2 ones. Make sure
that level-1 stuff always gets probed before level-2 stuff within each
SoC family. In all likelyhood, this would probably just involve
making sure that board specific machines get linked in before the
catchall machine.
- Change level-2 machine support to bind on soc-model instead of an
explicit compatible list.
Doing so should simplify adding new board ports. In many cases it
would just involve dropping in a new .dts file. However, it retains
the flexability of overriding generic code with platform specific
fixups as the need arises. I know we've been cautious about adding
catch-all bindings to the device tree, and it is a big deal to avoid
adding wildcards to compatible values. However, this solution should
be workable because it doesn't involve stating something that is not
true in the device tree and it maintains the ability to override
cleanly when new bugs are discovered. It also doesn't try to define
wildcard values in compatible or other device tree properties.
Thoughts?
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ 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