Re: [alsa-devel] [PATCH 2/2] Allow a custom ASOC machine driver with soc-of-simple

2008-07-26 Thread Grant Likely
On Wed, Jul 23, 2008 at 04:14:32PM +0100, Mark Brown wrote:
 On Wed, Jul 23, 2008 at 10:09:01AM -0400, Jon Smirl wrote:
  On 7/23/08, Mark Brown [EMAIL PROTECTED] wrote:
 
   ...this doesn't just allow a custom machine driver, it requires that
something configures at least the machine name.  That's not a problem
from the ASoC point of view but possibly from the PowerPC side?
 
  You have to configure at least the name. Otherwise if the machine
  driver is the last to register, how do you know to hold off the final
  registration and wait for the machine driver to appear?
 
 I understand why you have made this change but it's a substantial change
 which should at least be documented in the changelog (I'd expect to see
 some mention of how this is supposed to be configured, for example).
 I'd also expect something to handle the existing user.
 
 Like I said, I'm not entirely sure that you're supposed to be using this
 driver if you want a machine driver: this is a machine driver and I'm
 not sure if it's supposed to cover all cases or not.  Grant?

As I replied to the patch itself, I'm not thrilled with it, but I don't
have an immediate solution.  I do want this behaviour to at least
trigger on something in the device tree; not to become a hard
requirement.

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


Re: [alsa-devel] [PATCH 2/2] Allow a custom ASOC machine driver with soc-of-simple

2008-07-23 Thread Mark Brown
On Tue, Jul 22, 2008 at 07:53:51PM -0400, Jon Smirl wrote:

 Allow a custom ASOC machine driver with soc-of-simple

As with the clocking configuration: this looks fine from an ASoC point
of view but please fix the checkpatch warnings.  However...

   /* Only register the device if both the codec and platform have
* been registered */
 - if ((!of_soc-device.codec_data) || (!of_soc-platform_node))
 + if ((!of_soc-device.codec_data) || (!of_soc-platform_node) || 
 !machine_name)
   return;

...this doesn't just allow a custom machine driver, it requires that
something configures at least the machine name.  That's not a problem
from the ASoC point of view but possibly from the PowerPC side?
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH 2/2] Allow a custom ASOC machine driver with soc-of-simple

2008-07-23 Thread Jon Smirl
On 7/23/08, Mark Brown [EMAIL PROTECTED] wrote:
 On Tue, Jul 22, 2008 at 07:53:51PM -0400, Jon Smirl wrote:

   Allow a custom ASOC machine driver with soc-of-simple


 As with the clocking configuration: this looks fine from an ASoC point
  of view but please fix the checkpatch warnings.  However...


 /* Only register the device if both the codec and platform have
  * been registered */
   - if ((!of_soc-device.codec_data) || (!of_soc-platform_node))
   + if ((!of_soc-device.codec_data) || (!of_soc-platform_node) || 
 !machine_name)
 return;


 ...this doesn't just allow a custom machine driver, it requires that
  something configures at least the machine name.  That's not a problem
  from the ASoC point of view but possibly from the PowerPC side?

You have to configure at least the name. Otherwise if the machine
driver is the last to register, how do you know to hold off the final
registration and wait for the machine driver to appear?

Or is it ok for me to change these after the platform device has been created?
of_soc-dai_link.ops = machine_ops;
of_soc-machine.name = machine_name;

I have to have the machine driver in order to control my external clock chips.

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


Re: [alsa-devel] [PATCH 2/2] Allow a custom ASOC machine driver with soc-of-simple

2008-07-23 Thread Mark Brown
On Wed, Jul 23, 2008 at 10:09:01AM -0400, Jon Smirl wrote:
 On 7/23/08, Mark Brown [EMAIL PROTECTED] wrote:

  ...this doesn't just allow a custom machine driver, it requires that
   something configures at least the machine name.  That's not a problem
   from the ASoC point of view but possibly from the PowerPC side?

 You have to configure at least the name. Otherwise if the machine
 driver is the last to register, how do you know to hold off the final
 registration and wait for the machine driver to appear?

I understand why you have made this change but it's a substantial change
which should at least be documented in the changelog (I'd expect to see
some mention of how this is supposed to be configured, for example).
I'd also expect something to handle the existing user.

Like I said, I'm not entirely sure that you're supposed to be using this
driver if you want a machine driver: this is a machine driver and I'm
not sure if it's supposed to cover all cases or not.  Grant?

 Or is it ok for me to change these after the platform device has been created?
   of_soc-dai_link.ops = machine_ops;
   of_soc-machine.name = machine_name;

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


Re: [alsa-devel] [PATCH 2/2] Allow a custom ASOC machine driver with soc-of-simple

2008-07-23 Thread Jon Smirl
On 7/23/08, Mark Brown [EMAIL PROTECTED] wrote:
 On Wed, Jul 23, 2008 at 10:09:01AM -0400, Jon Smirl wrote:
   On 7/23/08, Mark Brown [EMAIL PROTECTED] wrote:


   ...this doesn't just allow a custom machine driver, it requires that
 something configures at least the machine name.  That's not a problem
 from the ASoC point of view but possibly from the PowerPC side?

   You have to configure at least the name. Otherwise if the machine
   driver is the last to register, how do you know to hold off the final
   registration and wait for the machine driver to appear?


 I understand why you have made this change but it's a substantial change
  which should at least be documented in the changelog (I'd expect to see
  some mention of how this is supposed to be configured, for example).
  I'd also expect something to handle the existing user.

This is a modification to Grant's new driver. Grant is the only other user.


  Like I said, I'm not entirely sure that you're supposed to be using this
  driver if you want a machine driver: this is a machine driver and I'm
  not sure if it's supposed to cover all cases or not.  Grant?


   Or is it ok for me to change these after the platform device has been 
 created?
 of_soc-dai_link.ops = machine_ops;
 of_soc-machine.name = machine_name;


 No.



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


Re: [alsa-devel] [PATCH 2/2] Allow a custom ASOC machine driver with soc-of-simple

2008-07-23 Thread Mark Brown
On Wed, Jul 23, 2008 at 11:16:17AM -0400, Jon Smirl wrote:
 On 7/23/08, Mark Brown [EMAIL PROTECTED] wrote:

   I'd also expect something to handle the existing user.

 This is a modification to Grant's new driver. Grant is the only other user.

Right, hence my use of the singular : ) .  Since you are adding this new
of_snd_soc_register_machine() Grant's existing platform won't be calling
it yet and will therefore stop instantiating the ASoC subsystem until it
is changed to do so.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev