LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: Audio codec device tree entries
From: Grant Likely @ 2007-10-24 15:43 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <9e4733910710240828x412f598dy7fc4a75faa76358d@mail.gmail.com>

On 10/24/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 10/24/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> > > Two valid methods have been proposed
> > > 1. a codec-
> >
> > oops
> >
> > 1. a codec-handle property in the i2s node
> > 2. an i2s-handle property in the codec node
> >
> > Either are reasonable.  I prefer putting the handle in the i2s node;
> > but I'm looking at it from the way that ethernet phys are being
> > described currently.  The other is also perfectly valid.
> >
> > I suppose it depends on what point of view you see the system from; either:
> > a. the codec is supported by the i2s bus, in which case use the
> > i2s-handle property
> > b. the i2s bus is supported by the codec; in which case use the
> > codec-handle property.
>
> Do you want to pick one and add it to the device tree documentation
> with an example for i2s and ac97? I'll use which ever one is picked.

Sure, I'll draft something up and post it for review.

On the device probing front; what about this method:

Rather than trying to figure things out from the board model, or the
combination of the codec and i2s bus; add another node to represent
the sound circuit.  All that node would need is a unique compatible
property and a phandle to either the i2s bus or the codec (depending
on which binding approach is used).  It could have additional
properties to represent optional features, etc.

For example:
sound@0 {
      compatible = "<mfg>,<board>,sound"   // The board might have
more than one sound i/f which could be wired differently
      codec-handle = <&codec0>;
};

This would give your fabric driver something unique to probe on; but
the i2c, i2s and codec nodes which actually describe interconnects
will still be present.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: Audio codec device tree entries
From: Timur Tabi @ 2007-10-24 15:40 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710240823v2022661ftb1b754a86cab88f3@mail.gmail.com>

Jon Smirl wrote:

> I see your point about putting the child node onto the control bus.
> ac97 is both a control and data bus. For the i2s case the child should
> go onto the i2c bus.

I know AC97 is *also* a control bus, but treating I2S and AC97 differently is 
bad, IMHO.  If you're going to put the child node in the AC97 node, you should 
also put it in the I2S node.

The 8610 has an SSI that can operate as either AC97 or I2S.  If I want to switch 
from AC97 to I2S, I should not have to move the child node out of the AC97 node. 
  I should instead just add an I2C node and point to it.

^ permalink raw reply

* Re: Audio codec device tree entries
From: Grant Likely @ 2007-10-24 15:28 UTC (permalink / raw)
  To: Timur Tabi; +Cc: PowerPC dev list
In-Reply-To: <471F5FC4.1040804@freescale.com>

On 10/24/07, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
> > What I meant was that there is no attempt to describe how the codec is
> > connected to the external world. Those connections are described in
> > the fabric driver.
>
> Hmmm, I'm not sure I'm okay with that.  We can always add properties to those
> nodes if it's necessary.  However, now you're basically defining some parts of
> the board layout in the DTS, and some parts in the fabric driver.  On PowerPC
> platforms, the fabric driver is supposed to get board layout information from
> the device tree.

No, not always; if the description is too complex (like gpio's for
instance) then it is perfectly valid to build the knowledge into the
platform code.  It should be avoided; but the device tree doesn't need
to describe *everything*.

>
> > i2s@2200 {           // PSC2
> >      compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
> >      cell-index = <1>;
> >      reg = <2200 100>;
> >      interrupts = <2 2 0>;
> >      interrupt-parent = <&mpc5200_pic>;
> >      codec2: i2s-codec@2 {
>
> This should probably be codec0, since it's the first code on this I2S bus.
>
> >            compatible = "wolson,wm8750";
> >            reg = <2>;
> >            i2c-handle = <&i2c@3d00>;
> >      };
> > };
> >
> > i2c@3d00 {
> >      compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
> >      #address-cells = <1>;
> >      #size-cells = <0>;
> >      cell-index = <0>;
> >      reg = <3d00 40>;
> >      interrupts = <2 f 0>;
> >      interrupt-parent = <&mpc5200_pic>;
> >      fsl5200-clocking;
> > }
>
> My vote is for this version.  In fact, I think it *has* to be this way.  If
> you're using a CS4270 codec (as I am), the I2C interface is *optional*.  So I
> want the I2S node to point to the I2C node if it exists.

It doesn't have to be this way.  If the codec does not have a control
interface, then it can happily be a child of the i2s node.  But if it
*does*; don't break convention by separating it from it's control
interface.

I strongly recommend following the lead of ethernet phys and mdio busses here.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: Audio codec device tree entries
From: Jon Smirl @ 2007-10-24 15:28 UTC (permalink / raw)
  To: Grant Likely; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <fa686aa40710240820l62d5a50ewcd7707fed8f9176f@mail.gmail.com>

On 10/24/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> > Two valid methods have been proposed
> > 1. a codec-
>
> oops
>
> 1. a codec-handle property in the i2s node
> 2. an i2s-handle property in the codec node
>
> Either are reasonable.  I prefer putting the handle in the i2s node;
> but I'm looking at it from the way that ethernet phys are being
> described currently.  The other is also perfectly valid.
>
> I suppose it depends on what point of view you see the system from; either:
> a. the codec is supported by the i2s bus, in which case use the
> i2s-handle property
> b. the i2s bus is supported by the codec; in which case use the
> codec-handle property.

Do you want to pick one and add it to the device tree documentation
with an example for i2s and ac97? I'll use which ever one is picked.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Audio codec device tree entries
From: Jon Smirl @ 2007-10-24 15:23 UTC (permalink / raw)
  To: Grant Likely; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <fa686aa40710240816l6187b559l31fb72656ec265c@mail.gmail.com>

On 10/24/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> > The DTC experts need to tell us which way to make the pointers between
> > i2s and i2c for the codec.  Here's a another way it could be done that
> > looks more like the ac97 model.
>
> I *really* don't think this is a good idea.  Put the node on the bus
> that the device is addressed from.  I2S is the *data* path, not the
> *control* path, but you cannot control the codec from there.
>
> Your suggestion only looks more like the AC97 model if you're looking
> at the data path.  If you're looking at the control path it looks
> completely different.  The device tree convention is to orient around
> the control path.

It doesn't make any difference to me which one we pick. I'm just
listing all of the possible combinations. I just want to pick a model
so that we can write the code.

I see your point about putting the child node onto the control bus.
ac97 is both a control and data bus. For the i2s case the child should
go onto the i2c bus.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Audio codec device tree entries
From: Grant Likely @ 2007-10-24 15:20 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <fa686aa40710240816l6187b559l31fb72656ec265c@mail.gmail.com>

