Re: Question on mpc52xx_common.c

Re: Question on mpc52xx_common.c

[Grant Likely]

(cc'ing the mailing list in my reply)

On Thu, Apr 3, 2008 at 10:27 AM, Detlev Zundel <dzu@denx.de> wrote:
> I wrote,
>
>  > debugging a lite5200b kernel (linux-2.6-denx equivalent of 2.6.25-rc8) I
>  > noticed that in mpc52xx_map_common_devices (mpc52xx_common.c:161) the
>  > cdm module is tentatively mapped.  It seems strange to me that there is no
>  > error checking here.  Is a FDT without an cdm entry usable at all?

Sure, why not?  If the firmware has already set it up correctly and no
devices using it are in use, then the kernel should be okay.  :-)
That said, I can't imagine choosing to not put the cdm node into the
device tree.

>  > Checking for the usage of mpc52xx_cdm I found mpc52xx_set_psc_clkdiv
>  > (mpc52xx_common.c:186) only returning ENODEV when no CDM module is
>  > mapped - but still not even a warning.  Now the only user of
>  > set_psc_clkdiv seems to be drivers/spi/mpc52xx_psc_spi.c:342 where the
>  > return value is not even checked....
>  >
>  > Wouldn't at least a warning on a failed mapping be in order here?

By rights, the psc_spi driver should be fixed to report an error.
Other than that, there are no users of the CDM in-tree right now.

>
>  And yes, I've seen that the lite5200 specific code warns on a missing
>  cdm node but thats no help for other platforms....

And that's because older lite5200 firmware doesn't setup the CDM correctly.

Now, all of the mpc5200 common code needs some work I think.  There
are too many places where bestcomm or other code redundantly goes
looking for common device nodes.  I'd like to amalgamate a bunch of
that stuff to map them all at platform initialization time and provide
an api for retrieving the mapped region or for controlling the shared
device.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Question on mpc52xx_common.c

[Matt Sealey]

Grant Likely wrote:
> (cc'ing the mailing list in my reply)
> 
> On Thu, Apr 3, 2008 at 10:27 AM, Detlev Zundel <dzu@denx.de> wrote:
>> I wrote,
>>
>>  > debugging a lite5200b kernel (linux-2.6-denx equivalent of 2.6.25-rc8) I
>>  > noticed that in mpc52xx_map_common_devices (mpc52xx_common.c:161) the
>>  > cdm module is tentatively mapped.  It seems strange to me that there is no
>>  > error checking here.  Is a FDT without an cdm entry usable at all?
> 
> Sure, why not?  If the firmware has already set it up correctly and no
> devices using it are in use, then the kernel should be okay.  :-)
> That said, I can't imagine choosing to not put the cdm node into the
> device tree.

*ahem* Efika.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: Question on mpc52xx_common.c

[Arnd Bergmann]

On Tuesday 08 April 2008, Matt Sealey wrote:
> Grant Likely wrote:
> >=20
> > Sure, why not? =A0If the firmware has already set it up correctly and no
> > devices using it are in use, then the kernel should be okay. =A0:-)
> > That said, I can't imagine choosing to not put the cdm node into the
> > device tree.
>=20
> *ahem* Efika.

Maybe we should just give up on making the efika boot with its regular
device tree and instead add a boot wrapper that either fixes up the
data provided by its firmware or just adds a proper dt blob?

	Arnd <><

Re: Question on mpc52xx_common.c

[Grant Likely]

On Mon, Apr 7, 2008 at 8:14 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2008, Matt Sealey wrote:
>
> > Grant Likely wrote:
>  > >
>  > > Sure, why not?  If the firmware has already set it up correctly and no
>  > > devices using it are in use, then the kernel should be okay.  :-)
>  > > That said, I can't imagine choosing to not put the cdm node into the
>  > > device tree.
>  >
>  > *ahem* Efika.
>
>  Maybe we should just give up on making the efika boot with its regular
>  device tree and instead add a boot wrapper that either fixes up the
>  data provided by its firmware or just adds a proper dt blob?

