Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Liam Girdwood
On Thu, 2008-01-10 at 00:41 -0500, Jon Smirl wrote:

 ASOC v2 is sitting on a Wolfson server out of tree. I have been using
 it for several months without problem. The pace of it being merged
 could probably be sped up.

I think we are probably looking at submission in the next 8 - 10 weeks.
Currently most of the core code is complete, however some platforms and
codecs still need porting.

Liam

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Timur Tabi
Liam Girdwood wrote:

 I think we are probably looking at submission in the next 8 - 10 weeks.
 Currently most of the core code is complete, however some platforms and
 codecs still need porting.

With that in mind, can I get some kind of consensus from the PPC side as to 
whether this ASoC V1 driver is okay?  I want to get it into 2.6.25.

Keep in mind:

1) ASoC V1 is not PowerPC-friendly, so it's impossible to make an ASoC V1 
PowerPC driver 100% correct.

2) The CS4270 driver does not support I2C nodes in the device tree, so there's 
not point in adding any to the 8610 DTS.

3) Liam and I are working on porting this driver to ASoC V2 and resolving all 
open PPC issue, but that won't be done in time for 2.6.25.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Grant Likely
On 1/10/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Liam Girdwood wrote:

  I think we are probably looking at submission in the next 8 - 10 weeks.
  Currently most of the core code is complete, however some platforms and
  codecs still need porting.

 With that in mind, can I get some kind of consensus from the PPC side as to
 whether this ASoC V1 driver is okay?  I want to get it into 2.6.25.

 Keep in mind:

 1) ASoC V1 is not PowerPC-friendly, so it's impossible to make an ASoC V1
 PowerPC driver 100% correct.

The driver doesn't need to be 100% correct.  Drivers are easy to
change if they aren't quite right.  There are no long term
consequences.

However, the device tree issues must be addressed before it is merged
and deployed.  Otherwise we end up having to support poorly designed
trees over the long term.

So, I'm okay with merging the driver *minus* the .dts and
booting-without-of.txt changes.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Jon Smirl
On 1/10/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

  The driver doesn't need to be 100% correct.  Drivers are easy to
  change if they aren't quite right.  There are no long term
  consequences.
  
  However, the device tree issues must be addressed before it is merged
  and deployed.  Otherwise we end up having to support poorly designed
  trees over the long term.

 I agree.  Correct me if I'm wrong, but I think the only device tree issue is
 the definition of the 'codec' node under the SSI node.  If so, I'm not sure 
 what
 other changes need to be mode.

Isn't your codec is i2c controlled? Where does the main node for the
code live, i2c bus or ssi bus? What does the link between the buses
look like for pointing to the codec entry?

What about fsl,ssi being too generic for a compatible type?


 --
 Timur Tabi
 Linux kernel developer at Freescale



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Grant Likely
On 1/10/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:

  Isn't your codec is i2c controlled? Where does the main node for the
  code live, i2c bus or ssi bus? What does the link between the buses
  look like for pointing to the codec entry?

 The CS4270 driver is old style, which means it probes all possible I2C
 addresses until it finds a hit, and then stops.  This has all the obvious
 drawbacks, but I'm stuck with that design for now.

  What about fsl,ssi being too generic for a compatible type?

 Already fixed:

 [EMAIL PROTECTED] {
 compatible = fsl,mpc8610-ssi;

Nit: node name should be either [EMAIL PROTECTED] (the mode that it is in), or
[EMAIL PROTECTED] (if you feel that this node encapsulates the concept of a
sound device enough).

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Jon Smirl
On 1/10/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:

  Isn't your codec is i2c controlled? Where does the main node for the
  code live, i2c bus or ssi bus? What does the link between the buses
  look like for pointing to the codec entry?

 The CS4270 driver is old style, which means it probes all possible I2C
 addresses until it finds a hit, and then stops.  This has all the obvious
 drawbacks, but I'm stuck with that design for now.

So the codec is controlled from the i2c bus and SSI is used to supply
it with data. Based on what has been said on this list, the device
tree node for the codec should be on the i2c bus with a link from the
ssi bus to it.


  What about fsl,ssi being too generic for a compatible type?

 Already fixed:

 [EMAIL PROTECTED] {
 compatible = fsl,mpc8610-ssi;
 --
 Timur Tabi
 Linux kernel developer at Freescale



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Timur Tabi
Jon Smirl wrote:

 So the codec is controlled from the i2c bus and SSI is used to supply
 it with data. Based on what has been said on this list, the device
 tree node for the codec should be on the i2c bus with a link from the
 ssi bus to it.

I'm working on that now.  I'll have a new patch with this exact change this 
afternoon.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-10 Thread Timur Tabi
Grant Likely wrote:

 Nit: node name should be either [EMAIL PROTECTED] (the mode that it is in), or
 [EMAIL PROTECTED] (if you feel that this node encapsulates the concept of a
 sound device enough).

Well, SSI stands for Synchronous Serial  Interface (although it's capable of 
asynchronous communication as well).  From the manual:

The SSI is a full-duplex, serial port that allows the chip to communicate with 
a variety of serial devices.  These serial devices can be standard 
CODer-DECoder 
(CODECs), Digital Signal Processors (DSPs), microprocessors, peripherals, and 
popular industry audio CODECs that implement the inter-IC sound bus standard 
(I2S) and Intel AC97 standard.

It might an I2S device in this case, but it could be an AC97 device in some 
other case.  It all depends on how the board is wired.  Do we really want to 
change the name of an SOC device based on what it's connected to?

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-09 Thread David Gibson
On Mon, Jan 07, 2008 at 06:28:54PM +, Mark Brown wrote:
 On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
  David Gibson wrote:
 
   Ok, but couldn't you strucutre your I2S or fabric driver so that it
   only becomes fully operational once the codec driver has registered
   with it?
 
  Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
  consideration for runtime-bindings and other OF goodies.  All connections 
  between the drivers are static, literally.  In fact, I wouldn't be 
  surprised if 
  some ASoC drivers cannot be compiled as modules.
 
 I'd just like to emphasise this point - ASoC v1 really doesn't
 understand the idea that the components of the sound subsystem might be
 probed separately.  It's set up to handle bare hardware with everything
 being probed from code in the machine/fabric driver.  This makes life
 very messy for platforms with something like the device tree.
 
 As has been said, handling this properly is one of the major motivations
 behind ASoC v2.

Ick.  Ok.

Nonetheless, messing up the device tree to workaround ASoC V1's silly
limitations is not a good idea.  The device tree must represent the
hardware as much as possible.  If that means we have to have a bunch
of platform-specific hacks to instatiate the drivers in the correct
order / combination, that's unfortunate, but there you go.

-- 
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


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-09 Thread Jon Smirl
On 1/9/08, David Gibson [EMAIL PROTECTED] wrote:
 On Mon, Jan 07, 2008 at 06:28:54PM +, Mark Brown wrote:
  On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
   David Gibson wrote:
 
Ok, but couldn't you strucutre your I2S or fabric driver so that it
only becomes fully operational once the codec driver has registered
with it?
 
   Not in ASoC V1.  You have to understand, ASoC V1 was designed without any
   consideration for runtime-bindings and other OF goodies.  All connections
   between the drivers are static, literally.  In fact, I wouldn't be 
   surprised if
   some ASoC drivers cannot be compiled as modules.
 
  I'd just like to emphasise this point - ASoC v1 really doesn't
  understand the idea that the components of the sound subsystem might be
  probed separately.  It's set up to handle bare hardware with everything
  being probed from code in the machine/fabric driver.  This makes life
  very messy for platforms with something like the device tree.
 
  As has been said, handling this properly is one of the major motivations
  behind ASoC v2.

 Ick.  Ok.

 Nonetheless, messing up the device tree to workaround ASoC V1's silly
 limitations is not a good idea.  The device tree must represent the
 hardware as much as possible.  If that means we have to have a bunch
 of platform-specific hacks to instatiate the drivers in the correct
 order / combination, that's unfortunate, but there you go.

ASOC v2 is sitting on a Wolfson server out of tree. I have been using
it for several months without problem. The pace of it being merged
could probably be sped up.


 --
 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



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-07 Thread Mark Brown
On Fri, Jan 04, 2008 at 08:43:17PM -0600, Timur Tabi wrote:
 Mark Brown wrote:

 In other words, ...

 clock1 = 0, bb8000
 clock2 = 1, 653230
 clock23 = 0, ab2372

Yes, something like that would cover it.  I'm not sure what is idiomatic
for the device tree.

  and of course the ordering matters.  

 Ok, you got me there.  But then, isn't this just another example where the 
 device tree is incapable of describing a complex configuration, and so we 
 need 
 a platform driver?

Yes, you could certainly do that - as you say, any device tree based
configuration would be optional so it's not a blocker if some things
aren't supported.

It'd be nice to have some idea of how to handle it should someone want
to do it but I wouldn't think it's essential.  The most common case
where specific ordering is required is that a PLL will need to have all
its inputs configured before the PLL is activated so it'd probably cover
a large proportion of cases to do that last.

  Indeed.  Providing the device tree stuff doesn't get set in stone I'm
  not sure we need to nail this down perfectly for ASoC v1 when we're
  running into trouble working around it.

 I definitely agree with that.  I'll be the first to admit that this driver, 
 much like ASoC V1, is a prototype.

Yes, from an ASoC point of view the driver looks good as it is.  The
only discussion is about the PowerPC probing stuff.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-07 Thread Mark Brown
On Sun, Jan 06, 2008 at 11:46:37AM +1100, David Gibson wrote:

 Ok, but couldn't you strucutre your I2S or fabric driver so that it
 only becomes fully operational once the codec driver has registered
 with it?

That's what ASoC v2 is doing, more or less (the core brings things on
line rather than drivers doing it).  For v1 so long as it doesn't cause
any problems in practice I'm not sure I'd worry about it too much.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-07 Thread Timur Tabi
David Gibson wrote:

 Ok, but couldn't you strucutre your I2S or fabric driver so that it
 only becomes fully operational once the codec driver has registered
 with it?

Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
consideration for runtime-bindings and other OF goodies.  All connections 
between the drivers are static, literally.  In fact, I wouldn't be surprised if 
some ASoC drivers cannot be compiled as modules.

So all I'm trying to do now is get my driver, with warts and all, into the tree 
so that I can focus with Liam et al to get a real ASoC V2 up and running.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-07 Thread Mark Brown
On Mon, Jan 07, 2008 at 09:52:03AM -0600, Timur Tabi wrote:
 David Gibson wrote:

  Ok, but couldn't you strucutre your I2S or fabric driver so that it
  only becomes fully operational once the codec driver has registered
  with it?

 Not in ASoC V1.  You have to understand, ASoC V1 was designed without any 
 consideration for runtime-bindings and other OF goodies.  All connections 
 between the drivers are static, literally.  In fact, I wouldn't be surprised 
 if 
 some ASoC drivers cannot be compiled as modules.

I'd just like to emphasise this point - ASoC v1 really doesn't
understand the idea that the components of the sound subsystem might be
probed separately.  It's set up to handle bare hardware with everything
being probed from code in the machine/fabric driver.  This makes life
very messy for platforms with something like the device tree.

As has been said, handling this properly is one of the major motivations
behind ASoC v2.

 So all I'm trying to do now is get my driver, with warts and all, into the 
 tree 
 so that I can focus with Liam et al to get a real ASoC V2 up and running.

This is certainly the approach we want to take from an ASoC point of
view.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-07 Thread Timur Tabi
Liam Girdwood wrote:
 On Mon, 2008-01-07 at 09:52 -0600, Timur Tabi wrote:
 So all I'm trying to do now is get my driver, with warts and all, into the 
 tree 
 so that I can focus with Liam et al to get a real ASoC V2 up and running.

 
 I'll commit the MPC8610 into the ASoC (v1) dev tree soon (hopefully
 tonight). This will allow folks to use it in the meantime whilst we sort
 out any changes.

I'm working on some minor updates, so hold off for now.  I'll post a new patch 
later this afternoon.

 I'll then port (what I can) to V2, although I may need some assistance
 with some of the PPC sections.

I'll be 100% available for that.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-07 Thread Liam Girdwood
On Mon, 2008-01-07 at 09:52 -0600, Timur Tabi wrote:
 
 So all I'm trying to do now is get my driver, with warts and all, into the 
 tree 
 so that I can focus with Liam et al to get a real ASoC V2 up and running.
 

I'll commit the MPC8610 into the ASoC (v1) dev tree soon (hopefully
tonight). This will allow folks to use it in the meantime whilst we sort
out any changes.

I'll then port (what I can) to V2, although I may need some assistance
with some of the PPC sections.

Fwiw we are looking at submitting V2 in March/April time. 


Liam

PS. Sorry for the silence lately. We've just moved to a new opensource
server over the holidays and have been without some services i.e. mail.



Privacy  Confidentiality Notice
-
This message and any attachments contain privileged and confidential 
information that is intended solely for the person(s) to whom it is addressed. 
If you are not an intended recipient you must not: read; copy; distribute; 
discuss; take any action in or make any reliance upon the contents of this 
message; nor open or read any attachment. If you have received this message in 
error, please notify us as soon as possible on the following telephone number 
and destroy this message including any attachments. Thank you.
-
Wolfson Microelectronics plc
Tel: +44 (0)131 272 7000
Fax: +44 (0)131 272 7001
Web: www.wolfsonmicro.com

Registered in Scotland

Company number SC089839

Registered office: 

Westfield House, 26 Westfield Road, Edinburgh, EH11 2QB, UK

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-05 Thread David Gibson
On Thu, Jan 03, 2008 at 09:54:45AM -0500, Jon Smirl wrote:
 On 1/2/08, David Gibson [EMAIL PROTECTED] wrote:
[snip]
  Instantiating the fabric driver off any node is wrong, precisely
  because it is an abstraction.  The fabric driver should be
  instantiated by the platform code.
 
 Instantiating it from the platform code forces me to put it either the
 of_platform_bus or the platform_bus since there aren't any other buses
 around when the platform code runs. Platform bus doesn't implement
 dynamic module loading. So that means it has to go onto the
 of_platform_bus. That implies that is it a pseudo-device without a
 pseudo-device entry in the device tree which is fine with me. I'll
 need to poke around in the of_bus code and see if the driver will load
 without a device tree entry.

You're letting implementation warts influence basic design decisions.
This is not sensible.  Step back and think for a moment, work out a
sane organization *then* think how you might need to fix or workaround
limitations of existing infrastructure.

 A simple fix to this would be to let me instantiate the driver off
 from the root node of the tree. That's the conceptually correct place
 for instantiating a driver that extends the platform code. Should I
 try adjusting the of probing code to pass the node in, or are there
 major objections?

The current probing system can't instantiate a device for the root
node in any sane way, since it takes a list of suitable busses.

The constructor based approach we're looking at implementing could do
it.  It should, in any case, be constructing a platform_device - so
the platform bus code would still need to be extended to handle the
module loading.  Creating it as an of_platform device bound to the
root node might be a workable interim solution though.

of_platform_device simply does not *ever* make sense conceptually: the
type of struct device wrapper in use depends on the bus the device is
attached to, not on how we figured out the device was there.  OF can
potentially give information about any sort of device be it
simple-bus, i2c, PCI or whatever connected.

 Also, as others have pointed out, this driver is not an abstraction.
 It represents the mess of wires hooking the codec up to the jacks on
 the back panel and possibly GPIO pins that control the wiring. You
 need this because the pins on HD audio codecs are completely
 reconfigurable and the same chip can be wired in a thousand different
 ways. It lets you have a generic codec driver and the move the
 platform specific code out of the driver.

Well, abstraction is maybe not the right word.  But the point is
that the fabric driver doesn't represent a neatly isolated device with
well defined bus connections.  Instead it represents the tangle of
essentially every link between audio devices in the platform.  About
the clearest possible example of a true platform device (as opposed
to a device on some bus that just doesn't have any bus-specific
logic).

-- 
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


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-05 Thread David Gibson
On Fri, Jan 04, 2008 at 08:39:54PM -0600, Timur Tabi wrote:
 David Gibson wrote:
 
  And what distinction are you drawing between first and second
  here? 
 
 Oh, that's an easy one:  The CS4270 can work without an I2C or SPI 
 connection, 
 but it will never work without an I2S connection.
 
  Why would the I2S need to scan for codecs?  Wouldn't it be up to the
  codec driver to register with I2S?
 
 Not in ASoC V1.  The codec driver registers with ASoC, but the actual 
 connection to other devices (e.g. the I2S driver) is done either in the I2S 
 driver or in the fabric driver, depending on your mood.  And that connection 
 is done via a pointer to a structure in the codec driver.

Ok, but couldn't you strucutre your I2S or fabric driver so that it
only becomes fully operational once the codec driver has registered
with it?

-- 
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


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-04 Thread Mark Brown
On Fri, Jan 04, 2008 at 10:47:25AM +1100, David Gibson wrote:
 On Thu, Jan 03, 2008 at 12:16:19PM -0600, Timur Tabi wrote:

  I'm no expert on this, but I think from the PowerPC point-of-view,
  the *ideal* situation would be if the ASoC fabric driver were
  generic, maybe even part of ASoC itself, and everything it needed
  could be obtained from the device tree.

 Nice idea in principle, and may be the way to go ultimately, but very
 tricky in practice.  The whole reason the fabric driver concept exists
 (from other archs) is that there are an awful lot of variants on how
 to wire the sound components together.  Devising a way of expressing
 those connections in the device tree that's sufficient will be very
 curly.  Then we'd have to build the fabric driver that can parse and
 process them all.

Yes, there's an issue with complexity here.  Some of the individual
components are going to have quite a lot of different things to
configure by themselves even for static use and the choices made may
depend on the usage at run time rather than being a static property of
the hardware.  It's also more than just connections - many machine
drivers will provide control for components like analogue switches or
simple amplifiers controlled through GPIO lines or memory mapped
registers (these are generally specific to the board).

As a result I would expect that you will always have systems using
platform based drivers.  I don't think that this is a bad thing -
something that can completely replace them would be able to do anything
that can be done in C in the kernel.

And then, people will no doubt produce device trees
 with errors in the connection information, so we'll still need
 platform-specific workarounds.

The other concern with this is that it risks turning the interface to
the codec and controller drivers into an ABI which isn't expected at the
minute and might cause problems in the future.  At the minute the
drivers export constants to their users defining the parameters they
can configure and (for things like source selection) the possible
values.  These can currently be changed at will and there's no great
consistency in their values between drivers.

There would also be difficulties in writing the device tree - without
the symbolic names you're going to end up with strings of numeric
constants in the device tree which are not going to be terribly readable
and will be error prone.

 If we want sound working any time soon, we'll want to stick with the
 platform based fabric drivers for the time being.

Like I say, I would expect that you're always going to want to have
platform based drivers.  Even if a given board can be represented in a
device tree some users will find it more straightforward or convenient
to write C code for their platform and have the device tree specify more
basic configuration options that correspond to the things they want to
vary between boards.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-04 Thread Timur Tabi
Grant Likely wrote:
 On 1/3/08, Scott Wood [EMAIL PROTECTED] wrote:
 Grant Likely wrote:
 On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

 Why not be a child of the i2c bus with a phandle to the ssi bus?
 Because when I probe the SSI node, I want to know what the attached codec 
 is.
   So if anything, I would need a pointer from the SSI bus *to* the 
 respective
 child on the I2C bus.
 That's fine too (it's what is done with Ethernet PHYs).  My preference
 is the other way around, but it's not a big issue in this case.
 I'd just link in both directions, and let software follow it in
 whichever direction it prefers.
 
 Gah!  Don't do that!  Then you need to maintain both directions in the
 dts file. 

So?  What's wrong with that?

 Software is good at generating reverse mappings. 

What software would that be?  Currently, there is no software that will do 
that?  Or are you saying that you want my driver to search the entire device 
tree until it finds the reverse mapping?  I don't think *that* is a good idea.

 Don't put
 that burden on the dts author. 

As the DTS author in question, I hereby declare that such a requirement is not 
a burden in the slightest.  Thank you.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-04 Thread Timur Tabi
David Gibson wrote:

 And what distinction are you drawing between first and second
 here? 

Oh, that's an easy one:  The CS4270 can work without an I2C or SPI connection, 
but it will never work without an I2S connection.

 Why would the I2S need to scan for codecs?  Wouldn't it be up to the
 codec driver to register with I2S?

Not in ASoC V1.  The codec driver registers with ASoC, but the actual 
connection to other devices (e.g. the I2S driver) is done either in the I2S 
driver or in the fabric driver, depending on your mood.  And that connection 
is done via a pointer to a structure in the codec driver.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-04 Thread Timur Tabi
Mark Brown wrote:

 Each individual call to set_sysclk() only takes three parameters but it
 can be called repeatedly and some configurations are going to require
 this. 

In other words, ...

clock1 = 0, bb8000
clock2 = 1, 653230
clock23 = 0, ab2372

 and of course the ordering matters.  

Ok, you got me there.  But then, isn't this just another example where the 
device tree is incapable of describing a complex configuration, and so we need 
a platform driver?


 Indeed.  Providing the device tree stuff doesn't get set in stone I'm
 not sure we need to nail this down perfectly for ASoC v1 when we're
 running into trouble working around it.

I definitely agree with that.  I'll be the first to admit that this driver, 
much like ASoC V1, is a prototype.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-04 Thread Grant Likely
On 1/4/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:
  Don't put
  that burden on the dts author.

 As the DTS author in question, I hereby declare that such a requirement is not
 a burden in the slightest.  Thank you.