On 10/24/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 10/24/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 10/24/07, Timur Tabi <timur@freescale.com> wrote:
> > > Jon Smirl wrote:
> > > > Is this consensus on how the tree should look?
> > > >
> > > > There is no attempt to describe the codec connections inside the
> > > > device tree.
> > >
> > > I don't think I agree with that.  The device tree should indicate which codec is
> > > connected to which I2S/AC97 device.
> >
> > What I meant was that there is no attempt to describe how the codec is
> > connected to the external world. Those connections are described in
> > the fabric driver.
> >
> > I'm getting conflicting opinions on how the devices should be linked
> > into the tree. We should pick one and add it to the documentation.
>
> Two valid methods have been proposed
> 1. a codec-

oops

1. a codec-handle property in the i2s node
2. an i2s-handle property in the codec node

Either are reasonable.  I prefer putting the handle in the i2s node;
but I'm looking at it from the way that ethernet phys are being
described currently.  The other is also perfectly valid.

I suppose it depends on what point of view you see the system from; either:
a. the codec is supported by the i2s bus, in which case use the
i2s-handle property
b. the i2s bus is supported by the codec; in which case use the
codec-handle property.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: Audio codec device tree entries
From: Jon Smirl @ 2007-10-24 15:19 UTC (permalink / raw)
  To: Grant Likely; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <fa686aa40710240808r5d634905t6ba81c802e8f123a@mail.gmail.com>