Current kernels boot the Efika without any firmware scripts.
prom_init.c is able to handle the few fixups that the kernel really
wants to see.  (So life is mostly happy in Efika land now.  :-)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Question on mpc52xx_common.c

[Grant Likely]

On Tue, Apr 8, 2008 at 3:37 AM, Matt Sealey <matt@genesi-usa.com> wrote:
> I'd not thank Grant.
>
>  I think the prom_init fixes are bordering on disgusting.. it would
>  make it's way into commercial code for sure, but only because nobody
>  would see what a hideous mess it is :)
>
>  The best solution by far is to release a new firmware with the
>  device tree fixed. The script method is just not tolerated by users,
>  and patching the Linux kernel to keep track with broken firmwares
>  is exactly what we hoped to AVOID with Aura in the first place.

It may be ideal, but I don't think it is realistic.  I'm now of the
firm opinion that device trees and firmware are *never* perfect.
Especially when the definition of perfect is a moving target.

There are certainly a number of things that are wrong and missing in
the Efika device tree, but in the long run it is proof that the design
of OF and the device tree is good.  The tree is unique.  Linux and
other OSes can derive the information they need.  Current Linux
drivers want data in a way slightly different from the way the Efika
offers it; some of that is Efika's fault, some of that is the driver's
fault, but OF provides the ability to massage the data and ensure the
board will boot.

As of right now; Linux support for the Efika contains only 4 distinct fixups:
1. Change device_type property of the / node from "chrp" to "efika".
Linux will run the wrong initialization code if this is "chrp"
2. change the format of the bestcomm interrupts property.  The Linux
drivers wants a list of interrupts and kind of treats the bestcomm
engine as it's own interrupt controller.  I think this was probably a
design flaw on the Linux driver, but it is what we currently have.
3. the /builtin/sound node is missing an interrupts property
4. The FEC driver wants MDIO and PHY nodes to talk to the Ethernet
Phy.  This one is big, but it is really just a single fixup.

All the other things that many not be what we *like* in the device
tree are really not serious flaws.  Mostly compatible properties are
missing any mfg prefix like 'fsl,'.  Of course, as Segher points out,
'fsl,' is better, but doesn't match recommended practice either
because UPPERCASE is supposed to be used with the prefix is the stock
ticker symbol.  See?  Device tree bindings are never perfect.  :-)
Regardless, the drivers deal with it and Linux is happy.

Cheers,
g.

>
>
>  --
>  Matt Sealey <matt@genesi-usa.com>
>  Genesi, Manager, Developer Relations
>
>  Raquel and Bill wrote:
>
> > Thanks Grant.
> >
> > R&B
> >
> >
> > On Mon, Apr 7, 2008 at 9:25 PM, Grant Likely <grant.likely@secretlab.ca
> <mailto:grant.likely@secretlab.ca>> wrote:
> >
> >    On Mon, Apr 7, 2008 at 8:14 PM, Arnd Bergmann <arnd@arndb.de
> >
> >    <mailto:arnd@arndb.de>> wrote:
> >     > On Tuesday 08 April 2008, Matt Sealey wrote:
> >     >
> >     > > Grant Likely wrote:
> >     >  > >
> >     >  > > Sure, why not?  If the firmware has already set it up
> >    correctly and no
> >     >  > > devices using it are in use, then the kernel should be okay.
> >     :-)
> >     >  > > That said, I can't imagine choosing to not put the cdm node
> >    into the
> >     >  > > device tree.
> >     >  >
> >     >  > *ahem* Efika.
> >     >
> >     >  Maybe we should just give up on making the efika boot with its
> >    regular
> >     >  device tree and instead add a boot wrapper that either fixes up the
> >     >  data provided by its firmware or just adds a proper dt blob?
> >
> >    Current kernels boot the Efika without any firmware scripts.
> >    prom_init.c is able to handle the few fixups that the kernel really
> >    wants to see.  (So life is mostly happy in Efika land now.  :-)
> >
> >    Cheers,
> >    g.
> >
> >    --
> >    Grant Likely, B.Sc., P.Eng.
> >    Secret Lab Technologies Ltd.
> >    _______________________________________________
> >    Linuxppc-dev mailing list
> >    Linuxppc-dev@ozlabs.org <mailto:Linuxppc-dev@ozlabs.org>
> >
> >    https://ozlabs.org/mailman/listinfo/linuxppc-dev
> >
> >
> >
> >
> > --
> > http://bbrv.blogspot.com/
> >
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Question on mpc52xx_common.c

[Robert Schwebel]

On Tue, Apr 08, 2008 at 08:52:55AM -0600, Grant Likely wrote:
> It may be ideal, but I don't think it is realistic.  I'm now of the
> firm opinion that device trees and firmware are *never* perfect.
> Especially when the definition of perfect is a moving target.

Well observed; isn't this the prove of the assumption that the whole
device tree idea is not working? It is *always* inconsistent and it is
*maintenance hell* because out-of-tree ports do *always* breakt because
of string inconsistencies. We have just ported a 8260 board from 2.6.22
to 2.6.25 and it is almost 100% oftree porting. And you do not even have
a single point of a parser, because all this string parsing is
completely scattered all over the tree.

The ARM method of using just a device number is so much easier ...

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: Question on mpc52xx_common.c

[Scott Wood]

Robert Schwebel wrote:
> Well observed; isn't this the prove of the assumption that the whole
> device tree idea is not working? It is *always* inconsistent and it is
> *maintenance hell* because out-of-tree ports do *always* breakt because
> of string inconsistencies. We have just ported a 8260 board from 2.6.22
> to 2.6.25 and it is almost 100% oftree porting.

There's going to be more churn in the initial stages than down the road. 
  82xx had barely been added to arch/powerpc in 2.6.22, and there was 
little review of the initial device tree bindings.

> The ARM method of using just a device number is so much easier ...

Yeah, it's so much fun to have to allocate a globally unique number for 
every minor tweak of a board, and to have to maintain a mapping from 
said numbers to information that is semantically equivalent to a device 
tree but in less maintainable form in the kernel source.

-Scott

Re: Question on mpc52xx_common.c

[David Gibson]

On Tue, Apr 08, 2008 at 03:07:58PM -0500, Scott Wood wrote:
> Robert Schwebel wrote:
>> Well observed; isn't this the prove of the assumption that the whole
>> device tree idea is not working? It is *always* inconsistent and it is
>> *maintenance hell* because out-of-tree ports do *always* breakt because
>> of string inconsistencies. We have just ported a 8260 board from 2.6.22
>> to 2.6.25 and it is almost 100% oftree porting.
>
> There's going to be more churn in the initial stages than down the road.  
> 82xx had barely been added to arch/powerpc in 2.6.22, and there was little 
> review of the initial device tree bindings.
>
>> The ARM method of using just a device number is so much easier ...
>
> Yeah, it's so much fun to have to allocate a globally unique number for 
> every minor tweak of a board, and to have to maintain a mapping from said 
> numbers to information that is semantically equivalent to a device tree but 
> in less maintainable form in the kernel source.

And we can already do device numbers if we really want.  A bootwrapper
that identifies the board and supplies a device tree essentially does
that, and that way the device tree is maintained in sync with the
kernel.

This is why I've always had mixed feelings about merging device trees
into u-boot, rather than having them supplied by the wrapper.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Question on mpc52xx_common.c

[Wolfgang Denk]

In message <20080408235150.GA8092@localhost.localdomain> you wrote:
>
> This is why I've always had mixed feelings about merging device trees
> into u-boot, rather than having them supplied by the wrapper.

On the other hand, we can now use  the  device  tree  to  dynamically
configure  U-Boot,  thus  allowing  to  run  the same binary image of
U-Boot on several different hardware configurations.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Parkinson's Law:  Work expands to fill the time alloted it.

Re: Question on mpc52xx_common.c

[Timur Tabi]

Robert Schwebel wrote:

> The ARM method of using just a device number is so much easier ...

And I was going to suggest that the ARM guys should use device trees, too.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: Question on mpc52xx_common.c

[Grant Likely]

On Tue, Apr 8, 2008 at 1:45 PM, Robert Schwebel
<r.schwebel@pengutronix.de> wrote:
> On Tue, Apr 08, 2008 at 08:52:55AM -0600, Grant Likely wrote:
>  > It may be ideal, but I don't think it is realistic.  I'm now of the
>  > firm opinion that device trees and firmware are *never* perfect.
>  > Especially when the definition of perfect is a moving target.
>
>  Well observed; isn't this the prove of the assumption that the whole
>  device tree idea is not working? It is *always* inconsistent and it is
>  *maintenance hell* because out-of-tree ports do *always* breakt because
>  of string inconsistencies. We have just ported a 8260 board from 2.6.22
>  to 2.6.25 and it is almost 100% oftree porting. And you do not even have
>  a single point of a parser, because all this string parsing is
>  completely scattered all over the tree.

I disagree and that is not my point.  My point is that perfection is
neither obtainable or necessary.  Many of the recently established
embedded guidelines are not "perfect" because they are counter to a
few of the OF recommended practices.  However, they are consistent
within themselves, they work, and once established they are not going
to change.  imperfect != bad.  I brought up a new 5200 board this week
which required zero kernel changes.  All I needed was a new dt to
describe the board.

Now, if out-of-tree ports continue to break then we've got a problem
that needs to be fixed.  Once a binding is established (which usually
takes a few kernel releases) it should be very stable and even if the
definition of ideal is changed, backwards compatibility must be
maintained.

>  The ARM method of using just a device number is so much easier ...

Only if the assumption is made that very little data needs to be
shared between the kernel and the firmware.  The moment you try to do
something more complex you either have the nightmare of bd_info or you
use a structured data format (like the device tree)

On another node, there are platforms where a device number is
unworkable.  For example, for Linux on an FPGA like the Xilinx Virtex,
there would need to be a new platform number every time the FPGA
bitstream was updated because it is effectively an entirely different
platform.

Finally, using a device number means you need to encode into the
kernel the exact layout of every platform it supports.  That adds up
to a lot of code in a real hurry; even if most of it is just
boilerplate instantiations.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Question on mpc52xx_common.c

[Segher Boessenkool]

> I disagree and that is not my point.  My point is that perfection is
> neither obtainable or necessary.

It's a nice goal though.

> Many of the recently established
> embedded guidelines are not "perfect" because they are counter to a
> few of the OF recommended practices.  However, they are consistent
> within themselves, they work, and once established they are not going
> to change.

Yeah.  Which is good, even if the bindings themselves aren't
so good.

> Now, if out-of-tree ports continue to break then we've got a problem
> that needs to be fixed.  Once a binding is established (which usually
> takes a few kernel releases)

This is a big problem that is totally avoidable.

*Before* accepting any patches that use a new binding (including
patches to .dts files), that binding itself needs to be discussed.
This will be a lot less work than trying to see what's going on in
some platform/device code and/or some example device tree, and
spotting mistakes there.  It might be a little more work upfront,
and it might seem like it would increase lead time, but as usual,
it's worth it.

Let's flat out refuse any patch series that uses a non-documented
binding.

> it should be very stable and even if the
> definition of ideal is changed, backwards compatibility must be
> maintained.

Which is a good argument why getting it right the first time is
so important (as with all interfaces, nothing specific about
device trees here).

>>  The ARM method of using just a device number is so much easier ...
>
> Only if the assumption is made that very little data needs to be
> shared between the kernel and the firmware.

...which means the kernel has to know *everything* about the hardware
setup and/or what settings the firmware established; i.e., the kernel
has to 100% replace the firmware.  This can be nice for some use cases
(it's the route LinuxBIOS took originally), but it simply doesn't
scale, not in any dimension.


Segher

Re: Question on mpc52xx_common.c

[Matt Sealey]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Tue, Apr 8, 2008 at 10:51 PM, Segher Boessenkool <
segher@kernel.crashing.org> wrote:

>
> Let's flat out refuse any patch series that uses a non-documented
> binding.
>

I would love this, it makes life easier, but let's make sure that the
documented binding is actually
fairly future proof, first, and not going to cause a billion problems
further down the line when kernels
and device trees diverge in their implementation/requirements as the
MPC5200B ethernet driver
has.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

[-- Attachment #2: Type: text/html, Size: 883 bytes --]

Re: Question on mpc52xx_common.c

[Robert Schwebel]

Hi Grant,

On Tue, Apr 08, 2008 at 03:26:11PM -0600, Grant Likely wrote:
> I disagree and that is not my point.

Well, having something like a device tree that describes the hardware is
definitely a good thing, in general.

> Now, if out-of-tree ports continue to break then we've got a problem
> that needs to be fixed.

Right, that's the point. Don't get me wrong, I'm deeply against
maintaining large code bases out-of-tree. But it shows in this case that
we have a maintenance problem with the current oftree concept.

When a simple BSP that supports almost nothing than CPU, mem, serial
console and flash needs > 1 week of hacking from an experienced kernel
hacker, just in order to get all the string changes right, something is
wrong.

> Once a binding is established (which usually takes a few kernel
> releases) it should be very stable and even if the definition of ideal
> is changed, backwards compatibility must be maintained.

That's theory, but in practise we see it changing every second day.

> >  The ARM method of using just a device number is so much easier ...
>
> Only if the assumption is made that very little data needs to be
> shared between the kernel and the firmware. The moment you try to do
> something more complex you either have the nightmare of bd_info or you
> use a structured data format (like the device tree)

I agree that the usual ARM hardwares are much less complicated and
configurable than what's available elsewhere. We usually need the
information "this is board FOO_BAR", maybe, if it is a module, on a
"BAZ" baseboard, and this is everything we need to register the platform
devices in some arch/arm/mach-*/my_cpu.c and arch/arm/mach-*/my_board.c
file.

> On another node, there are platforms where a device number is
> unworkable. For example, for Linux on an FPGA like the Xilinx Virtex,
> there would need to be a new platform number every time the FPGA
> bitstream was updated because it is effectively an entirely different
> platform.

Well, we have done FPGA based boards with ARM in the past, and we've
just added hardware auto-detection to the IP cores.

> Finally, using a device number means you need to encode into the
> kernel the exact layout of every platform it supports.  That adds up
> to a lot of code in a real hurry; even if most of it is just
> boilerplate instantiations.

You are right in general. However, it doesn't change the fact that we
are living in maintenance-nightmare land right now ...

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: Question on mpc52xx_common.c

[Sven Luther]

On Tue, Apr 08, 2008 at 04:14:42AM +0200, Arnd Bergmann wrote:
> On Tuesday 08 April 2008, Matt Sealey wrote:
> > Grant Likely wrote:
> > > 
> > > Sure, why not?  If the firmware has already set it up correctly and no
> > > devices using it are in use, then the kernel should be okay.  :-)
> > > That said, I can't imagine choosing to not put the cdm node into the
> > > device tree.
> > 
> > *ahem* Efika.
> 
> Maybe we should just give up on making the efika boot with its regular
> device tree and instead add a boot wrapper that either fixes up the
> data provided by its firmware or just adds a proper dt blob?

for 2.6.24, device-tree fixups where added to the linux kernel, since we
don't have a possibility to modify the efika dt, or at least not in a
foreseable future, we should just add more fixups so we can handle
various other problems in it.

Friendly,

Sven Luther