Dude, you work for *Freescale*.  The set of dts authors affected
include every engineer writing a device tree for a board that uses
this part.  :-)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread David Gibson
On Wed, Jan 02, 2008 at 12:12:00PM -0500, Jon Smirl wrote:
 On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
  On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
   On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
   mpc8610_hpcd is the harder one to load since it doesn't have a device
   tree entry. What you want to do it match on the compatible field of
   the root node.
  
   static struct of_device_id fabric_of_match[] = {
   {
   .compatible = fsl,MPC8610HPCD,
   },
   {},
   };
  
   But this doesn't work since the root is the device tree isn't passed
   down into the device probe code. (Could this be fixed?)
 
  The driver can always get the root node.  But better yet, instantiate
  the correct fabric device (probably as a platform_device) from the
  platform code.  Then the correct fabric driver can probe against it.
 
 The meaning of this has finally sunk into my consciousness. The
 platform code can create a device that isn't bound to a driver. So why
 not make this an of_platform_device?  This is basically a pseudo
 device that isn't in the device tree.
 
 Alternatively, the best place for this device would be on the ASOC
 bus, but the ASOC bus hasn't been created when the platform code runs.
 Maybe I can figure out a place in the platform code to create this
 device after the ASOC driver has loaded and created the bus. Does the
 platform code get control back after loading all of the device
 drivers?
 
 In the longer term I'd like to kill platform_bus on powerpc and only
 use of_platform_bus. Platform_bus seems to be functioning like a
 catch-all and collecting junk from lots of different platforms.

Not going to happen.  of_platform_bus is not the right solution, and
in fact we're looking at moving (gradually) away from using
of_platform_bus, and instead using platform devices (along with the
device node being available for *any* struct device via the
arch_sysdata).

-- 
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


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread David Gibson
On Wed, Jan 02, 2008 at 09:29:57AM -0600, Timur Tabi wrote:
 Jon Smirl wrote:
  On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
   sound/soc/fsl/fsl_ssi.c  |  614 +++
   sound/soc/fsl/fsl_ssi.h  |  224 +++
  
  I'm confused about this part. You built a driver for the mpc8610 ssi
  port.  This port has a device tree entry.
  
  +   [EMAIL PROTECTED] {
  +   compatible = fsl,ssi;
  +   cell-index = 0;
  +   reg = 16000 100;
  +   interrupt-parent = mpic;
  +   interrupts = 3e 2;
  +   fsl,mode = i2s-slave;
  +   codec {
  +   compatible = cirrus,cs4270;
  +   /* MCLK source is a stand-alone oscillator */
  +   bus-frequency = bb8000;
  +   };
  +   };
  
  But then you don't create an of_platform_driver for this device.
  Instead you create one for the fabric driver, struct
  of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
  driver into it.
 
 That's the best plan I came up with.  This is apparently fixed in ASoC 
 V2.  From ASoC V1's perspective, the fabric driver must be the master. 
 However, it doesn't make sense to have a node in the device tree for the 
 fabric driver, because there is no such device.  The fabric driver is 
 an abstraction.  So I need to chose some other node to probe the fabric 
 driver with.  I chose the SSI, since each SSI can have only one
 codec.

Instantiating the fabric driver off any node is wrong, precisely
because it is an abstraction.  The fabric driver should be
instantiated by the platform code.

-- 
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


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Jon Smirl
On 1/2/08, David Gibson [EMAIL PROTECTED] wrote:
 On Wed, Jan 02, 2008 at 12:12:00PM -0500, Jon Smirl wrote:
  On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
   On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
mpc8610_hpcd is the harder one to load since it doesn't have a device
tree entry. What you want to do it match on the compatible field of
the root node.
   
static struct of_device_id fabric_of_match[] = {
{
.compatible = fsl,MPC8610HPCD,
},
{},
};
   
But this doesn't work since the root is the device tree isn't passed
down into the device probe code. (Could this be fixed?)
  
   The driver can always get the root node.  But better yet, instantiate
   the correct fabric device (probably as a platform_device) from the
   platform code.  Then the correct fabric driver can probe against it.
 
  The meaning of this has finally sunk into my consciousness. The
  platform code can create a device that isn't bound to a driver. So why
  not make this an of_platform_device?  This is basically a pseudo
  device that isn't in the device tree.
 
  Alternatively, the best place for this device would be on the ASOC
  bus, but the ASOC bus hasn't been created when the platform code runs.
  Maybe I can figure out a place in the platform code to create this
  device after the ASOC driver has loaded and created the bus. Does the
  platform code get control back after loading all of the device
  drivers?
 
  In the longer term I'd like to kill platform_bus on powerpc and only
  use of_platform_bus. Platform_bus seems to be functioning like a
  catch-all and collecting junk from lots of different platforms.

 Not going to happen.  of_platform_bus is not the right solution, and
 in fact we're looking at moving (gradually) away from using
 of_platform_bus, and instead using platform devices (along with the
 device node being available for *any* struct device via the
 arch_sysdata).

I do agree that of_platform_bus should have been derived from
platform_bus, not a separate structure. This is causing problems in
the ASLA code  where they want typed pointers.

In my little test patch, I disabled platform_bus on my MPC5200. This
generated some compiler errors which exposed a bunch of MPC83xxx and
Apple code that was getting compiled into my MPC5200 kernel.  These
were platform bus drivers that weren't properly ifdef'd.

So I guess my objection is more along the lines of getting rid of
driver code inside the arch directory and switching everything to
modules. Then we could periodically turn off platform bus on each
platform and make sure everything still builds.



 --
 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



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Jon Smirl
On 1/2/08, David Gibson [EMAIL PROTECTED] wrote:
 On Wed, Jan 02, 2008 at 09:29:57AM -0600, Timur Tabi wrote:
  Jon Smirl wrote:
   On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
sound/soc/fsl/fsl_ssi.c  |  614 +++
sound/soc/fsl/fsl_ssi.h  |  224 +++
  
   I'm confused about this part. You built a driver for the mpc8610 ssi
   port.  This port has a device tree entry.
  
   +   [EMAIL PROTECTED] {
   +   compatible = fsl,ssi;
   +   cell-index = 0;
   +   reg = 16000 100;
   +   interrupt-parent = mpic;
   +   interrupts = 3e 2;
   +   fsl,mode = i2s-slave;
   +   codec {
   +   compatible = cirrus,cs4270;
   +   /* MCLK source is a stand-alone oscillator */
   +   bus-frequency = bb8000;
   +   };
   +   };
  
   But then you don't create an of_platform_driver for this device.
   Instead you create one for the fabric driver, struct
   of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
   driver into it.
 
  That's the best plan I came up with.  This is apparently fixed in ASoC
  V2.  From ASoC V1's perspective, the fabric driver must be the master.
  However, it doesn't make sense to have a node in the device tree for the
  fabric driver, because there is no such device.  The fabric driver is
  an abstraction.  So I need to chose some other node to probe the fabric
  driver with.  I chose the SSI, since each SSI can have only one
  codec.

 Instantiating the fabric driver off any node is wrong, precisely
 because it is an abstraction.  The fabric driver should be
 instantiated by the platform code.

Instantiating it from the platform code forces me to put it either the
of_platform_bus or the platform_bus since there aren't any other buses
around when the platform code runs. Platform bus doesn't implement
dynamic module loading. So that means it has to go onto the
of_platform_bus. That implies that is it a pseudo-device without a
pseudo-device entry in the device tree which is fine with me. I'll
need to poke around in the of_bus code and see if the driver will load
without a device tree entry.

A simple fix to this would be to let me instantiate the driver off
from the root node of the tree. That's the conceptually correct place
for instantiating a driver that extends the platform code. Should I
try adjusting the of probing code to pass the node in, or are there
major objections?

Also, as others have pointed out, this driver is not an abstraction.
It represents the mess of wires hooking the codec up to the jacks on
the back panel and possibly GPIO pins that control the wiring. You
need this because the pins on HD audio codecs are completely
reconfigurable and the same chip can be wired in a thousand different
ways. It lets you have a generic codec driver and the move the
platform specific code out of the driver.



 --
 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



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Jon Smirl wrote:
 On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:
 On 1/1/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
 +   [EMAIL PROTECTED] {
 +   compatible = fsl,ssi;
 +   cell-index = 0;
 +   reg = 16000 100;
 +   interrupt-parent = mpic;
 +   interrupts = 3e 2;
 +   fsl,mode = i2s-slave;
 +   codec {
 +   compatible = cirrus,cs4270;
 +   /* MCLK source is a stand-alone 
 oscillator */
 +   bus-frequency = bb8000;
 +   };
 +   };
 Does this need to be bus-frequency? It's always called MCLK in all of
 the literature.

 In my case the MCLK comes from a chip on the i2c bus that is
 programmable How would that be encoded?.
 Looking at the cs4270 codec driver it is controlled by i2c (supports
 SPI too).  What happened to the conversation about putting codecs on
 the controlling bus and then linking them to the data bus?
 The current CS4270 driver doesn't support device trees.  When I wrote
 it, the idea of putting I2C info in the device tree was not finalized,
 and since the driver is supposed to be cross-platform, I decided to do
 it the old-fashioned way.  Before I update the code, however, I'm
 waiting for:

 1) The current code to be accepted into the tree
 2) ASoC is updated to V2
 3) The current drivers are updated to support ASoC V2.
 
 I've been trying to get the i2c code in for two months. Hopefully it
 will go in soon, no one had made any comments on it recently. Have you
 tried your code with it?

No.  I don't like updating my patches with new features while they're 
undergoing review.  If something is clearly wrong with the patch, then I'll 
fix it and resubmit.  But I really don't like to support new stuff just 
because it's there.

 There is nothing stopping your from putting a node for the CS4270 on
 the i2c bus today. It just won't trigger the loading of the driver.

Yes, the thing that's stopping me is that I don't want to do 20 things at 
once.  I already have pending patches that I'm trying to get in.  Once those 
are in, then I will consider additional work.

 Don't we want to follow the device tree policy of putting the device
 on the controlling bus and then link it to the data bus?

Normally, that sounds like a good idea, but the cs4270 is an I2S device first, 
and an I2C device second.  I need to be able to find that codec from the I2S 
node.  My I2S driver would not know to scan all I2C devices to find the codec.

 It makes it a little easier but it doesn't fix everything. We need to
 start looking at it since none of the example driver for it are device
 tree based.

I will look at it, *after* my current V1 driver has been applied to the tree.

  It still has problems with wanting 'struct
 platform_device' when we have 'struct of_platform_device' pointers. It
 also doesn't know how to dynamically load codecs based on device
 trees.

I agree that these things need to be fixed.  I look forward to thinking about 
these problems, *after* my V1 patches have been applied.

 Liam messed up all of my code when he refactored it in late December.

Bummer.

 I've switched over to the current SOC code for the moment. The big
 thing that v2 fixes is that SOC is changed to being a subsystem
 instead of platform driver. Being a subsystem is the correct model.
 
 It would be good if more pieces of v2 get push forward. Then we can
 sort out the device tree issues in it.

I agree.

 Adding the second device tree node doesn't have anything to do with
 ASOC v2. It's specific to powerpc and device trees.

Ok, but making my CS4270 driver device-tree aware is a completely separate 
task from what this patchset is addressing.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Jon Smirl wrote:

 For this model to work you need to split your driver. fsl-ssi and
 mpc8610_hpcd need to be in  two separate drivers. 

They are two separate drivers.  sound/soc/fsl/fsl_ssi.c and 
sound/soc/fsl/mpc8610_hpcd.c

  fsl-ssi  is easy
 enough to load since it has a device tree entry.
 
 mpc8610_hpcd is the harder one to load since it doesn't have a device
 tree entry. What you want to do it match on the compatible field of
 the root node.
 
 static struct of_device_id fabric_of_match[] = {
   {
   .compatible = fsl,MPC8610HPCD,
   },
   {},
 };
 
 But this doesn't work since the root is the device tree isn't passed
 down into the device probe code. (Could this be fixed?)

I don't understand that sentence.  Is there a typo?

 Instead we could make the separated mpc8610_hpcd fabric driver attach
 to fsl,ssi.
 
 static struct of_device_id fabric_of_match[] = {
   {
   .compatible = fsl,ssi,
   },
   {},
 };
 
 Then in it's probe code check for the right platform.

That's what I do.  I attach to fsl,ssi, gather the information from the device 
tree, and then call a private API to initialize the SSI driver.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Grant Likely wrote:

 Why not be a child of the i2c bus with a phandle to the ssi bus? 

Because when I probe the SSI node, I want to know what the attached codec is. 
  So if anything, I would need a pointer from the SSI bus *to* the respective 
child on the I2C bus.

I know little about platform devices/drivers, so I don't know how to use them.

Currently, I have a design flaw in my driver in that if I have two SSIs, and 
each one is attached to a CS4270, I don't have any way of making sure that the 
right CS4270 is using the right I2C address.  I'm guessing that if I switch to 
a platform-based model, I can resolve this issue.  But for now, the CS4270 
doesn't support that, so that patchset I have submitted works with what I 
have.  After my patchset has been applied, I'll be more than happy to look 
into updating the CS4270 (and everything else) to use the platform model for 
I2C.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Grant Likely wrote:

 Does that mean with ASoC V2 you can instantiate it with the board
 specific platform code instead? 

I don't know.  I haven't really looked at V2 yet.  You'll have to ask Liam 
Girdwood.

 This is one of the examples of where the compatible properties are
 trying to be far to generic about what they are.  How do you define
 what fsl,ssi is? 

The SSI is a specific Freescale device, so I think it's pretty well defined.

 What happens when Freescale produces another
 peripheral that can do ssi but isn't register level compatible?

It won't be called the SSI.  It will be called something else.

 In my opinion, it is far better to be specific in the device tree and
 teach the driver about what versions it is able to bind against.  In
 this case, I would use fsl,mpc8610-ssi or maybe better yet:
 fsl,mpc8610-ssi,i2s (MPC8610 SSI device in I2S mode).

I can work with that, but the SSI could be placed into any future 83xx, 85xx, 
or 86xx SOC, and the driver will still work with it as-is.

 I don't like the idea of a separate fsl,mode property to describe the
 behaviour of multifunction peripherals.  It makes probing more
 difficult when there is a different driver for each mode.

Can you propose an alternative?  The driver needs to know what mode to use 
when communicating with its codec.  How am I supposed to know if I have an I2S 
codec or an AC97 codec?

 The fabric driver is specific to the board.  So you should be using
 Kconfig to select the fabric driver.  There is no node in the device
 tree for fabric drivers.  I thought that was the consensus.
 
 No, the desire is to go multiplatform in ppc.  That means you cannot
 use Kconfig to select the correct fabric driver.

I don't see any way of avoiding this with ASoC V1.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Grant Likely
On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:
  Don't we want to follow the device tree policy of putting the device
  on the controlling bus and then link it to the data bus?

 Normally, that sounds like a good idea, but the cs4270 is an I2S device first,
 and an I2C device second.  I need to be able to find that codec from the I2S
 node.  My I2S driver would not know to scan all I2C devices to find the codec.

The device tree is a description of the hardware; not software.  It's
not a good idea to break with convention due to current driver
architecture.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Mark Brown wrote:

 The machine support code (fabric driver in PowerPC terms, I think?)
 tells the core how everything is connected together by registering
 devices representing the links (eg, I2S) between the codecs, CPU and
 other devices.  The ASoC core is then responsible for ensuring that all
 the required components are present before it registers with the ALSA
 core.

I'm no expert on this, but I think from the PowerPC point-of-view, the *ideal* 
situation would be if the ASoC fabric driver were generic, maybe even part of 
ASoC itself, and everything it needed could be obtained from the device tree.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
David Gibson wrote:

 Instantiating the fabric driver off any node is wrong, precisely
 because it is an abstraction.  The fabric driver should be
 instantiated by the platform code.

Can you tell me how to do that?


-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Grant Likely
On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

  Why not be a child of the i2c bus with a phandle to the ssi bus?

 Because when I probe the SSI node, I want to know what the attached codec is.
   So if anything, I would need a pointer from the SSI bus *to* the respective
 child on the I2C bus.

That's fine too (it's what is done with Ethernet PHYs).  My preference
is the other way around, but it's not a big issue in this case.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Grant Likely wrote:

 The device tree is a description of the hardware; not software.  It's
 not a good idea to break with convention due to current driver
 architecture.

I believe that with ASoC V1, I'm stuck between a rock and a hard place, and so 
the only way to make this code work is to bend some rules.  Right now, the 
CS4270 driver does not support platform drivers or the device tree, so there's 
no point in putting a child I2C node for it.  As I mentioned in other posts, I 
will be more than happy to update the CS4270 driver to support this new 
paradigm (which was invented after the CS4270 driver was written) *after* this 
current patchset is applied.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Mark Brown wrote:

 clock1 = 0, bb8000
 
 Would that be better?
 
 To cover everything you'd need to be able to specify all the clocking
 parameters, especially a PLL configuration, and also specify more than
 one of each item.  Even then you'd still have problems like...

The ASoC V1 API for communicating clock data from the fabric driver to the 
codec driver only allows for three parameters.

 According to the documentation in your patch the bus frequency should
 already be optional 

My code does not currently support that configuration, and I don't have any 
hardware that works that way, so I don't know what it would look like.  I'm 
just trying to make the driver as flexible as possible, given ASoC V1 
constraints.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Grant Likely
On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

  Does that mean with ASoC V2 you can instantiate it with the board
  specific platform code instead?

 I don't know.  I haven't really looked at V2 yet.  You'll have to ask Liam
 Girdwood.

  This is one of the examples of where the compatible properties are
  trying to be far to generic about what they are.  How do you define
  what fsl,ssi is?

 The SSI is a specific Freescale device, so I think it's pretty well defined.

  What happens when Freescale produces another
  peripheral that can do ssi but isn't register level compatible?

 It won't be called the SSI.  It will be called something else.

Heh, I've seen enough to know that it's virtually impossible for a
company to maintain a consistent naming scheme all the time.  Better
to be specific now and add generic names sometime in the future rather
than the other way around.

  In my opinion, it is far better to be specific in the device tree and
  teach the driver about what versions it is able to bind against.  In
  this case, I would use fsl,mpc8610-ssi or maybe better yet:
  fsl,mpc8610-ssi,i2s (MPC8610 SSI device in I2S mode).

 I can work with that, but the SSI could be placed into any future 83xx, 85xx,
 or 86xx SOC, and the driver will still work with it as-is.

The have the device trees claim compatibility with the older
fsl,mpc8610-ssi device specifically.  ie: compatible =
fsl,mpc83whatever-ssi,ac97, fsl,mpc8610-ssi,ac97;


  I don't like the idea of a separate fsl,mode property to describe the
  behaviour of multifunction peripherals.  It makes probing more
  difficult when there is a different driver for each mode.

 Can you propose an alternative?  The driver needs to know what mode to use
 when communicating with its codec.  How am I supposed to know if I have an I2S
 codec or an AC97 codec?

Make the compatible property tell you!  :-)  If it's connected to an
I2S codec, then it could be compatible = fsl,mpc8610-ssi,i2s.  Or
for AC7, compatible = fsl,mpc8610-ssi,ac97

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Grant Likely
On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

  The device tree is a description of the hardware; not software.  It's
  not a good idea to break with convention due to current driver
  architecture.

 I believe that with ASoC V1, I'm stuck between a rock and a hard place, and so
 the only way to make this code work is to bend some rules.  Right now, the
 CS4270 driver does not support platform drivers or the device tree, so there's
 no point in putting a child I2C node for it.  As I mentioned in other posts, I
 will be more than happy to update the CS4270 driver to support this new
 paradigm (which was invented after the CS4270 driver was written) *after* this
 current patchset is applied.

If you need to bend rules, then do it in a place where it won't bite
us in the butt down the road.  (ie. not with the device tree).  Do
hacky stuff in the platform code if you need to because it can be
changed easily down the road.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Grant Likely
On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

  Make the compatible property tell you!  :-)  If it's connected to an
  I2S codec, then it could be compatible = fsl,mpc8610-ssi,i2s.  Or
  for AC7, compatible = fsl,mpc8610-ssi,ac97

 That won't work.  There are too many variations.  I think a separate property
 just makes more sense.  Frankly, I don't see what's wrong with it.

Sure it will, that's exactly what I'm doing with the 5200, but I won't
argue the point.  My *opinion* is that using compatible is a more
elegant approach for this type of multifunction device, but using a
mode property is neither wrong or bad.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Scott Wood
Grant Likely wrote:
 On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Grant Likely wrote:

 Why not be a child of the i2c bus with a phandle to the ssi bus?
 Because when I probe the SSI node, I want to know what the attached codec is.
   So if anything, I would need a pointer from the SSI bus *to* the respective
 child on the I2C bus.
 
 That's fine too (it's what is done with Ethernet PHYs).  My preference
 is the other way around, but it's not a big issue in this case.

I'd just link in both directions, and let software follow it in 
whichever direction it prefers.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Timur Tabi
Grant Likely wrote:

 Make the compatible property tell you!  :-)  If it's connected to an
 I2S codec, then it could be compatible = fsl,mpc8610-ssi,i2s.  Or
 for AC7, compatible = fsl,mpc8610-ssi,ac97

That won't work.  There are too many variations.  I think a separate property 
just makes more sense.  Frankly, I don't see what's wrong with it.



-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Scott Wood
Grant Likely wrote:
 On 1/3/08, Scott Wood [EMAIL PROTECTED] wrote:
 I'd just link in both directions, and let software follow it in
 whichever direction it prefers.
 
 Gah!  Don't do that!  Then you need to maintain both directions in the
 dts file.  Software is good at generating reverse mappings.

Software is, however, lousy at correctly wading through 
poorly-structured data (which device trees are full of) to figure out 
how to locate the link it wants to follow backwards.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Grant Likely
On 1/3/08, Scott Wood [EMAIL PROTECTED] wrote:
 Grant Likely wrote:
  On 1/3/08, Timur Tabi [EMAIL PROTECTED] wrote:
  Grant Likely wrote:
 
  Why not be a child of the i2c bus with a phandle to the ssi bus?
  Because when I probe the SSI node, I want to know what the attached codec 
  is.
So if anything, I would need a pointer from the SSI bus *to* the 
  respective
  child on the I2C bus.
 
  That's fine too (it's what is done with Ethernet PHYs).  My preference
  is the other way around, but it's not a big issue in this case.

 I'd just link in both directions, and let software follow it in
 whichever direction it prefers.

Gah!  Don't do that!  Then you need to maintain both directions in the
dts file.  Software is good at generating reverse mappings.  Don't put
that burden on the dts author.  (the software principle of defining
things in one place only applies here)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Mark Brown
On Thu, Jan 03, 2008 at 12:23:08PM -0600, Timur Tabi wrote:
 Mark Brown wrote:

  To cover everything you'd need to be able to specify all the clocking
  parameters, especially a PLL configuration, and also specify more than
  one of each item.  Even then you'd still have problems like...

 The ASoC V1 API for communicating clock data from the fabric driver to the 
 codec driver only allows for three parameters.

Each individual call to set_sysclk() only takes three parameters but it
can be called repeatedly and some configurations are going to require
this.  There's also the set_pll() call which will be required by some
things too (and again that can support multiple PLLs).  

For example, something like this isn't unknown:

 - Set PLL input to pin A.
 - Configure PLL input/output frequencies.
 - Set codec system clock source to be the PLL

and of course the ordering matters.  You can also have other dividers
and clock sources within the codec which need configuring and other
components outside the codec which need configuring to supply the clocks
to the codec.

  According to the documentation in your patch the bus frequency should
  already be optional 

 My code does not currently support that configuration, and I don't have any 
 hardware that works that way, so I don't know what it would look like.  I'm 
 just trying to make the driver as flexible as possible, given ASoC V1 
 constraints.

Indeed.  Providing the device tree stuff doesn't get set in stone I'm
not sure we need to nail this down perfectly for ASoC v1 when we're
running into trouble working around it.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread Mark Brown
On Thu, Jan 03, 2008 at 01:18:31PM -0600, Scott Wood wrote:
 Grant Likely wrote:

  Gah!  Don't do that!  Then you need to maintain both directions in the
  dts file.  Software is good at generating reverse mappings.

 Software is, however, lousy at correctly wading through 
 poorly-structured data (which device trees are full of) to figure out 
 how to locate the link it wants to follow backwards.

Thinking about that from an ASoC v2 perspective the approach that this
immediately suggests is to represent the links between the devices in
the device tree and then have those links reference the attached
devices.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-03 Thread David Gibson
On Thu, Jan 03, 2008 at 12:16:19PM -0600, Timur Tabi wrote:
 Mark Brown wrote:
 
  The machine support code (fabric driver in PowerPC terms, I think?)
  tells the core how everything is connected together by registering
  devices representing the links (eg, I2S) between the codecs, CPU and
  other devices.  The ASoC core is then responsible for ensuring that all
  the required components are present before it registers with the ALSA
  core.
 
 I'm no expert on this, but I think from the PowerPC point-of-view,
 the *ideal* situation would be if the ASoC fabric driver were
 generic, maybe even part of ASoC itself, and everything it needed
 could be obtained from the device tree.

Nice idea in principle, and may be the way to go ultimately, but very
tricky in practice.  The whole reason the fabric driver concept exists
(from other archs) is that there are an awful lot of variants on how
to wire the sound components together.  Devising a way of expressing
those connections in the device tree that's sufficient will be very
curly.  Then we'd have to build the fabric driver that can parse and
process them all.  And then, people will no doubt produce device trees
with errors in the connection information, so we'll still need
platform-specific workarounds.

If we want sound working any time soon, we'll want to stick with the
platform based fabric drivers for the time being.

-- 
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


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Timur Tabi
Jon Smirl wrote:
 On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
 +   [EMAIL PROTECTED] {
 +   compatible = fsl,ssi;
 +   cell-index = 0;
 +   reg = 16000 100;
 +   interrupt-parent = mpic;
 +   interrupts = 3e 2;
 +   fsl,mode = i2s-slave;
 +   codec {
 +   compatible = cirrus,cs4270;
 +   /* MCLK source is a stand-alone oscillator */
 +   bus-frequency = bb8000;
 +   };
 +   };
 
 Does this need to be bus-frequency? It's always called MCLK in all of
 the literature.

I'm trying to make this node as generic as possible.  The fabric driver 
is the one that will parse this node and pass the data to the codec 
driver, so I can't use any codec-specific terms.

The API from the fabric driver for passing clock information includes a 
clock ID, a direction, and a frequency.  I can do something like this:

clock1 = 0, bb8000

Would that be better?

 
 In my case the MCLK comes from a chip on the i2c bus that is
 programmable How would that be encoded?.

I'm going under the assumption that MCLK does not change once the board 
is up and running.  In your case, you'd need to do something quite 
different, because you're not reading the clock info from the device 
tree and passing it to the codec at initialization once.  If you want to 
define an extension to the 'codec' child node that handles that, I'll 
add it to the documentation.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Timur Tabi
Jon Smirl wrote:
 On 1/1/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
 +   [EMAIL PROTECTED] {
 +   compatible = fsl,ssi;
 +   cell-index = 0;
 +   reg = 16000 100;
 +   interrupt-parent = mpic;
 +   interrupts = 3e 2;
 +   fsl,mode = i2s-slave;
 +   codec {
 +   compatible = cirrus,cs4270;
 +   /* MCLK source is a stand-alone oscillator 
 */
 +   bus-frequency = bb8000;
 +   };
 +   };
 Does this need to be bus-frequency? It's always called MCLK in all of
 the literature.

 In my case the MCLK comes from a chip on the i2c bus that is
 programmable How would that be encoded?.
 
 Looking at the cs4270 codec driver it is controlled by i2c (supports
 SPI too).  What happened to the conversation about putting codecs on
 the controlling bus and then linking them to the data bus?

The current CS4270 driver doesn't support device trees.  When I wrote 
it, the idea of putting I2C info in the device tree was not finalized, 
and since the driver is supposed to be cross-platform, I decided to do 
it the old-fashioned way.  Before I update the code, however, I'm 
waiting for:

1) The current code to be accepted into the tree
2) ASoC is updated to V2
3) The current drivers are updated to support ASoC V2.

I think ASoC V2 will make it easier to support device trees, but I'm not 
ready yet for that.

 If that's the case the cs4270 should be in the i2c bus node (missing
 currently) and then a link from the SSI bus would point to it.

The CS4270 is a child of both the I2C bus *and* the SSI bus.  It needs 
to have two nodes, one under each.  Your're right in that there needs to 
be a link, but until the code is updated to ASoC V2, I think it's 
premature to add that support.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Timur Tabi
Jon Smirl wrote:
 On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
  sound/soc/fsl/fsl_ssi.c  |  614 +++
  sound/soc/fsl/fsl_ssi.h  |  224 +++
 
 I'm confused about this part. You built a driver for the mpc8610 ssi
 port.  This port has a device tree entry.
 
 + [EMAIL PROTECTED] {
 + compatible = fsl,ssi;
 + cell-index = 0;
 + reg = 16000 100;
 + interrupt-parent = mpic;
 + interrupts = 3e 2;
 + fsl,mode = i2s-slave;
 + codec {
 + compatible = cirrus,cs4270;
 + /* MCLK source is a stand-alone oscillator */
 + bus-frequency = bb8000;
 + };
 + };
 
 But then you don't create an of_platform_driver for this device.
 Instead you create one for the fabric driver, struct
 of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
 driver into it.

That's the best plan I came up with.  This is apparently fixed in ASoC 
V2.  From ASoC V1's perspective, the fabric driver must be the master. 
However, it doesn't make sense to have a node in the device tree for the 
fabric driver, because there is no such device.  The fabric driver is 
an abstraction.  So I need to chose some other node to probe the fabric 
driver with.  I chose the SSI, since each SSI can have only one codec.

 
 +static struct of_device_id mpc8610_hpcd_match[] = {
 + {
 + .compatible = fsl,ssi,
 + },
 + {}
 +};
 +MODULE_DEVICE_TABLE(of, mpc8610_hpcd_match);
 +
 +static struct of_platform_driver mpc8610_hpcd_of_driver = {
 + .owner  = THIS_MODULE,
 + .name   = mpc8610_hpcd,
 + .match_table= mpc8610_hpcd_match,
 + .probe  = mpc8610_hpcd_probe,
 + .remove = mpc8610_hpcd_remove,
 +};
 
 static int mpc8610_hpcd_probe(struct of_device *ofdev,
   const struct of_device_id *match)
 {
 .
   machine_data-dai.cpu_dai = fsl_ssi_create_dai(ssi_info);
 
 Isn't this two separate drivers that have been combined into one
 driver? Or does the fsl_ssi channel only work on the mpc8610_hpcd?

Sorry, I don't understand your question.

 This is the problem of knowing how to load the fabric driver that I
 was talking about in the other threads.

Yes, and the decision I made on this topic is to have the fabric driver 
probed on the SSI node.

For ASoC V1, I believe the problem is undefined and each driver should 
be implemented in whatever way works best.

  A device that can occur on
 more than one chip .compatible = fsl,ssi, is being used to pull in
 a platform specific fabric driver, mpc8610_hpcd. You can use the
 kernel config system to select the right driver for .compatible =
 fsl,ssi, that matches you hardware and compile it in.

Ok, I think I understand that.

 But that doesn't work in my environment. My generic channel is
 fsl,i2s. I have four different systems booting off from a shared
 network drive. Each of these systems needs the common fsl,i2s driver
 but they all four need different fabric drivers.

That, I don't understand.  fsl,ssi is pretty much the same thing as 
fsl,i2s, since the SSI *is* an I2S device.  It's also an AC97 device, 
which is why I added the fsl,mode property.

The fabric driver is specific to the board.  So you should be using 
Kconfig to select the fabric driver.  There is no node in the device 
tree for fabric drivers.  I thought that was the consensus.

Are you saying that you want to use the same kernel on four different 
systems?  If so, then you need to find a way to compile all fabric 
drivers together, and at boot time each fabric driver will decide 
whether it will do anything.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Jon Smirl
On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Are you saying that you want to use the same kernel on four different
 systems?  If so, then you need to find a way to compile all fabric
 drivers together, and at boot time each fabric driver will decide
 whether it will do anything.

Yes, I have four different but similar systems. They only differer in
the codec chips used. I want to make a single kernel image and then
use the device tree to dynamically load the correct codec driver from
initrd. That will let me ship a single kernel image that services all
four machines. The codecs implement different sound systems from low
end to high end.

The correct solution for this is to use kernel modules and trigger
their loading based on the device tree. This is the same mechanism
used by USB and PCI.

For this model to work you need to split your driver. fsl-ssi and
mpc8610_hpcd need to be in  two separate drivers. fsl-ssi  is easy
enough to load since it has a device tree entry.

mpc8610_hpcd is the harder one to load since it doesn't have a device
tree entry. What you want to do it match on the compatible field of
the root node.

static struct of_device_id fabric_of_match[] = {
{
.compatible = fsl,MPC8610HPCD,
},
{},
};

But this doesn't work since the root is the device tree isn't passed
down into the device probe code. (Could this be fixed?)

Instead we could make the separated mpc8610_hpcd fabric driver attach
to fsl,ssi.

static struct of_device_id fabric_of_match[] = {
{
.compatible = fsl,ssi,
},
{},
};

Then in it's probe code check for the right platform.

unsigned long node = of_get_flat_dt_root();
if (!of_flat_dt_is_compatible(node, fsl,MPC8610HPCD))
return 0;
.. activate the code ...

You also need a static flag to make sure you don't active the driver
more than once.

This isn't the best solution since my four fabric drivers will still
load and check what platform they are on before exiting but at least
it works.

-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Grant Likely
On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:
  If that's the case the cs4270 should be in the i2c bus node (missing
  currently) and then a link from the SSI bus would point to it.

 The CS4270 is a child of both the I2C bus *and* the SSI bus.  It needs
 to have two nodes, one under each.  Your're right in that there needs to
 be a link, but until the code is updated to ASoC V2, I think it's
 premature to add that support.

Why not be a child of the i2c bus with a phandle to the ssi bus?  That
is the direction we've gone with other multi attachment devices.  (ie.
Ethernet PHYs.  Child of the MDIO node, phandle to link the Ethernet
controller with the PHY)

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Grant Likely
On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
 That's the best plan I came up with.  This is apparently fixed in ASoC
 V2.  From ASoC V1's perspective, the fabric driver must be the master.
 However, it doesn't make sense to have a node in the device tree for the
 fabric driver, because there is no such device.  The fabric driver is
 an abstraction.  So I need to chose some other node to probe the fabric
 driver with.  I chose the SSI, since each SSI can have only one codec.

Does that mean with ASoC V2 you can instantiate it with the board
specific platform code instead?  I think that is the consensus we were
leaning towards in the last discussion about this issue.

  But that doesn't work in my environment. My generic channel is
  fsl,i2s. I have four different systems booting off from a shared
  network drive. Each of these systems needs the common fsl,i2s driver
  but they all four need different fabric drivers.

 That, I don't understand.  fsl,ssi is pretty much the same thing as
 fsl,i2s, since the SSI *is* an I2S device.  It's also an AC97 device,
 which is why I added the fsl,mode property.

This is one of the examples of where the compatible properties are
trying to be far to generic about what they are.  How do you define
what fsl,ssi is?  What happens when Freescale produces another
peripheral that can do ssi but isn't register level compatible?

In my opinion, it is far better to be specific in the device tree and
teach the driver about what versions it is able to bind against.  In
this case, I would use fsl,mpc8610-ssi or maybe better yet:
fsl,mpc8610-ssi,i2s (MPC8610 SSI device in I2S mode).

I don't like the idea of a separate fsl,mode property to describe the
behaviour of multifunction peripherals.  It makes probing more
difficult when there is a different driver for each mode.


 The fabric driver is specific to the board.  So you should be using
 Kconfig to select the fabric driver.  There is no node in the device
 tree for fabric drivers.  I thought that was the consensus.

No, the desire is to go multiplatform in ppc.  That means you cannot
use Kconfig to select the correct fabric driver.

 Are you saying that you want to use the same kernel on four different
 systems?  If so, then you need to find a way to compile all fabric
 drivers together, and at boot time each fabric driver will decide
 whether it will do anything.

Yes!  That is exactly what we want to support in arch/powerpc.  Use
platform code to select the correct fabric driver.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Grant Likely
On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
 mpc8610_hpcd is the harder one to load since it doesn't have a device
 tree entry. What you want to do it match on the compatible field of
 the root node.

 static struct of_device_id fabric_of_match[] = {
 {
 .compatible = fsl,MPC8610HPCD,
 },
 {},
 };

 But this doesn't work since the root is the device tree isn't passed
 down into the device probe code. (Could this be fixed?)

The driver can always get the root node.  But better yet, instantiate
the correct fabric device (probably as a platform_device) from the
platform code.  Then the correct fabric driver can probe against it.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Jon Smirl
On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
 On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
  On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:
  mpc8610_hpcd is the harder one to load since it doesn't have a device
  tree entry. What you want to do it match on the compatible field of
  the root node.
 
  static struct of_device_id fabric_of_match[] = {
  {
  .compatible = fsl,MPC8610HPCD,
  },
  {},
  };
 
  But this doesn't work since the root is the device tree isn't passed
  down into the device probe code. (Could this be fixed?)

 The driver can always get the root node.  But better yet, instantiate
 the correct fabric device (probably as a platform_device) from the
 platform code.  Then the correct fabric driver can probe against it.

The meaning of this has finally sunk into my consciousness. The
platform code can create a device that isn't bound to a driver. So why
not make this an of_platform_device?  This is basically a pseudo
device that isn't in the device tree.

Alternatively, the best place for this device would be on the ASOC
bus, but the ASOC bus hasn't been created when the platform code runs.
Maybe I can figure out a place in the platform code to create this
device after the ASOC driver has loaded and created the bus. Does the
platform code get control back after loading all of the device
drivers?

In the longer term I'd like to kill platform_bus on powerpc and only
use of_platform_bus. Platform_bus seems to be functioning like a
catch-all and collecting junk from lots of different platforms.


-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Mark Brown
On Wed, Jan 02, 2008 at 09:10:44AM -0600, Timur Tabi wrote:
 Jon Smirl wrote:

  Does this need to be bus-frequency? It's always called MCLK in all of
  the literature.

 I'm trying to make this node as generic as possible.  The fabric driver 
 is the one that will parse this node and pass the data to the codec 
 driver, so I can't use any codec-specific terms.

 The API from the fabric driver for passing clock information includes a 
 clock ID, a direction, and a frequency.  I can do something like this:

 clock1 = 0, bb8000

 Would that be better?

To cover everything you'd need to be able to specify all the clocking
parameters, especially a PLL configuration, and also specify more than
one of each item.  Even then you'd still have problems like...

  In my case the MCLK comes from a chip on the i2c bus that is
  programmable How would that be encoded?.

 I'm going under the assumption that MCLK does not change once the board 
 is up and running.  In your case, you'd need to do something quite 
 different, because you're not reading the clock info from the device 
 tree and passing it to the codec at initialization once.  If you want to 
 define an extension to the 'codec' child node that handles that, I'll 
 add it to the documentation.

According to the documentation in your patch the bus frequency should
already be optional (though I don't immediately see that in the code,
but then I'm entirely unfamiliar with OpenFirmware device trees).
Boards that reconfigure the clocking at run time can then provide
code to set the clocking up at the appropriate times, which is probably
what they want anyway.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Scott Wood
On Sat, Dec 22, 2007 at 08:58:21PM -0600, Timur Tabi wrote:
 Scott Wood wrote:

 None of the SOC nodes in any DTS have a compatible entry.

 Not quite true; ep88xc, mpc8272ads, and pq2fads have them.

 Ah ok.  So what should the compatible entry for 8641 be?

   compatible = fsl,mpc8641

Yes.

 That looks a lot like what a compatible entry for the CPU should be.

I guess technically the cpu should list something like fsl,e600 (or
whatever suffix is relevant).

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Jon Smirl
On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
  Alternatively, the best place for this device would be on the ASOC
  bus, but the ASOC bus hasn't been created when the platform code runs.
  Maybe I can figure out a place in the platform code to create this
  device after the ASOC driver has loaded and created the bus. Does the
  platform code get control back after loading all of the device
  drivers?

 Yes, but it requires the core ASoC code to not be a module.  Then you
 can use machine_device_initcall() to register the device at a later
 time.

How about this for a simpler solution? My mpc5200-psc-ac97 and
mpc5200-psc-i2c drivers can create a device on the ASOC bus named
after the first entry in the compatible field of the root node. That
will cause the correct driver to get activated. I'm in the process of
making ASOC drivers dynamically loadable like the i2c ones.


-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Mark Brown
On Wed, Jan 02, 2008 at 09:28:12AM -0700, Grant Likely wrote:
 On 1/2/08, Timur Tabi [EMAIL PROTECTED] wrote:

  However, it doesn't make sense to have a node in the device tree for the
  fabric driver, because there is no such device.  The fabric driver is
  an abstraction.  So I need to chose some other node to probe the fabric
  driver with.  I chose the SSI, since each SSI can have only one codec.

I'm not sure I'd go so far as to say that the fabric/machine driver is
purely an abstraction.  It does represent real hardware, often with
software control.

 Does that mean with ASoC V2 you can instantiate it with the board
 specific platform code instead?  I think that is the consensus we were
 leaning towards in the last discussion about this issue.

With ASoC v2 rather than having a single monolithic ASoC device which
probes everything ASoC is converted into a proper subsystem with each
component (codec, SoC CPU port, whatever) having a sysfs-visible driver.
These drivers register with the core as they are probed with the probing
happening through whatever mechanism is appropriate for the driver.

The machine support code (fabric driver in PowerPC terms, I think?)
tells the core how everything is connected together by registering
devices representing the links (eg, I2S) between the codecs, CPU and
other devices.  The ASoC core is then responsible for ensuring that all
the required components are present before it registers with the ALSA
core.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Grant Likely
On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
   Alternatively, the best place for this device would be on the ASOC
   bus, but the ASOC bus hasn't been created when the platform code runs.
   Maybe I can figure out a place in the platform code to create this
   device after the ASOC driver has loaded and created the bus. Does the
   platform code get control back after loading all of the device
   drivers?
 
  Yes, but it requires the core ASoC code to not be a module.  Then you
  can use machine_device_initcall() to register the device at a later
  time.

 How about this for a simpler solution? My mpc5200-psc-ac97 and
 mpc5200-psc-i2c drivers can create a device on the ASOC bus named
 after the first entry in the compatible field of the root node. That
 will cause the correct driver to get activated. I'm in the process of
 making ASOC drivers dynamically loadable like the i2c ones.

I little icky, but it doesn't sound dangerous (as in shouldn't cause
any name conflicts).  That may be the best we can do for the time
being.  But I don't think it is a good idea for the long term.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-02 Thread Jon Smirl
On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
 On 1/2/08, Jon Smirl [EMAIL PROTECTED] wrote:
  On 1/2/08, Grant Likely [EMAIL PROTECTED] wrote:
Alternatively, the best place for this device would be on the ASOC
bus, but the ASOC bus hasn't been created when the platform code runs.
Maybe I can figure out a place in the platform code to create this
device after the ASOC driver has loaded and created the bus. Does the
platform code get control back after loading all of the device
drivers?
  
   Yes, but it requires the core ASoC code to not be a module.  Then you
   can use machine_device_initcall() to register the device at a later
   time.
 
  How about this for a simpler solution? My mpc5200-psc-ac97 and
  mpc5200-psc-i2c drivers can create a device on the ASOC bus named
  after the first entry in the compatible field of the root node. That
  will cause the correct driver to get activated. I'm in the process of
  making ASOC drivers dynamically loadable like the i2c ones.

 I little icky, but it doesn't sound dangerous (as in shouldn't cause
 any name conflicts).  That may be the best we can do for the time
 being.  But I don't think it is a good idea for the long term.

Simplest long term fix is to allow drivers to bind on the root node.
Make this work:

 static struct of_device_id fabric_of_match[] = {
 {
 .compatible = fsl,MPC8610HPCD,
 },
 {},
 };




 Cheers,
 g.

 --
 Grant Likely, B.Sc., P.Eng.
 Secret Lab Technologies Ltd.
 [EMAIL PROTECTED]
 (403) 399-0195



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-01 Thread Jon Smirl
On 1/1/08, Jon Smirl [EMAIL PROTECTED] wrote:
 On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
  +   [EMAIL PROTECTED] {
  +   compatible = fsl,ssi;
  +   cell-index = 0;
  +   reg = 16000 100;
  +   interrupt-parent = mpic;
  +   interrupts = 3e 2;
  +   fsl,mode = i2s-slave;
  +   codec {
  +   compatible = cirrus,cs4270;
  +   /* MCLK source is a stand-alone oscillator 
  */
  +   bus-frequency = bb8000;
  +   };
  +   };

 Does this need to be bus-frequency? It's always called MCLK in all of
 the literature.

 In my case the MCLK comes from a chip on the i2c bus that is
 programmable How would that be encoded?.

Looking at the cs4270 codec driver it is controlled by i2c (supports
SPI too).  What happened to the conversation about putting codecs on
the controlling bus and then linking them to the data bus?

If that's the case the cs4270 should be in the i2c bus node (missing
currently) and then a link from the SSI bus would point to it.

cs4270 is still an old style i2c driver which is going to get
deprecated. It takes about thirty minutes to convert it to new style.
If was new style it could pick up its i2c address from the device tree
instead of searching for it.

-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-01 Thread David Gibson
On Tue, Jan 01, 2008 at 12:25:32PM -0500, Jon Smirl wrote:
 On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
  +   [EMAIL PROTECTED] {
  +   compatible = fsl,ssi;
  +   cell-index = 0;
  +   reg = 16000 100;
  +   interrupt-parent = mpic;
  +   interrupts = 3e 2;
  +   fsl,mode = i2s-slave;
  +   codec {
  +   compatible = cirrus,cs4270;
  +   /* MCLK source is a stand-alone oscillator 
  */
  +   bus-frequency = bb8000;
  +   };
  +   };
 
 Does this need to be bus-frequency? It's always called MCLK in all of
 the literature.
 
 In my case the MCLK comes from a chip on the i2c bus that is
 programmable How would that be encoded?.

Grah!  If there's one obvious frequency for a node, it should always
be clock-frequency.  This bus-frequency nonsense seems to be a
disease that started as a secondary frequency in Freescale CPU nodes,
and has escaped to all sorts of other places.

-- 
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


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2008-01-01 Thread Jon Smirl
On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:
  sound/soc/fsl/fsl_ssi.c  |  614 +++
  sound/soc/fsl/fsl_ssi.h  |  224 +++

I'm confused about this part. You built a driver for the mpc8610 ssi
port.  This port has a device tree entry.

+   [EMAIL PROTECTED] {
+   compatible = fsl,ssi;
+   cell-index = 0;
+   reg = 16000 100;
+   interrupt-parent = mpic;
+   interrupts = 3e 2;
+   fsl,mode = i2s-slave;
+   codec {
+   compatible = cirrus,cs4270;
+   /* MCLK source is a stand-alone oscillator */
+   bus-frequency = bb8000;
+   };
+   };

But then you don't create an of_platform_driver for this device.
Instead you create one for the fabric driver, struct
of_platform_driver mpc8610_hpcd_of_driver, and directly link the SSI
driver into it.

+static struct of_device_id mpc8610_hpcd_match[] = {
+   {
+   .compatible = fsl,ssi,
+   },
+   {}
+};
+MODULE_DEVICE_TABLE(of, mpc8610_hpcd_match);
+
+static struct of_platform_driver mpc8610_hpcd_of_driver = {
+   .owner  = THIS_MODULE,
+   .name   = mpc8610_hpcd,
+   .match_table= mpc8610_hpcd_match,
+   .probe  = mpc8610_hpcd_probe,
+   .remove = mpc8610_hpcd_remove,
+};

static int mpc8610_hpcd_probe(struct of_device *ofdev,
const struct of_device_id *match)
{
.
machine_data-dai.cpu_dai = fsl_ssi_create_dai(ssi_info);

Isn't this two separate drivers that have been combined into one
driver? Or does the fsl_ssi channel only work on the mpc8610_hpcd?

This is the problem of knowing how to load the fabric driver that I
was talking about in the other threads. A device that can occur on
more than one chip .compatible = fsl,ssi, is being used to pull in
a platform specific fabric driver, mpc8610_hpcd. You can use the
kernel config system to select the right driver for .compatible =
fsl,ssi, that matches you hardware and compile it in.

But that doesn't work in my environment. My generic channel is
fsl,i2s. I have four different systems booting off from a shared
network drive. Each of these systems needs the common fsl,i2s driver
but they all four need different fabric drivers.

-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-22 Thread Timur Tabi
Scott Wood wrote:

 None of the SOC nodes in any DTS have a compatible entry.
 
 Not quite true; ep88xc, mpc8272ads, and pq2fads have them.

Ah ok.  So what should the compatible entry for 8641 be?

compatible = fsl,mpc8641

That looks a lot like what a compatible entry for the CPU should be. 
How are we differentiating between the compatible cores and compatible SOCs?

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-22 Thread Timur Tabi
Lee Revell wrote:

 Please use DMA_32BIT_MASK (see include/linux/dma-mapping.h) instead of
 0x. 

No prob.  But did you see this comment:

/*
  * NOTE: do not use the below macros in new code and do not add new 
definitions
  * here.
  *
  * Instead, just open-code DMA_BIT_MASK(n) within your driver
  */

So I guess I should use DMA_BIT_MASK(32) instead.

 I've personally fixed a heisenbug in an ALSA driver
 caused by incorrectly typed DMA mask...

Can you explain to me what all of this does?  Is it okay to use a static 
u64 variable?  Why do so many drivers do it that way?  I don't even know 
if 0x is the right number for my platform.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Timur Tabi
Olof Johansson wrote:

 +static struct of_device_id mpc8610_ids[] = {
 +{ .type = soc, },
 +{}
 
 Please scan based on compatible instead of device_type.

I was just following the example from another board file.  However, the 'soc' 
node in the device tree does not have a compatible property, so I don't how to 
change this.

 +config SND_SOC_MPC8610
 +bool ALSA SoC support for the MPC8610 SOC
 +depends on SND_SOC # MPC8610_HPCD
 +default y #if MPC8610
 +help
 +  Say Y if you want to add support for codecs attached to the SSI
 +  device on an MPC8610.
 
 Don't default configs to 'y'. Also, what's with the commented-out
 dependencies and if?

Sorry, that was a development change that I forgot to put back.  The y # 
should be deleted.


 + * ssi_stx_phys: bus address of SSI STX register
 + * ssi_srx_phys: bus address of SSI SRX register
 + * dma_channel: pointer to the DMA channel's registers
 + * irq: IRQ for this DMA channel
 + * assigned: set to 1 if that DMA channel is assigned to a substream
 + */
 
 This goes for the whole patch: You've got good documentation, but it's
 not in docbook format. Please reformat it since it should be a pretty
 simple thing to do.

Ok.

 +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai *dai,
 +struct snd_pcm *pcm)
 +{
 +static u64 fsl_dma_dmamask = 0x;
 +int ret;
 +
 +if (!card-dev-dma_mask)
 +card-dev-dma_mask = fsl_dma_dmamask;
 
 I haven't read how your channel allocation works, but providing a
 pointer to a local static variable is a bit fishy no matter what.

I just copied this code from another module.  All the ALSA drivers do this, 
but I'll look into it and see if it can't be done different.  I make no 
promises, though!

 Do you ever anticipate having other dma users on the system, such as
 memcpy offload? You'll probably need allocation support for channels
 when that day comes (I ended up writing a simple library for dma channel
 management for that very reason on my platform).

Yes, I plan on updating this driver to work with the standard Freescale Elo 
device driver, but that will have to wait until that code is in the kernel and 
stabilized.  The SSI is limited in which DMA channels it can use, anyway.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Takashi Iwai
At Thu, 20 Dec 2007 08:24:35 -0600,
Timur Tabi wrote:
 
  +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai 
  *dai,
  +  struct snd_pcm *pcm)
  +{
  +  static u64 fsl_dma_dmamask = 0x;
  +  int ret;
  +
  +  if (!card-dev-dma_mask)
  +  card-dev-dma_mask = fsl_dma_dmamask;
  
  I haven't read how your channel allocation works, but providing a
  pointer to a local static variable is a bit fishy no matter what.
 
 I just copied this code from another module.  All the ALSA drivers do this, 

All?  No, only a few...
For PCI, usually pci_set_dma_mask() and pci_set_consistent_dma_mask()
are used, of course.


Takashi
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Timur Tabi
Takashi Iwai wrote:

 All?  No, only a few...
 For PCI, usually pci_set_dma_mask() and pci_set_consistent_dma_mask()
 are used, of course.

Hmm, ok I was wrong.  I took this code from the ASoC at91 driver.

Unfortunately, I don't understand what this code is trying to do.  The AT91 
driver isn't documented, so I don't even know if I need it.  Can someone 
explain 
what all this is?  What's the alternative to using a static global?

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Jon Smirl
On 12/19/07, Timur Tabi [EMAIL PROTECTED] wrote:

 +static struct of_device_id mpc8610_ids[] = {
 +   { .type = soc, },
 +   {}
 +};
 +
 +static int __init mpc8610_declare_of_platform_devices(void)
 +{
 +   if (!machine_is(mpc86xx_hpcd))
 +   return 0;
 +
 +   /* Without this call, the SSI device driver won't get probed. */
 +   of_platform_bus_probe(NULL, mpc8610_ids, NULL);
 +
 +   return 0;
 +}
 +device_initcall(mpc8610_declare_of_platform_devices);

How is of_platform_bus_probe() supposed to be called? mpc5200/virtex
call it with three NULLs. Is it necessary to name all of the buses in
a of_device_id? If it's not necessary to list the buses the
of_platform_bus_probe() call could be moved to common code.

Are these buses?
{ .compatible = ibm,plb4, },
{ .compatible = ibm,opb, },
{ .compatible = ibm,ebc, },

Could of_platform_bus_probe() be simplified? No one uses the first and
third parameters.

-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Timur Tabi
Jon Smirl wrote:

 How is of_platform_bus_probe() supposed to be called? mpc5200/virtex
 call it with three NULLs. Is it necessary to name all of the buses in
 a of_device_id? If it's not necessary to list the buses the
 of_platform_bus_probe() call could be moved to common code.

I added the above code because it is the only way I could get my SSI nodes to 
be 
probed.  If there's a better way to do it, I'm all ears.  I just copied that 
code from the mpc836x_mds.c platform file.

 Are these buses?
 { .compatible = ibm,plb4, },
 { .compatible = ibm,opb, },
 { .compatible = ibm,ebc, },

I have no idea.

 Could of_platform_bus_probe() be simplified? No one uses the first and
 third parameters.

Maybe, but that's not a discussion for this thread!

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Olof Johansson
On Thu, Dec 20, 2007 at 08:24:35AM -0600, Timur Tabi wrote:
 Olof Johansson wrote:
 
  +static struct of_device_id mpc8610_ids[] = {
  +  { .type = soc, },
  +  {}
  
  Please scan based on compatible instead of device_type.
 
 I was just following the example from another board file.  However, the 'soc' 
 node in the device tree does not have a compatible property, so I don't how 
 to 
 change this.

Then add an appropriate compatible entry to it, please.

  Do you ever anticipate having other dma users on the system, such as
  memcpy offload? You'll probably need allocation support for channels
  when that day comes (I ended up writing a simple library for dma channel
  management for that very reason on my platform).
 
 Yes, I plan on updating this driver to work with the standard Freescale Elo 
 device driver, but that will have to wait until that code is in the kernel 
 and 
 stabilized.  The SSI is limited in which DMA channels it can use, anyway.

Ok.


-Olof
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Timur Tabi
Olof Johansson wrote:

 I was just following the example from another board file.  However, the 
 'soc' 
 node in the device tree does not have a compatible property, so I don't how 
 to 
 change this.
 
 Then add an appropriate compatible entry to it, please.

None of the SOC nodes in any DTS have a compatible entry.  I understand the 
issue, but you're asking me to fix a larger problem, one that is beyond the 
scope of this patch.  You're saying that *all* SOC needs are incorrectly 
defined 
and need to be probed differently.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Jon Smirl
On 12/20/07, Timur Tabi [EMAIL PROTECTED] wrote:
 Jon Smirl wrote:

  How is of_platform_bus_probe() supposed to be called? mpc5200/virtex
  call it with three NULLs. Is it necessary to name all of the buses in
  a of_device_id? If it's not necessary to list the buses the
  of_platform_bus_probe() call could be moved to common code.

 I added the above code because it is the only way I could get my SSI nodes to 
 be
 probed.  If there's a better way to do it, I'm all ears.  I just copied that
 code from the mpc836x_mds.c platform file.

mpc5200 does it like this:
of_platform_bus_probe(NULL, NULL, NULL);

No need for the ids.



  Are these buses?
  { .compatible = ibm,plb4, },
  { .compatible = ibm,opb, },
  { .compatible = ibm,ebc, },

 I have no idea.

  Could of_platform_bus_probe() be simplified? No one uses the first and
  third parameters.

 Maybe, but that's not a discussion for this thread!

 --
 Timur Tabi
 Linux kernel developer at Freescale



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Scott Wood
Timur Tabi wrote:
 Olof Johansson wrote:
 
 I was just following the example from another board file.  However, the 
 'soc' 
 node in the device tree does not have a compatible property, so I don't how 
 to 
 change this.
 Then add an appropriate compatible entry to it, please.
 
 None of the SOC nodes in any DTS have a compatible entry.

Not quite true; ep88xc, mpc8272ads, and pq2fads have them.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Scott Wood
Timur Tabi wrote:
 Jon Smirl wrote:
 
 mpc5200 does it like this:
 of_platform_bus_probe(NULL, NULL, NULL);
 
 I think that tells the OF base code to probe everything in the device tree, 
 which is probably overkill.  I think fsl_soc.c covers most of the device 
 tree, 
 but the SSI is not defined in fsl_soc.c.

Not quite; it tells it to use a built-in list of bus matches.  Most of 
which are device_type-based, FWIW.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Jon Smirl
On 12/20/07, Scott Wood [EMAIL PROTECTED] wrote:
 Timur Tabi wrote:
  Jon Smirl wrote:
 
  mpc5200 does it like this:
  of_platform_bus_probe(NULL, NULL, NULL);
 
  I think that tells the OF base code to probe everything in the device tree,
  which is probably overkill.  I think fsl_soc.c covers most of the device 
  tree,
  but the SSI is not defined in fsl_soc.c.

 Not quite; it tells it to use a built-in list of bus matches.  Most of
 which are device_type-based, FWIW.

Here's the default. Using NULL would work.

static struct of_device_id of_default_bus_ids[] = {
{ .type = soc, },
{ .compatible = soc, },
{ .type = spider, },
{ .type = axon, },
{ .type = plb5, },
{ .type = plb4, },
{ .type = opb, },
{ .type = ebc, },
{},
};



 -Scott



-- 
Jon Smirl
[EMAIL PROTECTED]
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread David Gibson
On Thu, Dec 20, 2007 at 06:13:31PM -0500, Jon Smirl wrote:
 On 12/20/07, Scott Wood [EMAIL PROTECTED] wrote:
  Timur Tabi wrote:
   Jon Smirl wrote:
  
   mpc5200 does it like this:
   of_platform_bus_probe(NULL, NULL, NULL);
  
   I think that tells the OF base code to probe everything in the device 
   tree,
   which is probably overkill.  I think fsl_soc.c covers most of the device 
   tree,
   but the SSI is not defined in fsl_soc.c.
 
  Not quite; it tells it to use a built-in list of bus matches.  Most of
  which are device_type-based, FWIW.
 
 Here's the default. Using NULL would work.

It might work, but using the default list is discouraged.  Pass an
explicit list of match ids for the buses you need to scan instead (and
use compatible to match them, not device_type).

-- 
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


Re: [alsa-devel] [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-20 Thread Lee Revell
On Dec 20, 2007 8:54 AM, Takashi Iwai [EMAIL PROTECTED] wrote:
 At Thu, 20 Dec 2007 08:24:35 -0600,
 Timur Tabi wrote:
 
   +static int fsl_dma_new(struct snd_card *card, struct snd_soc_codec_dai 
   *dai,
   +  struct snd_pcm *pcm)
   +{
   +  static u64 fsl_dma_dmamask = 0x;
   +  int ret;
   +
   +  if (!card-dev-dma_mask)
   +  card-dev-dma_mask = fsl_dma_dmamask;
  
   I haven't read how your channel allocation works, but providing a
   pointer to a local static variable is a bit fishy no matter what.
 
  I just copied this code from another module.  All the ALSA drivers do this,

 All?  No, only a few...
 For PCI, usually pci_set_dma_mask() and pci_set_consistent_dma_mask()
 are used, of course.

Timur,

Nicely commented driver!  I wish they were all like this ;-)

Please use DMA_32BIT_MASK (see include/linux/dma-mapping.h) instead of
0x.  I've personally fixed a heisenbug in an ALSA driver
caused by incorrectly typed DMA mask...

Lee
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH] ASoC drivers for the Freescale MPC8610 SoC

2007-12-19 Thread Olof Johansson
Hi,

This is a fairly substantial driver to get through, but here are some
initial comments on some of the simpler stuff:


On Wed, Dec 19, 2007 at 06:03:09PM -0600, Timur Tabi wrote:
 This patch adds ALSA SoC device drivers for the Freescale MPC8610 SoC
 and the MPC8610-HPCD reference board.

[...]

 diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c 
 b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
 index 6390895..6e1bde3 100644
 --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
 +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
 @@ -34,9 +34,27 @@
  
  #include asm/mpic.h
  
 +#include asm/of_platform.h
  #include sysdev/fsl_pci.h
  #include sysdev/fsl_soc.h
  
 +static struct of_device_id mpc8610_ids[] = {
 + { .type = soc, },
 + {}

Please scan based on compatible instead of device_type.

 diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
 new file mode 100644
 index 000..4a5bbd2
 --- /dev/null
 +++ b/sound/soc/fsl/Kconfig
 @@ -0,0 +1,21 @@
 +menu ALSA SoC audio for Freescale SOCs
 +
 +config SND_SOC_MPC8610
 + bool ALSA SoC support for the MPC8610 SOC
 + depends on SND_SOC # MPC8610_HPCD
 + default y #if MPC8610
 + help
 +   Say Y if you want to add support for codecs attached to the SSI
 +  device on an MPC8610.

Don't default configs to 'y'. Also, what's with the commented-out
dependencies and if?

 +config SND_SOC_MPC8610_HPCD
 + # ALSA SoC support for Freescale MPC8610HPCD
 + bool ALSA SoC support for the Freescale MPC8610 HPCD board
 + depends on SND_SOC_MPC8610
 + select SND_SOC_CS4270
 + select SND_SOC_CS4270_VD33_ERRATA
 + default y #if MPC8610_HPCD
 + help
 +   Say Y if you want to enable audio on the Freescale MPC8610 HPCD.

Same here w.r.t. defaults and dependencies.

 diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
 new file mode 100644
 index 000..6b86be0
 --- /dev/null
 +++ b/sound/soc/fsl/fsl_dma.c
 @@ -0,0 +1,819 @@
 +/*
 + * Freescale DMA ALSA SoC PCM driver
 + *
 + * Author: Timur Tabi [EMAIL PROTECTED]
 + *
 + * Copyright 2007 Freescale Semiconductor, Inc.  This file is licensed under
 + * the terms of the GNU General Public License version 2.  This program
 + * is licensed as is without any warranty of any kind, whether express
 + * or implied.
 + *
 + * This driver implements ASoC support for the Elo DMA controller, which is
 + * the DMA controller on Freescale 83xx, 85xx, and 86xx SOCs. In ALSA terms,
 + * the PCM driver is what handles the DMA buffer.
 + */
 +
 +#include linux/module.h
 +#include linux/init.h
 +#include linux/platform_device.h
 +#include linux/dma-mapping.h
 +#include linux/interrupt.h
 +#include linux/delay.h
 +
 +#include sound/driver.h
 +#include sound/core.h
 +#include sound/pcm.h
 +#include sound/pcm_params.h
 +#include sound/soc.h
 +
 +#include asm/io.h
 +
 +#include fsl_dma.h
 +
 +/*
 + * The formats that the DMA controller supports, which is anything
 + * that is 8, 16, or 32 bits.
 + */
 +#define FSLDMA_PCM_FORMATS (SNDRV_PCM_FMTBIT_S8  | \
 + SNDRV_PCM_FMTBIT_U8 | \
 + SNDRV_PCM_FMTBIT_S16_LE | \
 + SNDRV_PCM_FMTBIT_S16_BE | \
 + SNDRV_PCM_FMTBIT_U16_LE | \
 + SNDRV_PCM_FMTBIT_U16_BE | \
 + SNDRV_PCM_FMTBIT_S24_LE | \
 + SNDRV_PCM_FMTBIT_S24_BE | \
 + SNDRV_PCM_FMTBIT_U24_LE | \
 + SNDRV_PCM_FMTBIT_U24_BE | \
 + SNDRV_PCM_FMTBIT_S32_LE | \
 + SNDRV_PCM_FMTBIT_S32_BE | \
 + SNDRV_PCM_FMTBIT_U32_LE | \
 + SNDRV_PCM_FMTBIT_U32_BE)
 +
 +#define FSLDMA_PCM_RATES (SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_8000_192000 | 
 \
 +   SNDRV_PCM_RATE_CONTINUOUS)
 +
 +/* DMA global data.  This structure is used by fsl_dma_open() to determine
 + * which DMA channels to assign to a substream.  Unfortunately, ASoC V1 does
 + * not allow the machine driver to provide this information to the PCM
 + * driver in advance, and there's no way to differentiate between the two
 + * DMA controllers.  So for now, this driver only supports one SSI device
 + * using two DMA channels.  We cannot support multiple DMA devices.
 + *
 + * ssi_stx_phys: bus address of SSI STX register
 + * ssi_srx_phys: bus address of SSI SRX register
 + * dma_channel: pointer to the DMA channel's registers
 + * irq: IRQ for this DMA channel
 + * assigned: set to 1 if that DMA channel is assigned to a substream
 + */

This goes for the whole patch: You've got good documentation, but it's
not in docbook format. Please reformat it since it should be a pretty
simple thing to do.

 +/*
 + * Initialize this PCM driver.
 + *
 + * This function is called when the codec driver calls snd_soc_new_pcms(),
 + * once for each .dai_link in the