On 10/24/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 10/24/07, Timur Tabi <timur@freescale.com> wrote:
> > >       codec0: i2s-codec@0 {
> > >             compatible = "ti,tas5508";
> > >             reg = <0>;
> > >             i2s-handle = <&i2s@2000>;
> > >       };
> >
> > I'd do this the other way around -- that is:
> >
> > i2s@2200 {           // PSC2
> >         compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
> >         ...
> >         i2c-handle = <&codec0>;  /* Or something like that */
>
> i2c-handle is a poor property name here.  It should be 'codec-handle'.
>  The codec could theoretically live on just about *any* control bus;
> not just i2c.

That's one of the reasons I put the second option in the post.

In the second option the i2s driver would instantiate first. Next the
generic code would get loaded. The generic codec will know the control
but for the device and it can go look in the i2c node for the address.
i2c node still lists all of the devices on the i2c bus. But the codecs
are in the i2c-handle property so they don't trigger a second loading
of the codec.

A fundamental question is, which bus should trigger the loading of the
generic codec driver. The answer to this determines how the device
tree should look.

I'm using the model that a child node means load the driver and a
reference does not load a driver.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: Audio codec device tree entries
From: Grant Likely @ 2007-10-24 15:16 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <9e4733910710240800y24952e70g8c318e35e2e45e2e@mail.gmail.com>

On 10/24/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 10/24/07, Timur Tabi <timur@freescale.com> wrote:
> > Jon Smirl wrote:
> > > Is this consensus on how the tree should look?
> > >
> > > There is no attempt to describe the codec connections inside the
> > > device tree.
> >
> > I don't think I agree with that.  The device tree should indicate which codec is
> > connected to which I2S/AC97 device.
>
> What I meant was that there is no attempt to describe how the codec is
> connected to the external world. Those connections are described in
> the fabric driver.
>
> I'm getting conflicting opinions on how the devices should be linked
> into the tree. We should pick one and add it to the documentation.

Two valid methods have been proposed
1. a codec-

> The DTC experts need to tell us which way to make the pointers between
> i2s and i2c for the codec.  Here's a another way it could be done that
> looks more like the ac97 model.

I *really* don't think this is a good idea.  Put the node on the bus
that the device is addressed from.  I2S is the *data* path, not the
*control* path, but you cannot control the codec from there.

Your suggestion only looks more like the AC97 model if you're looking
at the data path.  If you're looking at the control path it looks
completely different.  The device tree convention is to orient around
the control path.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: Audio codec device tree entries
From: Grant Likely @ 2007-10-24 15:08 UTC (permalink / raw)
  To: Timur Tabi; +Cc: PowerPC dev list
In-Reply-To: <471F52ED.10007@freescale.com>

On 10/24/07, Timur Tabi <timur@freescale.com> wrote:
> >       codec0: i2s-codec@0 {
> >             compatible = "ti,tas5508";
> >             reg = <0>;
> >             i2s-handle = <&i2s@2000>;
> >       };
>
> I'd do this the other way around -- that is:
>
> i2s@2200 {           // PSC2
>         compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
>         ...
>         i2c-handle = <&codec0>;  /* Or something like that */

i2c-handle is a poor property name here.  It should be 'codec-handle'.
 The codec could theoretically live on just about *any* control bus;
not just i2c.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: Audio codec device tree entries
From: Timur Tabi @ 2007-10-24 15:07 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710240800y24952e70g8c318e35e2e45e2e@mail.gmail.com>

Jon Smirl wrote:

> What I meant was that there is no attempt to describe how the codec is
> connected to the external world. Those connections are described in
> the fabric driver.

Hmmm, I'm not sure I'm okay with that.  We can always add properties to those 
nodes if it's necessary.  However, now you're basically defining some parts of 
the board layout in the DTS, and some parts in the fabric driver.  On PowerPC 
platforms, the fabric driver is supposed to get board layout information from 
the device tree.

> I'm getting conflicting opinions on how the devices should be linked
> into the tree. We should pick one and add it to the documentation.

It's a battle of wills!

> The DTC experts need to tell us which way to make the pointers between
> i2s and i2c for the codec.  Here's a another way it could be done that
> looks more like the ac97 model.
> 
> i2s@2000 {           // PSC1
>      compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
>      cell-index = <0>;
>      reg = <2000 100>;
>      interrupts = <2 1 0>;
>      interrupt-parent = <&mpc5200_pic>;
>      codec0: i2s-codec@0 {
>            compatible = "ti,tas5508";
>            reg = <0>;
>            i2c-handle = <&i2c@3d00>;
>      };
>      codec1: i2s-codec@1 {
>            compatible = "ti,tas5508";
>            reg = <1>;
>            i2c-handle = <&i2c@3d00>;
>      };
> };
> 
> i2s@2200 {           // PSC2
>      compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
>      cell-index = <1>;
>      reg = <2200 100>;
>      interrupts = <2 2 0>;
>      interrupt-parent = <&mpc5200_pic>;
>      codec2: i2s-codec@2 {

This should probably be codec0, since it's the first code on this I2S bus.

>            compatible = "wolson,wm8750";
>            reg = <2>;
>            i2c-handle = <&i2c@3d00>;
>      };
> };
> 
> i2c@3d00 {
>      compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
>      #address-cells = <1>;
>      #size-cells = <0>;
>      cell-index = <0>;
>      reg = <3d00 40>;
>      interrupts = <2 f 0>;
>      interrupt-parent = <&mpc5200_pic>;
>      fsl5200-clocking;
> }

My vote is for this version.  In fact, I think it *has* to be this way.  If 
you're using a CS4270 codec (as I am), the I2C interface is *optional*.  So I 
want the I2S node to point to the I2C node if it exists.

^ permalink raw reply

* Re: Audio codec device tree entries
From: Jon Smirl @ 2007-10-24 15:00 UTC (permalink / raw)
  To: Timur Tabi; +Cc: PowerPC dev list
In-Reply-To: <471F52ED.10007@freescale.com>

On 10/24/07, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
> > Is this consensus on how the tree should look?
> >
> > There is no attempt to describe the codec connections inside the
> > device tree.
>
> I don't think I agree with that.  The device tree should indicate which codec is
> connected to which I2S/AC97 device.

What I meant was that there is no attempt to describe how the codec is
connected to the external world. Those connections are described in
the fabric driver.

I'm getting conflicting opinions on how the devices should be linked
into the tree. We should pick one and add it to the documentation.

The DTC experts need to tell us which way to make the pointers between
i2s and i2c for the codec.  Here's a another way it could be done that
looks more like the ac97 model.

i2s@2000 {           // PSC1
     compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
     cell-index = <0>;
     reg = <2000 100>;
     interrupts = <2 1 0>;
     interrupt-parent = <&mpc5200_pic>;
     codec0: i2s-codec@0 {
           compatible = "ti,tas5508";
           reg = <0>;
           i2c-handle = <&i2c@3d00>;
     };
     codec1: i2s-codec@1 {
           compatible = "ti,tas5508";
           reg = <1>;
           i2c-handle = <&i2c@3d00>;
     };
};

i2s@2200 {           // PSC2
     compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
     cell-index = <1>;
     reg = <2200 100>;
     interrupts = <2 2 0>;
     interrupt-parent = <&mpc5200_pic>;
     codec2: i2s-codec@2 {
           compatible = "wolson,wm8750";
           reg = <2>;
           i2c-handle = <&i2c@3d00>;
     };
};

i2c@3d00 {
     compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
     #address-cells = <1>;
     #size-cells = <0>;
     cell-index = <0>;
     reg = <3d00 40>;
     interrupts = <2 f 0>;
     interrupt-parent = <&mpc5200_pic>;
     fsl5200-clocking;
}

----- or would this be better? ---------------------

i2s@2000 {           // PSC1
     compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
     cell-index = <0>;
     reg = <2000 100>;
     interrupts = <2 1 0>;
     interrupt-parent = <&mpc5200_pic>;
     i2s-codec@0 {
           compatible = "ti,tas5508";
           reg = <0>;
     };
     i2s-codec@1 {
           compatible = "ti,tas5508";
           reg = <1>;
     };
};

i2s@2200 {           // PSC2
     compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
     cell-index = <1>;
     reg = <2200 100>;
     interrupts = <2 2 0>;
     interrupt-parent = <&mpc5200_pic>;
     i2s-codec@2 {
           compatible = "wolson,wm8750";
           reg = <2>;
     };
};

i2c@3d00 {
     compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
     #address-cells = <1>;
     #size-cells = <0>;
     cell-index = <0>;
     reg = <3d00 40>;
     interrupts = <2 f 0>;
     interrupt-parent = <&mpc5200_pic>;
     fsl5200-clocking;
     i2c-handle = <&i2s-codec@0 &i2s-codec@1 &i2s-codec@2>
}



>
> I see that you do that for the AC97 node, but not the I2S node.  Why?
>
> > I'm still not clear on how to trigger the load of the fabric driver.
> > Right now I have a single kernel that works on Efika and my target
> > hardware.  This gets sorted out by define_machine(xxxx). I'll write
> > some code tonight to figure out how to load drivers and match on
> > codec0, codec1, etc. But how do I probe for the fabric driver I need
> > to figure out whether to load the Efika one or my target one.
>
> I've been struggling with that one, too.  To keep it simple, I have the fabric
> driver just search for all the I2S nodes in my tree, and create ASoC objects for
> each one it finds.  There's some hackery there, but I don't think we need to
> solve all the problems at once.  The only thing that *has* to be right the first
> time is the device tree.
>
> > i2s@2200 {           // PSC2
> >       compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
> >       cell-index = <1>;
> >       reg = <2200 100>;
> >       interrupts = <2 2 0>;
> >       interrupt-parent = <&mpc5200_pic>;
> > };
> >
> > i2c@3d00 {
> >       compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
> >       #address-cells = <1>;
> >       #size-cells = <0>;
> >       cell-index = <0>;
> >       reg = <3d00 40>;
> >       interrupts = <2 f 0>;
> >       interrupt-parent = <&mpc5200_pic>;
> >       fsl5200-clocking;
> >
> >       codec0: i2s-codec@0 {
> >             compatible = "ti,tas5508";
> >             reg = <0>;
> >             i2s-handle = <&i2s@2000>;
> >       };
>
> I'd do this the other way around -- that is:
>
> i2s@2200 {           // PSC2
>         compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
>         ...
>         i2c-handle = <&codec0>;  /* Or something like that */
> };
>
> The reason is because I think the I2S driver will be instantiated *first* as an
> I2S driver and then it will create the I2C instantiation.
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: libfdt: Add some documenting comments in libfdt.h
From: Jon Loeliger @ 2007-10-24 15:00 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071024071058.GC17853@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> This patch adds some internal documentation in libfdt.h, in the form
> of comments on the error codes and some functions.  Only a couple of
> functions are covered so far, leaving the documentation still woefully
> inadequate, but hey, it's a start.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

I did miss the fdt_next_tag patch earlier.
These are not the droids you are looking for.
Move along.

jdl

^ permalink raw reply

* Re: libfdt: Rename and publish _fdt_next_tag()
From: Jon Loeliger @ 2007-10-24 14:59 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071024010609.GH10595@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> Although it's a low-level function that shouldn't normally be needed,
> there are circumstances where it's useful for users of libfdt to use
> the _fdt_next_tag() function.  Therefore, this patch renames it to
> fdt_next_tag() and publishes it in libfdt.h.
> 
> In addition, this patch adds a new testcase using fdt_next_tag(),
> dtbs_equal_ordered.  This testcase tests for structural equality of
> two dtbs, including the order of properties and subnodes, but ignoring
> NOP tags, the order of the dtb sections and the layout of strings in
> the strings block.  This will be useful for testing other dtc
> functionality in the future.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

Thanks,
jdl

^ permalink raw reply

* Re: [PATCH] fix appletouch geyser 1 breakage
From: Dmitry Torokhov @ 2007-10-24 14:29 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev list, Anton Ekblad
In-Reply-To: <1193233015.4510.44.camel@johannes.berg>

On 10/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2007-10-24 at 09:34 -0400, Dmitry Torokhov wrote:
>
> > Well, but what about fountains then? Regardless of the model, if there
> > is a way to stop "empty" meaurements, we should do it.
>
> There is no way on fountains though. We could check the measurement
> ourselves and if no finger is detected decrease the polling frequency or
> something, but there's no hw support.
>

Do yo know who has powerbooks with older geyser models (0x214, 215,
216)? It would be nice to know if they send the data continiously and
whether the geyser 3 reset hack works on them.

-- 
Dmitry

^ permalink raw reply

* Re: ioctl32 unknown cmds with 2.6.24-rc1
From: Arnd Bergmann @ 2007-10-24 14:27 UTC (permalink / raw)
  To: linuxppc-dev, Geert Uytterhoeven; +Cc: Johannes Berg
In-Reply-To: <1193229045.4510.35.camel@johannes.berg>

On Wednesday 24 October 2007, Johannes Berg wrote:
>   Show Details
>   I've been getting these warnings (many more of them but this is a list
> of unique ones) on my quad G5 with 32-bit userspace:
> 
> ioctl32(cdrom_id:1078): Unknown cmd fd(3) cmd(00005331){t:'S';sz:0} arg(00000000) on /dev/.tmp-3-0
> ioctl32(ata_id:1095): Unknown cmd fd(3) cmd(0000030d){t:03;sz:0} arg(ff863970) on /dev/.tmp-3-0
> ioctl32(smartd:3563): Unknown cmd fd(3) cmd(0000031f){t:03;sz:0} arg(ffeb5480) on /dev/sda
> ioctl32(hald-probe-stor:3761): Unknown cmd fd(4) cmd(00005320){t:'S';sz:0} arg(00000004) on /dev/hda
> ioctl32(gnome-terminal:4187): Unknown cmd fd(19) cmd(0000530b){t:'S';sz:0} arg(0fd8e400) on /dev/pts/0
> 
> Does anybody know whether this is expected?

It's probably my fault, since I changed the compat ioctl handling for
block devices. Geert already reported the same, but I haven't had
a chance to reproduce it on my system to look into what went wrong.
Probably a trivial bug I introduced in block/compat_ioctl.c

	Arnd <><

^ permalink raw reply

* Re: [PATCH] PowerPC 440EPx Sequoia USB OHCI DTS entry
From: Valentine Barshak @ 2007-10-24 14:23 UTC (permalink / raw)
  To: Dale Farnsworth; +Cc: Linuxppc-dev
In-Reply-To: <20071024141041.GA13586@xyzzy.farnsworth.org>

Dale Farnsworth wrote:
> On Wed, Oct 24, 2007 at 05:44:51PM +0400, Valentine Barshak wrote:
>> Dale Farnsworth wrote:
>>> On Wed, Oct 24, 2007 at 03:19:14PM +0400, Valentine Barshak wrote:
>>>> Dale Farnsworth wrote:
>>>>> Valentine wrote:
>>>>>> Actually I also don't see much reason for the 
>>>>>> USB_OHCI_HCD_PPC_OF_BE/USB_OHCI_HCD_PPC_OF_LE stuff.
>>>>>> Is this really needed?
>>>>> I think so.  The SOC host controllers are BE and the PCI
>>>>> host controllers are LE.  Or, do you have an alternative
>>>>> method of handling both types?
>>>> Yes, PCI controllers are LE, but do we really need user-selectable 
>>>> USB_OHCI_HCD_PPC_OF_LE option, since USB_OHCI_LITTLE_ENDIAN is selected
>>>> by default for USB_OHCI_HCD_PCI?
>>>> The USB_OHCI_HCD_PPC_OF_LE/BE stuff is related to PPC OF glue only.
>>>> I think it's useless. We should always enable
>>>> USB_OHCI_BIG_ENDIAN_DESC and USB_OHCI_BIG_ENDIAN_MMIO for PPC OF
>>>> and the real LE/BE implementation should be selected by the 
>>>> corresponding properties in the device tree.
>>> I agree that they don't need to be user selectable.  It is far preferable
>>> to deduce their values from existing information, if possible.
>>>
>>> -Dale
>> This is the original thread:
>> http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025054.html
>>
>> I think the USB_OHCI_HCD_PPC_OF_LE/BE should be removed.
>> We can't avoid the slight overhead even using these options, since 
>> USB_OHCI_BIG_ENDIAN_MMIO/DESC should always be anabled for PPC OF and we 
>> we still enable USB_OHCI_LITTLE_ENDIAN for USB_OHCI_HCD_PCI even if 
>> USB_OHCI_HCD_PPC_OF_LE is not set.
> 
> I believe you are saying that we can select any valid combination
> of USB_OHCI_BIG_ENDIAN_DESC, USB_OHCI_BIG_ENDIAN_MMIO, and
> USB_OHCI_LITTLE_ENDIAN, without using USB_OHCI_HCD_PPC_OF_BE and
> USB_OHCI_HCD_PPC_OF_LE.  I agree.  It looks like we can get rid of
> these last two with zero loss in performance or functionality.
> 
> Do you have a patch?

No I don't have it yet :)
I planed to make, test and submit it a bit later.
Thanks,
Valentine.

> 
> -Dale

^ permalink raw reply

* Re: [PATCH 09/11] [POWERPC] Motion-PRO: Add LED support.
From: Grant Likely @ 2007-10-24 14:18 UTC (permalink / raw)
  To: Marian Balakowicz; +Cc: linuxppc-dev
In-Reply-To: <20071023231358.29359.90259.stgit@hekate.izotz.org>

On 10/23/07, Marian Balakowicz <m8@semihalf.com> wrote:
> Add LED driver for Promess Motion-PRO board.
>
> Signed-off-by: Jan Wrobel <wrr@semihalf.com>
> Signed-off-by: Marian Balakowicz <m8@semihalf.com>
> ---
>
>  drivers/leds/Kconfig          |    7 +
>  drivers/leds/Makefile         |    3 -
>  drivers/leds/leds-motionpro.c |  222 +++++++++++++++++++++++++++++++++++++++++
>  include/asm-powerpc/mpc52xx.h |    5 +
>  4 files changed, 236 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/leds/leds-motionpro.c
>
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index ec568fa..1567ed6 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -55,6 +55,13 @@ config LEDS_TOSA
>           This option enables support for the LEDs on Sharp Zaurus
>           SL-6000 series.
>
> +config LEDS_MOTIONPRO
> +       tristate "Motion-PRO LEDs Support"
> +       depends on LEDS_CLASS && PPC_MPC5200
> +       help
> +         This option enables support for status and ready LEDs connected
> +         to GPIO lines on Motion-PRO board.
> +
>  config LEDS_S3C24XX
>         tristate "LED Support for Samsung S3C24XX GPIO LEDs"
>         depends on LEDS_CLASS && ARCH_S3C2410
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index a60de1b..a56d399 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -18,7 +18,8 @@ obj-$(CONFIG_LEDS_H1940)              += leds-h1940.o
>  obj-$(CONFIG_LEDS_COBALT_QUBE)         += leds-cobalt-qube.o
>  obj-$(CONFIG_LEDS_COBALT_RAQ)          += leds-cobalt-raq.o
>  obj-$(CONFIG_LEDS_GPIO)                        += leds-gpio.o
> -obj-$(CONFIG_LEDS_CM_X270)              += leds-cm-x270.o
> +obj-$(CONFIG_LEDS_CM_X270)             += leds-cm-x270.o
> +obj-$(CONFIG_LEDS_MOTIONPRO)           += leds-motionpro.o
>
>  # LED Triggers
>  obj-$(CONFIG_LEDS_TRIGGER_TIMER)       += ledtrig-timer.o
> diff --git a/drivers/leds/leds-motionpro.c b/drivers/leds/leds-motionpro.c
> new file mode 100644
> index 0000000..d4b872c
> --- /dev/null
> +++ b/drivers/leds/leds-motionpro.c
> @@ -0,0 +1,222 @@
> +/*
> + * LEDs driver for the Motionpro board.
> + *
> + * Copyright (C) 2007 Semihalf
> + *
> + * Author: Jan Wrobel <wrr@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc., 51
> + * Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + *
> + * This driver enables control over Motionpro's status and ready LEDs through
> + * sysfs. LEDs can be controlled by writing to sysfs files:
> + * class/leds/motionpro-(ready|status)led/(brightness|delay_off|delay_on).
> + * See Documentation/leds-class.txt for more details
> + *
> + * Before user issues first control command via sysfs, LED blinking is
> + * controlled by the kernel. By default status LED is blinking fast and ready
> + * LED is turned off.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/leds.h>
> +
> +#include <asm/mpc52xx.h>
> +#include <asm/io.h>
> +
> +/* Led status */
> +#define LED_NOT_REGISTERED     0
> +#define LED_KERNEL_CONTROLLED  1
> +#define LED_USER_CONTROLLED    2
> +
> +/* Led control bits */
> +#define LED_ON MPC52xx_GPT_OUTPUT_1
> +
> +static void mpled_set(struct led_classdev *led_cdev,
> +                     enum led_brightness brightness);
> +
> +struct motionpro_led{
> +       /* Protects the led data */
> +       spinlock_t led_lock;
> +
> +       /* Path to led's control register DTS node */
> +       char *reg_compat;
> +
> +       /* Address to access led's register */
> +       void __iomem *reg_addr;
> +
> +       int status;
> +
> +       /* Blinking timer used when led is controlled by the kernel */
> +       struct timer_list kernel_mode_timer;
> +
> +       /*
> +        * Delay between blinks when led is controlled by the kernel.
> +        * If set to 0 led blinking is off.
> +        */
> +       int kernel_mode_delay;
> +
> +       struct led_classdev classdev;
> +};
> +
> +static struct motionpro_led led[] = {
> +       {
> +               .reg_compat = "promess,motionpro-statusled",
> +               .reg_addr = 0,
> +               .led_lock = SPIN_LOCK_UNLOCKED,
> +               .status = LED_NOT_REGISTERED,
> +               .kernel_mode_delay = HZ / 10,
> +               .classdev = {
> +                       .name = "motionpro-statusled",
> +                       .brightness_set = mpled_set,
> +                       .default_trigger = "timer",
> +               },
> +       },
> +       {
> +               .reg_compat = "promess,motionpro-readyled",
> +               .reg_addr = 0,
> +               .led_lock = SPIN_LOCK_UNLOCKED,
> +               .status = LED_NOT_REGISTERED,
> +               .kernel_mode_delay = 0,
> +               .classdev = {
> +                       .name = "motionpro-readyled",
> +                       .brightness_set = mpled_set,
> +                       .default_trigger = "timer",
> +               }
> +       }
> +};
> +
> +/* Timer event - blinks led before user takes control over it */
> +static void mpled_timer_toggle(unsigned long ptr)
> +{
> +       struct motionpro_led *mled = (struct motionpro_led *) ptr;
> +
> +       spin_lock_bh(&mled->led_lock);
> +       if (mled->status == LED_KERNEL_CONTROLLED){
> +               u32 reg = in_be32(mled->reg_addr);
> +               reg ^= LED_ON;
> +               out_be32(mled->reg_addr, reg);
> +               led->kernel_mode_timer.expires = jiffies +
> +                       led->kernel_mode_delay;
> +               add_timer(&led->kernel_mode_timer);
> +       }
> +       spin_unlock_bh(&mled->led_lock);
> +}
> +
> +
> +/*
> + * Turn on/off led according to user settings in sysfs.
> + * First call to this function disables kernel blinking.
> + */
> +static void mpled_set(struct led_classdev *led_cdev,
> +                     enum led_brightness brightness)
> +{
> +       struct motionpro_led *mled;
> +       u32 reg;
> +
> +       mled = container_of(led_cdev, struct motionpro_led, classdev);
> +
> +       spin_lock_bh(&mled->led_lock);
> +       mled->status = LED_USER_CONTROLLED;
> +
> +       reg = in_be32(mled->reg_addr);
> +       if (brightness)
> +               reg |= LED_ON;
> +       else
> +               reg &= ~LED_ON;
> +       out_be32(mled->reg_addr, reg);
> +
> +       spin_unlock_bh(&mled->led_lock);
> +}
> +
> +static void mpled_init_led(void __iomem *reg_addr)
> +{
> +       u32 reg = in_be32(reg_addr);
> +       reg |= MPC52xx_GPT_ENABLE_OUTPUT;
> +       reg &= ~LED_ON;
> +       out_be32(reg_addr, reg);
> +}
> +
> +static void mpled_clean(void)
> +{
> +       int i;
> +       for (i = 0; i < sizeof(led) / sizeof(struct motionpro_led); i++){
> +               if (led[i].status != LED_NOT_REGISTERED){
> +                       spin_lock_bh(&led[i].led_lock);
> +                       led[i].status = LED_NOT_REGISTERED;
> +                       spin_unlock_bh(&led[i].led_lock);
> +                       led_classdev_unregister(&led[i].classdev);
> +               }
> +               if (led[i].reg_addr){
> +                       iounmap(led[i].reg_addr);
> +                       led[i].reg_addr = 0;
> +               }
> +       }
> +}
> +
> +static int __init mpled_init(void)
> +{
> +       int i, error;
> +
> +       for (i = 0; i < sizeof(led) / sizeof(struct motionpro_led); i++){
> +               led[i].reg_addr = mpc52xx_find_and_map(led[i].reg_compat);

Please use of-platform-bus bindings instead.  Let the of-platform bus
take care of scanning the tree for you.  Don't do it manually.

> +               if (!led[i].reg_addr){
> +                       printk(KERN_ERR __FILE__ ": "
> +                              "Error while mapping GPIO register for LED %s\n",
> +                              led[i].classdev.name);
> +                       error = -EIO;
> +                       goto err;
> +               }
> +
> +               mpled_init_led(led[i].reg_addr);
> +               led[i].status = LED_KERNEL_CONTROLLED;
> +               if (led[i].kernel_mode_delay){
> +                       init_timer(&led[i].kernel_mode_timer);
> +                       led[i].kernel_mode_timer.function = mpled_timer_toggle;
> +                       led[i].kernel_mode_timer.data = (unsigned long)&led[i];
> +                       led[i].kernel_mode_timer.expires =
> +                               jiffies + led[i].kernel_mode_delay;
> +                       add_timer(&led[i].kernel_mode_timer);
> +               }
> +
> +               if ((error = led_classdev_register(NULL, &led[i].classdev)) < 0){
> +                       printk(KERN_ERR __FILE__ ": "
> +                              "Error while registering class device for LED "
> +                              "%s\n",
> +                              led[i].classdev.name);
> +                       goto err;
> +               }
> +       }
> +
> +       printk("Motionpro LEDs driver initialized\n");
> +       return 0;
> +err:
> +       mpled_clean();
> +       return error;
> +}
> +
> +static void __exit mpled_exit(void)
> +{
> +       mpled_clean();
> +}
> +
> +module_init(mpled_init);
> +module_exit(mpled_exit);
> +
> +MODULE_LICENSE("GPL")
> +MODULE_DESCRIPTION("LEDs support for Motionpro");
> +MODULE_AUTHOR("Jan Wrobel <wrr@semihalf.com>");
> diff --git a/include/asm-powerpc/mpc52xx.h b/include/asm-powerpc/mpc52xx.h
> index 859ffb0..7e20f54 100644
> --- a/include/asm-powerpc/mpc52xx.h
> +++ b/include/asm-powerpc/mpc52xx.h
> @@ -140,6 +140,11 @@ struct mpc52xx_gpio {
>  #define MPC52xx_GPIO_PSC_CONFIG_UART_WITH_CD   5
>  #define MPC52xx_GPIO_PCI_DIS                   (1<<15)
>
> +/* Enables GPT register to operate as simple GPIO output register */
> +#define MPC52xx_GPT_ENABLE_OUTPUT      0x00000024
> +/* Puts 1 on GPT output pin */
> +#define MPC52xx_GPT_OUTPUT_1           0x00000010
> +
>  /* GPIO with WakeUp*/
>  struct mpc52xx_gpio_wkup {
>         u8 wkup_gpioe;          /* GPIO_WKUP + 0x00 */
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: Audio codec device tree entries
From: Timur Tabi @ 2007-10-24 14:13 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710231529h1089eacdy888306f20af92555@mail.gmail.com>

Jon Smirl wrote:
> Is this consensus on how the tree should look?
> 
> There is no attempt to describe the codec connections inside the
> device tree. 

I don't think I agree with that.  The device tree should indicate which codec is 
connected to which I2S/AC97 device.

I see that you do that for the AC97 node, but not the I2S node.  Why?

> I'm still not clear on how to trigger the load of the fabric driver.
> Right now I have a single kernel that works on Efika and my target
> hardware.  This gets sorted out by define_machine(xxxx). I'll write
> some code tonight to figure out how to load drivers and match on
> codec0, codec1, etc. But how do I probe for the fabric driver I need
> to figure out whether to load the Efika one or my target one.

I've been struggling with that one, too.  To keep it simple, I have the fabric 
driver just search for all the I2S nodes in my tree, and create ASoC objects for 
each one it finds.  There's some hackery there, but I don't think we need to 
solve all the problems at once.  The only thing that *has* to be right the first 
time is the device tree.

> i2s@2200 {           // PSC2
>       compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
>       cell-index = <1>;
>       reg = <2200 100>;
>       interrupts = <2 2 0>;
>       interrupt-parent = <&mpc5200_pic>;
> };
> 
> i2c@3d00 {
>       compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
>       #address-cells = <1>;
>       #size-cells = <0>;
>       cell-index = <0>;
>       reg = <3d00 40>;
>       interrupts = <2 f 0>;
>       interrupt-parent = <&mpc5200_pic>;
>       fsl5200-clocking;
> 
>       codec0: i2s-codec@0 {
>             compatible = "ti,tas5508";
>             reg = <0>;
>             i2s-handle = <&i2s@2000>;
>       };

I'd do this the other way around -- that is:

i2s@2200 {           // PSC2
       	compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
	...
	i2c-handle = <&codec0>;	 /* Or something like that */
};

The reason is because I think the I2S driver will be instantiated *first* as an 
I2S driver and then it will create the I2C instantiation.

^ permalink raw reply

* Re: [PATCH] PowerPC 440EPx Sequoia USB OHCI DTS entry
From: Dale Farnsworth @ 2007-10-24 14:10 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: Linuxppc-dev
In-Reply-To: <471F4C53.1070506@ru.mvista.com>

On Wed, Oct 24, 2007 at 05:44:51PM +0400, Valentine Barshak wrote:
> Dale Farnsworth wrote:
> >On Wed, Oct 24, 2007 at 03:19:14PM +0400, Valentine Barshak wrote:
> >>Dale Farnsworth wrote:
> >>>Valentine wrote:
> >>>>Actually I also don't see much reason for the 
> >>>>USB_OHCI_HCD_PPC_OF_BE/USB_OHCI_HCD_PPC_OF_LE stuff.
> >>>>Is this really needed?
> >>>I think so.  The SOC host controllers are BE and the PCI
> >>>host controllers are LE.  Or, do you have an alternative
> >>>method of handling both types?
> >>Yes, PCI controllers are LE, but do we really need user-selectable 
> >>USB_OHCI_HCD_PPC_OF_LE option, since USB_OHCI_LITTLE_ENDIAN is selected
> >>by default for USB_OHCI_HCD_PCI?
> >>The USB_OHCI_HCD_PPC_OF_LE/BE stuff is related to PPC OF glue only.
> >>I think it's useless. We should always enable
> >>USB_OHCI_BIG_ENDIAN_DESC and USB_OHCI_BIG_ENDIAN_MMIO for PPC OF
> >>and the real LE/BE implementation should be selected by the 
> >>corresponding properties in the device tree.
> >
> >I agree that they don't need to be user selectable.  It is far preferable
> >to deduce their values from existing information, if possible.
> >
> >-Dale
> 
> This is the original thread:
> http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025054.html
> 
> I think the USB_OHCI_HCD_PPC_OF_LE/BE should be removed.
> We can't avoid the slight overhead even using these options, since 
> USB_OHCI_BIG_ENDIAN_MMIO/DESC should always be anabled for PPC OF and we 
> we still enable USB_OHCI_LITTLE_ENDIAN for USB_OHCI_HCD_PCI even if 
> USB_OHCI_HCD_PPC_OF_LE is not set.

I believe you are saying that we can select any valid combination
of USB_OHCI_BIG_ENDIAN_DESC, USB_OHCI_BIG_ENDIAN_MMIO, and
USB_OHCI_LITTLE_ENDIAN, without using USB_OHCI_HCD_PPC_OF_BE and
USB_OHCI_HCD_PPC_OF_LE.  I agree.  It looks like we can get rid of
these last two with zero loss in performance or functionality.

Do you have a patch?

-Dale

^ permalink raw reply

* Re: [PATCH 05/11] [POWERPC] TQM5200 DTS
From: Grant Likely @ 2007-10-24 14:09 UTC (permalink / raw)
  To: Marian Balakowicz, linuxppc-dev
In-Reply-To: <20071024015115.GK10595@localhost.localdomain>

On 10/23/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Oct 24, 2007 at 01:13:33AM +0200, Marian Balakowicz wrote:
> > Add device tree source file for TQM5200 board.
> >
> > Signed-off-by: Marian Balakowicz <m8@semihalf.com>
> > ---
> >
> >  arch/powerpc/boot/dts/tqm5200.dts |  236 +++++++++++++++++++++++++++++++++++++
> >  1 files changed, 236 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/powerpc/boot/dts/tqm5200.dts
> >
> >
> > diff --git a/arch/powerpc/boot/dts/tqm5200.dts b/arch/powerpc/boot/dts/tqm5200.dts
> > new file mode 100644
> > index 0000000..01c7778
> > --- /dev/null
> > +++ b/arch/powerpc/boot/dts/tqm5200.dts
> > @@ -0,0 +1,236 @@
> > +/*
> > + * TQM5200 board Device Tree Source
> [snip]
> > +     soc5200@f0000000 {
>
> I thought we were moving towards calling these just /soc@address?

The only reason this is still like this is that u-boot does a path
based lookup for the soc5200 node on the TQM board.  We need to change
u-boot too before the 5200 can be dropped.

>
> > +             model = "fsl,mpc5200";
> > +             compatible = "mpc5200";
>
> That should have a vendor prefix.
>
> [snip]
> > +             serial@2000 {           // PSC1
> > +                     device_type = "serial";
> > +                     compatible = "mpc5200-psc-uart";
> > +                     port-number = <0>;  // Logical port assignment
>
> How are these port-number things used?  The device tree shouldn't
> generally contain information that isn't inherent to the hardware.
> There can be reasons for hacks like this, but we should avoid them if
> possible.

This was an approach I was taking a while back to assign logical ports
(ttyPSC0) to a PSC.  I'm working on eliminating this.  As you
suggested, I'm looking into using aliases for this.

>
> > +                     cell-index = <0>;
>
> cell-index should only be used if the index number is used when
> manipulating the hardware (e.g. if there's a global control register
> which takes this number).

And there is in this case.

>
> [snip]
> > +             ata@3a00 {
> > +                     device_type = "ata";
>
> No such thing as device_type = "ata", drop it.  In general, never
> include a device_type unless a binding explicitly says to do so.

Again, my fault from the lite5200.

>
> [snip]
> > +     lpb@fc000000 {
> > +             model = "fsl,lpb";
> > +             compatible = "lpb";
>
> Not nearly specific enough.  Must include a vendor prefix at least,
> and should have a lot more revision information.  You should always be
> able to pick the right driver with compatible alone, "model" should
> generally be for human consumption, the driver shouldn't need it.
>
> > +             device_type = "lpb";
>
> Drop this.  Again, presence of a device_type property is the
> exception, not the rule.
>
> > +             ranges = <0 fc000000 02000000>;
>
> You need #address-cells and #size-cells properties, too.
>
> [snip]
>
> --
> 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
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [PATCH 04/11] [POWERPC] Add generic support for MPC5200 based boards
From: Grant Likely @ 2007-10-24 14:03 UTC (permalink / raw)
  To: Marian Balakowicz; +Cc: linuxppc-dev
In-Reply-To: <20071023231327.29359.28666.stgit@hekate.izotz.org>

On 10/23/07, Marian Balakowicz <m8@semihalf.com> wrote:
> This patch adds support for 'generic-mpc5200' compatible
> boards which do not need a platform specific setup.
>
> Signed-off-by: Marian Balakowicz <m8@semihalf.com>

I like this patch and I think it will make it easier to support
multiple 5200 based boards.  However, we must be very clear here what
'generic-mpc5200' is (in fact, my suggestion of the name
"generic-mpc5200" might be a bad one.  "mpc5200-simple-platform" might
be a better name).

Please add documentation to both the Kconfig and the generic_mpc5200.c
files describing exactly the assumptions which are made for this
platform.  For example:
- port_config and clocking are setup correctly by firmware,
- if the fsl,has-wdt property is present in one of the gpt nodes, then
it is safe to use it to reset the board
etc.

Cheers,
g.

> ---
>
>  arch/powerpc/platforms/52xx/Kconfig           |    8 ++-
>  arch/powerpc/platforms/52xx/Makefile          |    1
>  arch/powerpc/platforms/52xx/generic_mpc5200.c |   63 +++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/platforms/52xx/generic_mpc5200.c
>
>
> diff --git a/arch/powerpc/platforms/52xx/Kconfig b/arch/powerpc/platforms/52xx/Kconfig
> index 2938d49..59c67b5 100644
> --- a/arch/powerpc/platforms/52xx/Kconfig
> +++ b/arch/powerpc/platforms/52xx/Kconfig
> @@ -19,6 +19,12 @@ config PPC_MPC5200_BUGFIX
>
>           It is safe to say 'Y' here
>
> +config PPC_GENERIC_MPC5200
> +       bool "Generic support for MPC5200 based boards"
> +       depends on PPC_MULTIPLATFORM && PPC32
> +       select PPC_MPC5200
> +       default n
> +
>  config PPC_EFIKA
>         bool "bPlan Efika 5k2. MPC5200B based computer"
>         depends on PPC_MULTIPLATFORM && PPC32
> @@ -34,5 +40,3 @@ config PPC_LITE5200
>         select WANT_DEVICE_TREE
>         select PPC_MPC5200
>         default n
> -
> -
> diff --git a/arch/powerpc/platforms/52xx/Makefile b/arch/powerpc/platforms/52xx/Makefile
> index 307dbc1..bea05df 100644
> --- a/arch/powerpc/platforms/52xx/Makefile
> +++ b/arch/powerpc/platforms/52xx/Makefile
> @@ -6,6 +6,7 @@ obj-y                           += mpc52xx_pic.o mpc52xx_common.o
>  obj-$(CONFIG_PCI)              += mpc52xx_pci.o
>  endif
>
> +obj-$(CONFIG_PPC_GENERIC_MPC5200) += generic_mpc5200.o
>  obj-$(CONFIG_PPC_EFIKA)                += efika.o
>  obj-$(CONFIG_PPC_LITE5200)     += lite5200.o
>
> diff --git a/arch/powerpc/platforms/52xx/generic_mpc5200.c b/arch/powerpc/platforms/52xx/generic_mpc5200.c
> new file mode 100644
> index 0000000..a4ec899
> --- /dev/null
> +++ b/arch/powerpc/platforms/52xx/generic_mpc5200.c
> @@ -0,0 +1,63 @@
> +/*
> + * Support for 'generic-mpc5200' compatible platforms.
> + *
> + * Written by Marian Balakowicz <m8@semihalf.com>
> + * Copyright (C) 2007 Semihalf
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#undef DEBUG
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include <linux/of.h>
> +#include <asm/time.h>
> +#include <asm/io.h>
> +#include <asm/machdep.h>
> +#include <asm/prom.h>
> +#include <asm/mpc52xx.h>
> +
> +/*
> + * Setup the architecture
> + */
> +static void __init generic_mpc5200_setup_arch(void)
> +{
> +       if (ppc_md.progress)
> +               ppc_md.progress("generic_mpc5200_setup_arch()", 0);
> +
> +       /* Some mpc5200 & mpc5200b related configuration */
> +       mpc5200_setup_xlb_arbiter();
> +
> +       /* Map wdt for mpc52xx_restart() */
> +       mpc52xx_map_wdt();
> +
> +#ifdef CONFIG_PCI
> +       mpc52xx_setup_pci();
> +#endif
> +}
> +
> +/*
> + * Called very early, MMU is off, device-tree isn't unflattened
> + */
> +static int __init generic_mpc5200_probe(void)
> +{
> +       unsigned long node = of_get_flat_dt_root();
> +
> +       if (!of_flat_dt_is_compatible(node, "generic-mpc5200"))
> +               return 0;
> +       return 1;
> +}
> +
> +define_machine(generic_mpc5200) {
> +       .name           = "generic-mpc5200",
> +       .probe          = generic_mpc5200_probe,
> +       .setup_arch     = generic_mpc5200_setup_arch,
> +       .init           = mpc52xx_declare_of_platform_devices,
> +       .init_IRQ       = mpc52xx_init_irq,
> +       .get_irq        = mpc52xx_get_irq,
> +       .restart        = mpc52xx_restart,
> +       .calibrate_decr = generic_calibrate_decr,
> +};
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: libfdt: Add some documenting comments in libfdt.h
From: Jon Loeliger @ 2007-10-24 13:45 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071024071058.GC17853@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> This patch adds some internal documentation in libfdt.h, in the form
> of comments on the error codes and some functions.  Only a couple of
> functions are covered so far, leaving the documentation still woefully
> inadequate, but hey, it's a start.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Dave,

Sorry, I can't get this patch to apply either before or after
the previous patch.  Did I miss one?

Thanks,
jdl

^ permalink raw reply

* Re: [PATCH] PowerPC 440EPx Sequoia USB OHCI DTS entry
From: Valentine Barshak @ 2007-10-24 13:44 UTC (permalink / raw)
  To: Dale Farnsworth; +Cc: Linuxppc-dev
In-Reply-To: <20071024120804.GA1180@xyzzy.farnsworth.org>

Dale Farnsworth wrote:
> On Wed, Oct 24, 2007 at 03:19:14PM +0400, Valentine Barshak wrote:
>> Dale Farnsworth wrote:
>>> Valentine wrote:
>>>> Actually I also don't see much reason for the 
>>>> USB_OHCI_HCD_PPC_OF_BE/USB_OHCI_HCD_PPC_OF_LE stuff.
>>>> Is this really needed?
>>> I think so.  The SOC host controllers are BE and the PCI
>>> host controllers are LE.  Or, do you have an alternative
>>> method of handling both types?
>> Yes, PCI controllers are LE, but do we really need user-selectable 
>> USB_OHCI_HCD_PPC_OF_LE option, since USB_OHCI_LITTLE_ENDIAN is selected
>> by default for USB_OHCI_HCD_PCI?
>> The USB_OHCI_HCD_PPC_OF_LE/BE stuff is related to PPC OF glue only.
>> I think it's useless. We should always enable
>> USB_OHCI_BIG_ENDIAN_DESC and USB_OHCI_BIG_ENDIAN_MMIO for PPC OF
>> and the real LE/BE implementation should be selected by the 
>> corresponding properties in the device tree.
> 
> I agree that they don't need to be user selectable.  It is far preferable
> to deduce their values from existing information, if possible.
> 
> -Dale

This is the original thread:
http://ozlabs.org/pipermail/linuxppc-embedded/2006-November/025054.html

I think the USB_OHCI_HCD_PPC_OF_LE/BE should be removed.
We can't avoid the slight overhead even using these options, since 
USB_OHCI_BIG_ENDIAN_MMIO/DESC should always be anabled for PPC OF and we 
we still enable USB_OHCI_LITTLE_ENDIAN for USB_OHCI_HCD_PCI even if 
USB_OHCI_HCD_PPC_OF_LE is not set.
Thanks,
Valentine.

^ permalink raw reply

* Re: [PATCH] fix appletouch geyser 1 breakage
From: Johannes Berg @ 2007-10-24 13:36 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linuxppc-dev list, Anton Ekblad
In-Reply-To: <d120d5000710240634j21d2b660t267ace56bd9d6af6@mail.gmail.com>

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

On Wed, 2007-10-24 at 09:34 -0400, Dmitry Torokhov wrote:

> Well, but what about fountains then? Regardless of the model, if there
> is a way to stop "empty" meaurements, we should do it.

There is no way on fountains though. We could check the measurement
ourselves and if no finger is detected decrease the polling frequency or
something, but there's no hw support.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH] fix appletouch geyser 1 breakage
From: Dmitry Torokhov @ 2007-10-24 13:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev list, Anton Ekblad
In-Reply-To: <1193231110.4510.42.camel@johannes.berg>

On 10/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> Hi,
>
> > My fault, sorry.
>
> No, actually, I was wrong about Geyser 1, mine is a fountain.
>
> > Is there a way to "plug" these Geysers? Waking up the kernel
> > continuously is not nice.
>
> Not sure really, maybe checking for is_geyser instead of is_geyser_3?
>

Well, but what about fountains then? Regardless of the model, if there
is a way to stop "empty" meaurements, we should do it.

-- 
Dmitry

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox