lkml.org 
[lkml]   [2009]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 3/5] MFD: twl4030-codec: APLL_INFREQ handling in the MFD driver
Date
On Monday 02 November 2009 19:30:35 ext Mark Brown wrote:
> On Mon, Nov 02, 2009 at 02:34:53PM +0200, Peter Ujfalusi wrote:
> > Move the APLL_CTL register configuration to the twl4030-codec
> > MFD driver.
> > Provide also a function for childs to query the audio_mclk
> > frequency.
>
> This all looks good to me, some nitpicks below.
>
> > +unsigned int twl4030_codec_get_mclk(void)
> > +{
> > + struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
> > +
> > + return codec->audio_mclk;
> > +}
> > +EXPORT_SYMBOL_GPL(twl4030_codec_get_mclk);
>
> As I said in my followup to patch 5 this feels like it should have a
> parameter to specify the twl4030 though in practical systems it won't
> matter.

I agree that this does not look quite nice, but as you already mentioned, it is
highly unlikely that one system would have more than one twl series of PM chip
on board. Another reason is that we have other part of the twl, which needs
resources from the codec part, but it is not loaded through the codec MFD, so
providing the needed information is kind of tricky with that setup.

>
> > + if (!(pdata->audio_mclk == 19200000 ||
> > + pdata->audio_mclk == 26000000 ||
> > + pdata->audio_mclk == 38400000)) {
> > + dev_err(&pdev->dev, "Invalid audio_mclk\n");
> > + return -EINVAL;
> > + }
>
> Might flow more naturally with a switch statement?

Yes, true. This looks weird, I'll change it.

>

--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-11-03 14:41    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site