public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@nokia.com>
To: ext Samuel Ortiz <sameo@linux.intel.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"tony@atomide.com" <tony@atomide.com>,
	"broonie@opensource.wolfsonmicro.com" 
	<broonie@opensource.wolfsonmicro.com>
Subject: Re: [PATCH 1/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
Date: Thu, 22 Oct 2009 09:04:32 +0300	[thread overview]
Message-ID: <200910220904.32676.peter.ujfalusi@nokia.com> (raw)
In-Reply-To: <20091021231311.GF17796@sortiz.org>

On Thursday 22 October 2009 02:13:11 ext Samuel Ortiz wrote:
> As Mark noticed already, you dont really want users to explicitely select
>  this obscure mfd driver to get their audio and vibre driver selectable. It
>  should be the other way around, and I think you already agreed with that.

Yes, I have already made the modification.

> 
> > +static struct device *
> > +twl4030_codec_new_child(const char *name, void *pdata, unsigned
> > pdata_len) +{

...
 
> This could really use the mfd-core API, and avoid duplicating code.
> You just would have to define a couple cells, and call mfd_add_devices on
> them.

Good point. When I wrote this driver I have been looking at the twl4030-core 
driver, which is not using the mfd-core API. I'll make the change.

> 
> > +static int __devexit twl4030_codec_remove(struct platform_device *pdev)
> > +{
> > +	struct twl4030_codec *codec = platform_get_drvdata(pdev);
> > +
> > +	platform_set_drvdata(pdev, NULL);
> > +	kfree(codec);
> > +	twl4030_codec_dev = NULL;
> > +
> > +	return 0;
> > +}
> 
> I think you're missing a platform_device_unregister() here (or an
> mfd_remove_devices() if you're going to switch to the mfd-core API)

You are right, although I have used the bool in the Kconfig for the twl4030-
codec in a same way as the twl4030-core (and the core did not unregister the 
devices either), but yes, this is not correct so I'll fix it.

I have now question about the practicalities on how this series would be taken, 
and via which tree.
The final patch for the soc codec driver depends on the change in MFD and in the 
OMAP board files.
The change in the OMAP board files depends on the MFD changes, obviously.

So in order to have the soc codec changes the MFD and OMAP part has to be 
applied before, otherwise the audio will be broken.

the mfd-2.6:for-next branch has some patches against the twl4030-core in 
addition to the ones in l-o and in sound-2.6:topic/asoc.

I'll check, if the MFD patch applies to mfd-2.6:for-next also, but to have the 
soc codec changes the MFD patch should go to the sound-2.6 tree as well to make 
sure it is not braking things.

All-in-all, how these things can be handled?

Thanks, 
Péter

  reply	other threads:[~2009-10-22  6:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-19 12:42 [PATCH 0/4] twl4030 codec as MFD device Peter Ujfalusi
2009-10-19 12:42 ` [PATCH 1/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core Peter Ujfalusi
2009-10-19 12:42   ` [PATCH 2/4] OMAP: Platform support for twl4030_codec MFD Peter Ujfalusi
2009-10-19 12:42     ` [PATCH 3/4] ASoC: TWL4030: Only update the needed bits in *set_dai_sysclk Peter Ujfalusi
2009-10-19 12:42       ` [PATCH 4/4] ASoC: TWL4030: Driver registration via twl4030_codec MFD Peter Ujfalusi
2009-10-20 10:25         ` Mark Brown
2009-10-20 11:30           ` Peter Ujfalusi
2009-10-20 11:51             ` Mark Brown
2009-10-20 12:01               ` Peter Ujfalusi
2009-10-19 12:55       ` [PATCH 3/4] ASoC: TWL4030: Only update the needed bits in *set_dai_sysclk Mark Brown
2009-10-19 12:59         ` Peter Ujfalusi
2009-10-20 19:01     ` [PATCH 2/4] OMAP: Platform support for twl4030_codec MFD Tony Lindgren
2009-10-20 10:03   ` [PATCH 1/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core Mark Brown
2009-10-20 11:16     ` [alsa-devel] " Peter Ujfalusi
2009-10-21 23:13   ` Samuel Ortiz
2009-10-22  6:04     ` Peter Ujfalusi [this message]
2009-10-22  7:57       ` Samuel Ortiz
2009-10-22 10:55         ` Mark Brown
2009-10-22 11:02           ` Peter Ujfalusi
2009-10-21  8:56 ` [PATCH 0/4] twl4030 codec as MFD device Peter Ujfalusi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200910220904.32676.peter.ujfalusi@nokia.com \
    --to=peter.ujfalusi@nokia.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=sameo@linux.intel.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox