Re: [alsa-devel] [RFC/PATCH 0/1] ALSA SoC support for OMAP

2008-04-04 Thread Mark Brown
On Fri, Apr 04, 2008 at 12:22:27PM +0300, Jarkko Nikula wrote:

> Here is the second version of ASoC support for OMAP. Now based to dev branch 
> of
> git://opensource.wolfsonmicro.com/linux-2.6-asoc but applies to linux-omap
> by modifying Kconfig and Makefile in sound/soc/.

Looks good from an ASoC point of view, thanks - I've applied the patch
to the dev branch but won't push it anywhere for the time being.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [RFC/PATCH 1/1] ASoC: Add drivers for the Texas Instruments OMAP processors

2008-04-22 Thread Mark Brown
On Tue, Apr 22, 2008 at 09:48:45AM +0300, Jarkko Nikula wrote:
> "ext Tony Lindgren" <[EMAIL PROTECTED]> wrote:

> > The platform_device_register() should be done in board-*.c files.

> This is a valid point. I think it should be possible to have also in
> ASoC v1 e.g n810_mcbsp_aic33 driver whose probe function would be more
> or less like n810_soc_init now. At least sound/soc/fsl/mpc8610_hpcd.c is
> doing similar thing.

> Mark, any thoughts?

Leave it as-is for ASoC v1, the soc-audio device can't meaningfully be
defined in the platform data.  ASoC v2 lets you define a device for the
platform driver for the particular board (as well as for things like I2S
controllers on the SoC) which deals with this a lot better.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] Add support for TPS6235x support

2009-02-13 Thread Mark Brown
On Fri, Feb 13, 2009 at 02:15:53PM -0800, David Brownell wrote:

> So what's the plan for merging it then?  I suspect it
> relates to board-specific init ... regulator_register()
> having changed signature for the 2.6.30 merge window,
> and this FIXME remaining un-addressed:

Yes, it'll need to be updated for mainline.  I'd expect the updated
version to be submitted via linux-kernel to myself and Liam (it's Liam
who'd be doing the actual merge with Linus).

> >/* FIXME board init code should provide init_data->driver_data
> > * saying how to configure this regulator:  how big is the
> > * inductor (affects light PFM mode optimization), slew rate,
> > * PLL multiplier, and so forth.
> > */

> Maybe there should be a 
> header with that board-specific data, and the framework
> data that's now thankfully not required to be held in
> the platform_data.  (That init_data->driver_data was

I'd expect that would be done for whatever data is required - it's the
standard thing and it's not new for regulator drivers either.  I've no
specific knowledge of this chip so I can't really assess what's vital to
get implemented prior to merge and what can be added in further patches
later.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] Add support for TPS6235x support

2009-02-14 Thread Mark Brown
On Fri, Feb 13, 2009 at 03:41:05PM -0800, David Brownell wrote:

> Especially with 2.6.29-rc5 out now ... get it in the regulator
> merge queue, so it can cook for a bit in the MM tree.  I think
> the URL is

>   
> http://git.kernel.org/?p=linux/kernel/git/lrg/voltage-2.6.git;a=shortlog;h=for-next

Yes, that's right - branch for-next.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Configuring a TWL GPIO pin as an interrupt

2009-02-22 Thread Mark Brown
On Sat, Feb 21, 2009 at 11:33:43AM +0100, Koen Kooi wrote:

> Don't forget to use the proper API for it: 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=include/sound/jack.h;hb=master

Yes, please, but note that there's an ASoC wrapper for that in sound/soc
in -next which should be used instead of the raw jack reporting API.  It
separates the definition of the jack from the detection mechanisms since
the wiring is always going to be board specific - eg, headphone and
microphone may be exposed on one physical jack or as separate jacks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Configuring a TWL GPIO pin as an interrupt

2009-02-23 Thread Mark Brown
On Mon, Feb 23, 2009 at 12:07:25AM -0600, Lopez Cruz, Misael wrote:

> In the particular case of ALSA SoC, could the machine/board driver be a 
> better place to handle all GPIO/IRQ configuration? That driver also contains 
> only board specific code.--
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Normally the arch/arm code would configure any multi-function pins (this
is normally required for lowest power consumption) and the actual IRQ
requesting and so on would be done by the machine and/or codec drivers
(depending on how exactly the hardware implements this).
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators

2009-02-23 Thread Mark Brown
On Mon, Feb 23, 2009 at 12:45:44PM -0800, David Brownell wrote:

> > another with a TWL4030 driver using that API 
> > and a third patch making the core do something with that data.

> Best IMO to switch the last two around.  Effectively
> there'd be one patch "add new features to regulator
> core", followed by the first of a set of "implement
> those new features in the driver for regulator X".

> And in fact that's what I've done with the two patches
> I'll be sending in a moment.

The reason I'm suggesting splitting things up the way I am is that it
separates out the TWL4030 driver (which looks very mergable to me right
now) from the behaviour changes.  Ordering things that way makes it
clear what the dependency is.  Another way of splitting it out would be
to remove the new API from the TWL4030, make that the first patch, then
have further patches adding the new API and the TWL4030 code to use it.

I don't see any reason why the TWL4030 regulator support needs to be
blocked on adding the new API and it makes review easier to keep them
separate.

> > This 
> > would result in something which maintains consistent behaviour over all
> > regulator drivers,

> It's already consistent, even without such patches!!

> There is no driver which pays attention to regulator(_dev)
> constraints that does it any differently than this one.

That's because nobody's doing it at all; once we add a custom
implementation in one driver we then need to police each driver
individually for consistency with the new interface.  If the logic is
factored out then that problem doesn't exist and drivers don't need to
reimplement any of it.

> > > > Your current patch does also set constraints for the voltages if they
> > > > weren't there previously

> > > I was thinking that boards which don't need constraints
> > > shouldn't need to create any ... they could just pass on
> > > the capabilities of the underlying regulator.

> > For safety the regulator API won't do anything without being explicitly
> > told that it's OK to do so - if we were to do this we'd need to have an
> > explicit flag in the constraints which says that this is what to do.
> > Constraints are also permission grants.

> I like to avoid flags unless they're absolutely required.
> In this case my initial reaction is to say that hooking up
> the components in the first place was the permission grant.

The trouble is we don't know what's connected to the regulator without
being told - even if some of the components hanging off the supply are
visible to software that's no guarantee that all of them are.  Keeping
the responsibility in the hands of the machine driver helps ensure that
users have made a concious decision that their settings are OK for their
design.

> > Indeed.  Like I say, a very large proportion of the development of the
> > regulator API has been done on reference designs which are built in this
> > fashion, including both pluggable PMIC boards and other daughtercards.

> That doesn't seem to have escaped its development cage yet.  ;)

There's a couple on their way to mainline right now, should make it in
the next merge window hopefully.  Unfortunately they're for systems that
aren't very completely supported in mainline right now which makes them
less useful as examples than they might be.  There's also none of them
where there's any immediate prospect of more than one of the PMIC board
options going into mainline.

> > Normally the primary PMIC cards are handled with conditional code in the
> > machine file (either compile time or run time) or by registering drivers

> Fair enough.  I'd de-emphasize "conditional", since that sounds
> way too much like #ifdeffing.  The source code should just call
> something like is_pmic_cardX(), and not care how that works.

It's normally a combination of the two - normally you don't get any form
of board ID readback and you get things like mutually exclusive options
for I2C or SPI control due to shared multi function pin setup that can't
be autoprobed entirely safely, for example.  Once you've decided on the
control bus it's normally safe to do autoprobing, though.

> Then what would you call constraints by/from the regulator?

They're subsumed within the constraints supplied by the machine driver
at the minute.

> I suggest updating your terminology.  "machine constraints"
> would be much more clear for what's there now:  they relate
> to the machine.  Other constraints (regulator, consumer)
> relate to the other components ... the ones for which they
> are an adjective.

Yeah, I kind of agree.  To avoid confusion from changing the names I'd
be tempted to go for something like "regulator driver constraints" but
it's not desparately nice.

> I don't mind moving it ... later, after the regulator
> core has proper support for decoupling machine-specific
> constraints from regulator-specific ones.  I view that
> code as a workaround for a current limitation of that
> core, so like all such workarounds it should va

Re: Configuring a TWL GPIO pin as an interrupt

2009-02-23 Thread Mark Brown
On Mon, Feb 23, 2009 at 04:11:04PM -0600, Lopez Cruz, Misael wrote:

[Please fix your mail client to wrap lines at ~80 characters, it makes
your mails much easier to work with.]

> I think that if I move the platform_device registration from machine
> driver to board file I can append jack detection information (gpio
> pin, irq) through "platform_data" of "dev" field in platform_device
> structure. And then in the "probe" part in ASoC machine driver I can
> receive it.

Yes, though unless you actually have a generic ASoC machine driver that
works over multiple boards it's as well just skipping the platform data.
Look at s3c24xx_uda134x for an example of doing this.

If these are CPU side GPIOs that you're talking about you'll also want
to write the standard utility for using gpiolib for jack detection that
I've not got round to doing yet :)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators

2009-02-23 Thread Mark Brown
On Mon, Feb 23, 2009 at 02:43:16PM -0800, David Brownell wrote:
> On Monday 23 February 2009, Mark Brown wrote:

> > Yeah, I kind of agree.  To avoid confusion from changing the names I'd
> > be tempted to go for something like "regulator driver constraints" but
> > it's not desparately nice.

> Hence my suggestion:  {regulator,machine,consumer} constraints,
> going from bottom up.  They aren't driver constraints; they
> are hardware constraints:  regulators can't supply arbitrary
> voltages.

Trouble is that the term regulator gets very overloaded and causes
confusion.  There's also fun and games to be had with accuracy once you
start looking too closely at the discrete voltages.

> >  To be honest I'm not
> > 100% clear why this new feature is essential to supporting the TWL4030 -
> > I can see that it could be useful but I'm not clear on what makes it
> > essential for this driver.

> I never said it was "essential".  However it does simplify

Apologies, you didn't actually say that, no - I was reading between the
lines there.

> the core driver code a lot by moving a lot of error checks
> to the init code; the checks need to live someplace.  You're

I'm not sure I see that for the constraint tightening, and indeed the
TWL4030 set_voltage() operation does have a range check in it.  Unless
I'm missing something if the tightened constraint is usable then it
should flop out of the range based requests with the constraints left
unmodified.  The reason the TWL4030 driver still has the range check in
the set_voltage() operation is that it is checking to make sure that the
requests that come in can be satisfied from the discrete values the
regulator is able to produce which is a good thing to do.

> the one asking for them to be packaged as a new framework
> feature.

The change to add voltage range constraints if none were supplied is a
noticable policy change from the existing framework standard - it allows
machines to enable voltage changes without specifying what the valid
values are.  I'm not convinced that this is a good idea in the first
place and it will result in the opposite behaviour to the current core
code (which should end up erroring out in constraint checking at runtime).
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators

2009-02-24 Thread Mark Brown
On Mon, Feb 23, 2009 at 06:03:49PM -0800, David Brownell wrote:
> On Monday 23 February 2009, Mark Brown wrote:

> > The change to add voltage range constraints if none were supplied is a
> > noticable policy change from the existing framework standard - it allows
> > machines to enable voltage changes without specifying what the valid
> > values are. 

> "Whatever the hardware handles" *is* a specification.

> And there's no more assurance it's right than any
> other specification would be ... except that, as a

You're right that we can't guarantee that users get everything correct,
the goal is more to ensure that some machine specific checks have been
done and that the regulator API won't go off by itself and start doing
things since it has no idea at all what's supportable.

> rule, hardware designers like to avoid assemblies
> subject to trivial misconfiguration mistakes (like
> firing up a 2.5V-max rail at 5V).

The issue is what happens when software starts changing the
configuration, for static configurations it doesn't make any difference.

> > I'm not convinced that this is a good idea in the first 
> > place and it will result in the opposite behaviour to the current core
> > code (which should end up erroring out in constraint checking at runtime).

> Well, if you really dislike it so much, that can
> easily be removed.  Got any comments on the
> framework patch I sent?  I'll take that as the
> first one, even though it's a different thread.

Yes, I wanted to sleep on it.  I'll reply at some point today.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Configuring a TWL GPIO pin as an interrupt

2009-02-24 Thread Mark Brown
On Tue, Feb 24, 2009 at 11:37:47AM -0600, Lopez Cruz, Misael wrote:

> About adding GPIO functionality to jack detection, if the right
> place to request GPIOs and set its data direction is in board files
> (arch/*/mach-*/board-*.c), then there won't be much to do in jack
> code, only the irq request. Unless we move request and data direction
> setting to jack detection layer.

No, do that in the jack code.  The board code should only be doing
things like putting the pins into GPIO mode (if that's not the default)
and any other steps required to ensure that they don't do anything like
consume a lot of power without the audio driver loaded.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages

2009-02-24 Thread Mark Brown
On Mon, Feb 23, 2009 at 12:52:01PM -0800, David Brownell wrote:

> Use those methods to force machine-level constraints into bounds.
> (Example:  regulator supports 1.8V, 2.4V, 2.6V, 3.3V, and board
> constraints for that rail are 2.0V to 3.6V ... so the range of
> voltages is then 2.4V to 3.3V on this board.)

Being able to support this is definitely a win, thanks for looking at
this.

> + /* maybe force machine-wide voltage constraints to match the
> +  * voltages supported by this regulator.  use the regulator's
> +  * entire range for boards with no particular constraints.
> +  */

I'd really rather the second bit weren't here.  I'd like to see a
warning for the first part.

> + * @count_voltages: Return the number of supported voltage indices which
> + *   may be passed to @list_voltage().  Some indices may correspond to
> + *   voltages that are not usable on this system.
> + * @list_voltage: Return one of the supported voltages, in microvolts;
> + *   or negative errno.  Indices range from zero to one less than
> + *   @count_voltages().  Voltages may be reported in any order.

I'm having a hard time loving this interface for the driver.  It feels
fairly cumbersome to have to provide two functions to look up what I'd
expect to be static data - I'd be fairly surprised to see it change at
runtime.  I think I'd expect to see something more like the way ALSA
represents dB scales where the driver supplies a list of ranges that can
either be simple values or value ranges expressed as (start, step,
count).  That would cover both complicated cases like the TWL4030 and
the other common case with large regular ranges of voltages.

This mostly applies to the driver interface - on the consumer side it
feels a bit cumbersome to use but I can't think of anything that's
particularly better.  An interface that also allows consumers to ask if
particular ranges can be satisfied might help here - it'd be nice for
things like MMC that want to check for a relatively small set of
voltages or voltage ranges (which I'd expect is the common case).
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages

2009-02-25 Thread Mark Brown
On Tue, Feb 24, 2009 at 04:17:22PM -0800, David Brownell wrote:
> On Tuesday 24 February 2009, Mark Brown wrote:

> > > + /* maybe force machine-wide voltage constraints to match the
> > > +  * voltages supported by this regulator.  use the regulator's
> > > +  * entire range for boards with no particular constraints.
> > > +  */

> > I'd really rather the second bit weren't here.  I'd like to see a
> > warning for the first part.

> Fixable in an update.  I still think it's better to require less
> manual configuration, not more ... manual configuration is by
> definition error prone; requiring more manual configuration makes

This whole interface is structured around the idea that the consequences
of getting this wrong include things like lasting hardware damage.  This
hardware damage may not be immediately obvious but may develop over time
if components are kept running out of spec, either way it's not likely
to make users happy.

> systems be more fragile.  Which is why I wouldn't naturally want
> to see a warning:  it implies manual configuration is desirable.

If we weren't so reliant on the machine driver getting things right I'd
agree with you.  My thought here is that because there is room for human
error here it'd be good to use the information we do have to flag
potential problems to people, helping catch any mistakes that they make.

> > I'm having a hard time loving this interface for the driver.  It feels
> > fairly cumbersome to have to provide two functions to look up what I'd
> > expect to be static data - I'd be fairly surprised to see it change at
> > runtime. 

> It can be a function of configuration, and I didn't want to force
> such seldom-used data into wasteful standardized-format tables.
> Notice that with the twl4030 code, a functional interface takes
> less space ... and is more flexible, since it copes with the
> voltage options that are defined as "not supported".

Yeah, TWL4030 is pretty special here :/ .  The only gotcha I can see is
having to have a second table with only supported values in it for the
case where you're not allowing the out of spec values but TBH I don't
see that as a big deal.

> Consider also the TPS6235x regulators:  the voltages they support are
> a simple linear function of an index 0..63, and that driver handles
> seven such chips.  64 values * 4 bytes * 7 chips == about 2KB of
> table data ... vs a few dozen bytes of function code.

That's actually the most common case in the regulators I'd seen and is
why I'd suggested doing something like the ALSA dB scales which handle
this nicely - you don't need to actually have a table with all the
values, you just provide parmeters that expresses each sub range.  You
say things like

   DECLARE_TLV_DB_SCALE(mix_tlv, -1500, 300, 0);

As far as hardware requirements go I've seen regulators which provide:

 - A set of irregularly distributed values (usually fairly small).
 - A range of regularly distributed values.
 - A large range of values with several regular ranges in it (usually
   you get higher resolution at the low end).

Either way can be made to work for all of these, the concerns I have are
that the fact that it's a function based interface makes it look like
this might be dynamic data and that it's exposing a bit too much of the
implementation details (see below) which made that suggestion seem even
stronger.

[ALSA dB scale style]
> Another virtue of functional interfaces for enumeration is
> they avoid the need for callers to see and handle a variety
> of different models, like that!!

I'd expect the core to deal with unrolling the data rather than the
consumers, this is why...

> > This mostly applies to the driver interface - on the consumer side it
> > feels a bit cumbersome to use but I can't think of anything that's
> > particularly better. 

> There's a LONG history of functional iterator models, such as
> the one I used.  I think they are clearly better, and don't
> understand why you'd prefer that more cumbersome approach.

...I am, as I say, reasonably OK with it on the consumer side.

The only issue I have with it on the consumer side is that the invalid
slots in the array are visible to users since it feels icky to have
error conditions be part of the normal iteration process for what should
be well known constant data.  I worry that it's going to catch people
out since relatively few regulator drivers do that (the fact that it's
there is an implementation detail for drivers which have holes in the
range of register values they can set).

Thinking about it that could be hidden by mapping the invalid values to
zero or some value that is actually valid instead of returning an er

Re: [alsa-devel] [PATCH] ASoC: Add support for OMAP3 EVM

2009-02-25 Thread Mark Brown
On Wed, Feb 25, 2009 at 03:25:59PM -0500, George G. Davis wrote:

> Is someone working on updating ASoC drivers for OMAP TWL4030 based
> boards as recommended above?  Just curious since I already made the
> mistake of creating my own OMAP3 EVM ASoC driver w/o checking the
> list first.  : /

Not to my knowledge.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages

2009-02-25 Thread Mark Brown
On Wed, Feb 25, 2009 at 02:12:26PM -0800, David Brownell wrote:
> On Wednesday 25 February 2009, Mark Brown wrote:

> > This whole interface is structured around the idea that the consequences
> > of getting this wrong include things like lasting hardware damage.  This
> > hardware damage may not be immediately obvious but may develop over time
> > if components are kept running out of spec, either way it's not likely
> > to make users happy.

> And as I noted:  *hardware* designers don't like to make
> such goofage possible.  So that's not a common scenario.

That's often not possible with software controllable regulators - it's
usually hard to stop them being set to any value they support and the
desire for any additional protection needs to be traded off against
power consumption and space and BoM costs.

> Not for me.  I had seen two and three bit voltage selector fields,
> defining fairly irregular sets of voltages.

> I think the rationale has to do with getting better system-wide
> efficiency, to stretch battery life.  Circuits generating the
> reference voltages can be more efficient if they don't need to
> be continually adjustable over some range(s).

It's certainly not the common case for regulators that people are
contributing to the kernel (most of which seem to be intended to be
primary PMICs).  Make of that what you will :)

> > As far as hardware requirements go I've seen regulators which provide:

> >  - A set of irregularly distributed values (usually fairly small).
> >  - A range of regularly distributed values.
> >  - A large range of values with several regular ranges in it (usually
> >you get higher resolution at the low end).

> All of which model nicely as a mapping { selector --> voltage }.

> Hardware probably even has a register bitfield holding selector
> values.  Maybe in that third case there's a second bitfield to
> hold selector bits which specify the range.

Yes, you can clearly always do selector->voltage since there's going to
be a finite number of register bits that it'll be possible to set.

> > Either way can be made to work for all of these, the concerns I have are
> > that the fact that it's a function based interface makes it look like
> > this might be dynamic data and that it's exposing a bit too much of the
> > implementation details (see below) which made that suggestion seem even
> > stronger.

> That still doesn't make sense to me.  It doesn't say a thing
> about what it *is* ... just how to find the voltage associated
> with a given index/selector.  

A function that return errors suggests something non-static to me.

> > I'd expect the core to deal with unrolling the data rather than the
> > consumers, this is why...

> I don't see why the core should "unroll" anything at all!
> The regulator driver is already doing that for get_voltage:

>   get_voltage() {
>   read selector from hardware
>   map selector to voltage
>   return that voltage
>   }

> So it's trivial for similar code to take the selector as
> a function parameter, and do the same thing.  Repackage
> the existing code a bit; bzzt, done!

Yes, that's a reasonable point (though I'd still like to see the maximum
turn into a static value now I think about it).

> > I worry that it's going to catch people 
> > out since relatively few regulator drivers do that (the fact that it's
> > there is an implementation detail for drivers which have holes in the
> > range of register values they can set).

> It will be fairly common for the regulator to support voltages
> that are disallowed by the machine constraints, though.  That
> can produce "holes" too; and not necessarily only for the lowest
> or highest selector codes.

At present only continous ranges are possible, though.  I can't think of
any systems I've seen that'd want discontinous constraints, though I'm
sure there are some.

> > Thinking about it that could be hidden by mapping the invalid values to
> > zero or some value that is actually valid instead of returning an error
> > - not entirely nice but it keeps the pain away from the consumers.

> The test for an invalid voltage is "v <= 0" regardless.

If you're looking for a bound you'd just check for things within that
bound anyway.  It's if there's explicit "this is an error" return that
people start wanting to do something with it rather than silently ignore
it (we hope, anyway).

> > You can either write the loop the way you have by iterating over the
> > voltages offered or you can write it by a

Re: [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages

2009-02-26 Thread Mark Brown
On Wed, Feb 25, 2009 at 05:02:03PM -0800, David Brownell wrote:

> Oh, one more comment.  Requiring manual configuration
> of fixed-voltage regulators is pure time-wastage.

Unless, of course, you happen to be using one of those consumers that
wants to know the voltage it's running at to configure itself.  Examples
I've seen include sensors and analogue components which can be
configured for better performance if they know the voltage they run at -
and yes, people do run these things off regulators that they vary at
runtime.

If you want to submit a patch making it optional that's fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages

2009-02-26 Thread Mark Brown
On Wed, Feb 25, 2009 at 03:47:52PM -0800, David Brownell wrote:
> On Wednesday 25 February 2009, Mark Brown wrote:

> In terms of the consumer interface, not -- "struct regulator"
> is opaque to consumers, and everything is a functional accessor.
> So I'll leave that as-is.

Yes, obviously.

> > At present only continous ranges are possible, though. ?I can't think of
> > any systems I've seen that'd want discontinous constraints, though I'm
> > sure there are some.

> Consider a regulator where voltage selectors 0..3 correspond to
> voltages

>   { 3.3V, 1.8V, 4.2V, 5.0V }

> With machine constraints that say voltages go from 3V to 4.5V ...

Continuous ranges of voltages, not continous indexes.  The indexes are
meaningless.  At the minute consumers and machines always supply
constraints as min,max pairs.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/3] ASoC: Add GPIO support for jack reporting interface

2009-02-26 Thread Mark Brown
On Thu, Feb 26, 2009 at 01:57:03AM -0600, Lopez Cruz, Misael wrote:

>  struct snd_soc_jack_pin {
> + struct snd_soc_jack *jack;
> + struct snd_soc_jack_gpio *gpio_pin;
>   struct list_head list;
>   const char *pin;
>   int mask;
>   bool invert;
> + /* GPIO */
> + unsigned int gpio;
> + unsigned int irq;
> + unsigned long irqflags;
> + irq_handler_t handler;
> + struct work_struct work;
>  };

This needs to be rethought - it breaks the abstraction layers.

There are three things working together here:

 - The snd_soc_jack, which represents a physical jack on the system and
   is what is visible to user space.
 - The snd_soc_jack_pin, which represents a DAPM pin to update depending
   on some of the status bits supported by the jack.  Each snd_soc_jack
   has zero or more of these which are updated automatically.
 - The jack reporting mechanism, which represents something that can do
   detection - it is associated with a snd_soc_jack, reporting a subset
   of the status bits supported by the snd_soc_jack.  Each jack may
   have multiple reporting mechanisms, though it will need at least one
   to be useful.

These are all hooked together by the machine driver depending on the
system hardware.  The machine driver will set up the snd_soc_jack and
the list of pins to update then set up one or more jack detection
mechanisms to update that jack based on their current status.

For example, a system may have a stereo headset jack with two reporting
mechansms, one for the headphone and one for the microphone.  Some
systems won't be able to use their speaker output while a headphone is
connected and so will want to make sure to update both speaker and
headphone when the headphone jack status changes.

The GPIO jack detection code should operate only on a snd_soc_jack that
is provided to it by a machine driver - you'll need to define a
structure to hold the information the GPIO jack detection needs (there
is a structure there but you've not defined it so I'm not sure what you
have in it at the minute).

Please look at the wm8350 headphone detection for an example of how to
integrate a detection mechanism.

> +static void gpio_work(struct work_struct *work)
> +{
> +   struct snd_soc_jack_pin *pin;
> +   int report;
> +
> +   pin = container_of(work, struct snd_soc_jack_pin, work);
> +   report = pin->jack->status & pin->mask;
> +   if (gpio_get_value(pin->gpio))
> +   report |= pin->mask;
> +   else
> +   report &= ~pin->mask;
> +
> +   snd_soc_jack_report(pin->jack, report, pin->jack->jack->type);
> +}

The value to report should be supplied by the machine driver.

BTW, please remember to CC the maintainers of the things you're
submitting patches for.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems while designing TPS65023 regulator driver

2009-02-26 Thread Mark Brown
On Thu, Feb 26, 2009 at 02:41:54PM +0530, Aggarwal, Anuj wrote:

> Since all the five regulators can be controlled using a single i2c
> device, I made a single i2c_board_info structure in my platform
> specific file and put all the regulator_init_data information there:

This is very common - most of the devices that have multiple regulators
also have some other subsystems on them (eg, an RTC or a watchdog) and
use a core driver in drivers/mfd with the individual functions of the
device as child platform drivers so this hasn't come up much.

> Now, the problem is in the tps_65023_probe function. Since it will be
> called only once as there is only one i2c device, I have to register
> all the regulators in that only. But I am not able to communicate the
> same to the regulator core layer. Inside the regulator_register(),
> variable init_data, which equals to dev->platform_data, is always
> pointing to the first array member, which is coming from the evm
> specific file. And it fails to register my second regulator instance,
> set_consumer_device_supply() specifically failing for the second
> iteration. Because of this, the probe function fails.

> How should I handle this scenario? Am I missing something in my 
> implementation?

Use -next or the regulator git at:

git://git.kernel.org/pub/scm/linux/kernel/git/lrg/voltage-2.6

There the init data is passed as a parameter to regulator_register()
rather than being read from the platform data so the problem goes away.
The relevant commit is 8ec143c801ff0514ce92e69aa2f7bd48e73b9baa.

[Please fix your mail client to wrap at 80 columns - currently you have
no line breaks in paragraphs which makes your mails a bit hard to read
and reply to.]
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages

2009-02-26 Thread Mark Brown
On Thu, Feb 26, 2009 at 10:56:05AM -0800, David Brownell wrote:
> On Thursday 26 February 2009, Mark Brown wrote:

> > Unless, of course, you happen to be using one of those consumers that
> > wants to know the voltage it's running at to configure itself.

> In which case it can ask the regulator what voltage it's using.  :)

I suspect we're talking at cross purposes here - I was talking about the
fixed voltage regulator driver.  I suspect you were talking about
something else?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc6 1/2] regulator: enumerate voltages (v2)

2009-02-26 Thread Mark Brown
On Thu, Feb 26, 2009 at 11:48:36AM -0800, David Brownell wrote:

> Updates since previous version:  address feedback, simplify.

Acked-by: Mark Brown 

This looks good to merge to me - coincidentally I've got a use case for
it lined up already.  Might it be worth merging the MMC client along
with this patch if the relevant maintainers are OK with that, could help
get it in faster?

Just two very minor points which might be nice to fix at some point:

> + cmin = INT_MIN;
> + cmax = INT_MAX;
> + }
> +
> + /* else require explicit machine-level constraints */
> + else if (cmin <= 0 || cmax <= 0 || cmax < cmin) {

That indentation is going to catch some people out :)

> + /* final: [min_uV..max_uV] valid iff constraints valid */
> + if (max_uV < min_uV) {
> + pr_err("%s: %s '%s' voltage constraints\n",
> +__func__, "unsupportable", name);
> + ret = -EINVAL;

That style is going to hurt grepability for the error.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch/rfc 2.6.29-rc6 2/2] regulator: twl4030 voltage enumeration (v2)

2009-02-26 Thread Mark Brown
On Thu, Feb 26, 2009 at 11:50:14AM -0800, David Brownell wrote:
> From: David Brownell 

> Update previously-posted twl4030 regulator driver to export
> supported voltages to upper layers using a new mechanism.

> Signed-off-by: David Brownell 

Looks good, thanks!

Acked-by: Mark Brown 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/3] ASoC: Add GPIO support for jack reporting interface

2009-02-27 Thread Mark Brown
On Fri, Feb 27, 2009 at 04:52:35AM -0600, Lopez Cruz, Misael wrote:

> * Create a new structure "snd_soc_jack_gpio" holding info specific for a
>   gpio pin like: gpio, irq, irqflags, irqhandler, private data (to be
>   passed to irqhandler).

Yes, roughly.  The jack_gpio will also need to know the status bits to
update and which jack to update.  I'd expect something along the lines
of:

struct snd_soc_jack_gpio {
struct snd_soc_jack *jack;
int report; /* Value to report when jack detected */
int invert_report;  /* Report presence when GPIO low */
int gpio;   /* GPIO to read */
};

possibly with some other data stored (eg, a debounce time).  You can use
gpio_to_irq() to get the interrupt number.

If the machine drivers need to customise the IRQ handler code itself
then it's probably getting to the point where another detection method
should be written, though perhaps I'm missing something?

> * Create a new function "snd_soc_jack_add_gpios" to add all jack_gpios that
>   belong to a specific jack. This function should add all gpio pin references
>   in a linked list as it's done for dapm pins. The linked list will be
>   useful to be able to release acquired resources in another function
>   "snd_soc_jack_free_gpios".

Since the detection mechanism will need to know the jack it's notifying
it should be possible to set up every GPIO detector at once - they'll
all have to know which jack to point to anyway.  Given that it'd be as
easy to use an array and not bother with the linked list.  The reason
the pins are added to a list is that we need to iterate over them all
whenever the jack status changes to update the status of the pins.

> * Machine driver will be responsible to call add_gpios function passing an
>   array of gpios related to each jack.

Yes, the machine driver should set up that link.

> * Machine driver will tie each jack_gpio with corresponding jack in a
>   machine specific jack_data structure, one hook per jack_gpio in the jack.
>   A handler will also be associated to the jack_data structure. This
>   jack_data struct will be passed to the gpio irqhandler as private data.

I'd *expect* you can live without the custom handler, though I could be
wrong.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/3] ASoC: Add GPIO support for jack reporting interface

2009-03-02 Thread Mark Brown
On Sun, Mar 01, 2009 at 07:54:36PM -0600, Lopez Cruz, Misael wrote:

> detection/report. There can be a generic detection/report function
> for gpio, I was thinking in something like:

Yes, that's more or less much what I was looking for.

> > Some systems won't be able to use their speaker output while a
> > headphone is connected and so will want to make sure to update
> > both speaker and headphone when the headphone jack status
> > changes.

> Having a single/generic report function like shown above (as is) we
> can't handle that case.

Sure it can - remember that the DAPM updates are done separately to the 
detection and can be set to invert the status reported for the power.
The speaker won't be visible as a jack in a situation like this.

> Could we leave the actual implementation of this report function to
> the machine driver? Since the things being done in detection function
> are common (even if other status are wanted to be updated), then
> probably machine driver could define a specific function ("action")
> for doing extra tasks, it can be called from generic gpio detect
> function. Could it be a valid approach?

That sounds like adding a callback for power updates on the jack itself
to me (which isn't a bad idea), rather than changing the report function
of the jack detection method.  The need for machine-specific extra
actions probably isn't specific to jacks that are detected via GPIOs.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/3] ASoC: Add GPIO support for jack reporting interface

2009-03-02 Thread Mark Brown
On Mon, Mar 02, 2009 at 03:16:22PM -0600, Lopez Cruz, Misael wrote:

> > That sounds like adding a callback for power updates on the jack itself
> > to me (which isn't a bad idea), rather than changing the report function
> > of the jack detection method.  The need for machine-specific extra
> > actions probably isn't specific to jacks that are detected via GPIOs.

> In that situation, power updates should come only when the jack reporting
> bits are either all active (jack enabled) or none (jack disabled), is that
> correct?

No, the jack detect bits can change independently - you won't always
physically be able to get everything that can be detected to report at
once (eg, something that can distinguish between headphones and line
output) or things may not always be present together (eg, a jack could
detect headphones but no microphone even if it's possible that both may
be present simultaneously).

> If so, then machine drivers can create callbacks receiving the soc_codec
> the jack belongs to and the current state of the jack. All the power updates
> (dapm_enable_pin/damp_disable_pin) will happen in the callback in machine
> driver but the dapm sync will happen in soc jack framework (i.e. when
> reporting current status of the jack).

No.  The framework takes care of doing the power updates already, there
is no need to add that functionality since the machine drivers can just
use a simple data table.  A callback could be used to do things that
can't be coped with already or more complex updates that aren't simply
directly mirroring the status in DAPM pins.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/3] ASoC: Add GPIO support for jack reporting interface

2009-03-02 Thread Mark Brown
On Mon, Mar 02, 2009 at 06:03:12PM -0600, Lopez Cruz, Misael wrote:

> Then, when to trigger the callback? Every time jack status is going to
> be updated?

Yes.  Note that if we're going to add a callback it should be done as a
separate patch since it's a separate feature.  It's probably as well to
get the GPIO jack detection and support for your system merged and then
worry about any callback separately.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] ASoC: Add DAPM machine widgets to SDP3430 driver

2009-03-04 Thread Mark Brown
On Wed, Mar 04, 2009 at 11:39:07AM -0600, Lopez Cruz, Misael wrote:
> Add DAPM machine domain widgets to SDP3430 machine driver.
> Interconnection:
> * Ext Mic: MAINMIC, SUBMIC
> * Ext Spk: HFL, HFR
> * Headset Jack: HSMIC, HSOL, HSOR

> Signed-off-by: Misael Lopez Cruz 

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

2009-03-04 Thread Mark Brown
On Wed, Mar 04, 2009 at 11:39:12AM -0600, Lopez Cruz, Misael wrote:

> +/* Headset jack */
> +struct snd_soc_jack *hs_jack;

This (and all your other globals) should be declared static.

> +/* Headset jack DAPM pins */
> +struct snd_soc_jack_pin hs_jack_pins[] = {
> + {
> + .pin = "Headset Jack",
> + .mask = SND_JACK_HEADSET,
> + .invert = 0,
> + },
> +};

No need to initialise things to zero, that's the default for static
variables.

> + /* Headset jack detection */
> + hs_jack = kzalloc(sizeof(struct snd_soc_jack), GFP_KERNEL);
> + if (!hs_jack)
> + return -ENOMEM;

If you declare hs_jack as a direct static variable you won't need to
kzalloc() it and save some error checking and cleanup code too :)

> + ret = snd_soc_jack_new(&snd_soc_sdp3430, "SDP3430 headset jack",
> + SND_JACK_HEADSET, hs_jack);
> + if (ret)
> + return ret;

This leaks the jack.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

2009-03-06 Thread Mark Brown
On Thu, Mar 05, 2009 at 11:32:31AM -0600, Lopez Cruz, Misael wrote:
> Add headset jack detection for SDP3430 boards using SoC jack
> reporting interface. Headset detection on SDP3430 board is
> achieved through TWL4030 GPIO_2 pin.

Applied, thanks.  One comment:

> +/* Headset jack detection gpios */
> +static struct snd_soc_jack_gpio hs_jack_gpios[] = {
> + {
> + .gpio = (OMAP_MAX_GPIO_LINES + 2),

It feels like there should be a nicer way of specifying the GPIO
number?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems while designing TPS65023 regulator driver

2009-03-06 Thread Mark Brown
On Thu, Mar 05, 2009 at 05:33:55PM +0530, Aggarwal, Anuj wrote:

[Please fix your mail client to wrap lines at ~80 columns - not doing so
makes your mails much harder to read and reply to.]

> But still when I call regulator_disable() after doing a _get() on it,
> the call fails saying " unbalanced disables for supply". Then I checked
> the same repository again and found commit
> 38db9f31d6dc6147b87692b3b5a8a32de1a6cbe6 (regulator: Allow boot_on
> regulators to be disabled by clients). But still, it is not allowing me
> to disable the regulator as soon as I do a get on it. 

You'll need to do an enable followed by a disable for the benefit of the
reference counting that is done for the consumer usage.  What is the
consumer driver here?

> Later, I found out that in set_machine_constraints(),ops->enable() is
> being called if the boot_on flag is set. What is the purpose of doing
> this? Since the regulator is already enabled, why we are calling the
> ops->enable() to do the same again? In my opinion, regulator_enable()

This ensures that the regulator is actually turned on.  Previously
boot_on was equivalent to always_on and there was no way for a machine
driver to turn a regulator on at startup so the semantics of boot_on
were changed slightly to be usable to switch a regulator on at boot.

We could check to see if the regulator is already enabled but it didn't
really seem worth it - if it's a problem a check could be added to query
to see if the regulator is enabled before applying the boot/always on
constraints.

> should have been called to let the framework increase its usage count so
> that the user can disable the same as and when required.

This wouldn't do what you want - the regulator reference counts are two
level, they're counted in the consumer and then the regulator counts the
number of consumers which enable it.  If the core uses regulator_enable
that means it has a consumer allocated for the regulator and that
consumer will end up forcing the regulator to be always on (this was
essentially what the previous boot_on implementation ended up doing).

Consumers need to enable regulators they want to use even if they are
already enabled since otherwise the core may decide to disable the
regulator due to the action of some other consumer which is sharing the
supply.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] regulator: twl4030 VAUX3 supports 3.0V

2009-03-06 Thread Mark Brown
On Fri, Mar 06, 2009 at 11:16:20AM -0800, David Brownell wrote:

> Do you really need 3.0V out of that regulator?  If so,
> then I'd rather see a patch exposing that CONFIG_*
> setting to enable all the unsupported/out-of-range
> values, rather than just selectively hacking those
> tables to permit some (but not all) of them to be used
> out-of-range.

Would it make sense to make this platform data so that if a given board
requires running the chip like this it can be enabled for those boards
but it's not something people might turn on because it seems useful?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems while designing TPS65023 regulator driver

2009-03-07 Thread Mark Brown
On Fri, Mar 06, 2009 at 04:07:16PM -0800, David Brownell wrote:

> The boot_on semantics are kind of odd then ...

> What I thought they meant:  Bootloader turned this on.

That's still roughly the case, though in practice there's no actual need
to do this for the vast majority of regulators since it is possible for
the kernel to just read the status of the regulator back at runtime
which is obviously more reliable.

> What you describe above:Kernel turn this on during startup.

What's happening here is that the kernel is making sure that the
information it was given about the state of the regulator is actually
true in case it was important (things could drift if the bootloader or
hardware are improved to boot up with a better default configuration,
for example).  The kernel could warn here but we'd need to be clear why
the constraints tell us that the regulator is on.

This also has the side effect of allowing the constraints to turn the
regulator on at startup, perhaps to aid early boot or perhaps because
not all the drivers in the system that need it have regulator support
yet, but that wasn't the primary purpose.

> Versus normal behavior: Consumer turns it on, as needed.

That now works as normal.  Originally using boot_on would've had the
effect of setting an always_on constraint which really wasn't desirable
since we already have that.

> I wouldn't have thought there would be a need for that
> second case, since the board-specific init code can
> just define a consumer that turns it on if that's what
> it needs.

Yes, it could - or it could define an always_on constraint if that were
what's needed.  On the other hand, it's the sort of thing that more than
one board is going to need to do so it does make sense to factor it out
a bit.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems while designing TPS65023 regulator driver

2009-03-08 Thread Mark Brown
On Sun, Mar 08, 2009 at 12:54:35PM -0800, David Brownell wrote:

> but the bootloader turned the regulator on, then drivers
> can't disable the regulator (on penalty of a stackdump!)
> unless they issue a spurious/pointless/undesirable enable()
> beforehand ...

We can't easily have both reference counting and unbalanced disables,
sadly.

>  - Regulators not marked as "boot_on" or "always_on" won't
>be active (and usecount will be 0) on return from setup.

This breaks the idea that we don't do anything unless explictly told to
do so.  I did actually still consider adding code to power off the
regulator but thought that there may also be situations where the state
really is unknown (eg, it depends on what the system booted from) and
it'd be useful to be able to punt to the consumers to figure it out.

I'm a bit ambivalent on this one, though - avoiding a sprawl of options
is certainly neater.  An enum for the initial power state has an appeal
here.

> - /* are we enabled at boot time by firmware / bootloader */
> - if (rdev->constraints->boot_on)
> - rdev->use_count = 1;
> -

That's not there with the current regulator tree (this was the bug with
not being able to disable boot_on regulators, there's no way to drop
that use count later on).

Much of the rest of your patch will fail to apply due to similar
changes; the logic that's there now is roughly the same as what you have
here except we don't bother to check is_enabled() any more (no harm
adding that back, it'd be useful if enable() can't be called for an
already enabled regulator) and we don't disable the regulator.

> + } else if (is_enabled < 0) {
> + pr_warning("%s: hoping regulator '%s' is %sd...\n",
> +__func__, name, "enable");
> + }

I'm really not loving this %s for the enabled - yes, it'll save a small
amount of memory but it hurts gepability.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to test regulator driver?

2009-03-09 Thread Mark Brown
On Mon, Mar 09, 2009 at 08:32:55PM +0530, Anuj Aggarwal wrote:

> I want to test my regulator driver by writing a small kernel module.
> But I am a little confused as what should be passed as the first
> argument of regulator_get(). How would the kernel module know about
> the device pointer that needs to be passed to the _get function?

regulator_get() takes the struct device for the consumer as an argument
so it depends on what your consumer is.  For test purposes there's an
existing virtual consumer driver in the tree which should hopefully save
you having to write your own - it allows you to poke the settings from
sysfs.  drivers/regulator/virtual.c

Questions like this that aren't OMAP-specific should really be asked on
the relevant general list (in this case linux-kernel).
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems while designing TPS65023 regulator driver

2009-03-10 Thread Mark Brown
On Mon, Mar 09, 2009 at 04:45:26PM -0800, David Brownell wrote:
> On Sunday 08 March 2009, Mark Brown wrote:

> > >  - Regulators not marked as "boot_on" or "always_on" won't
> > >be active (and usecount will be 0) on return from setup.

> > This breaks the idea that we don't do anything unless explictly told to
> > do so.

> I'm not sure where you're drawing this "explicitly" line, but
> clearly it's not where I would draw it!  The board init code
> explicitly said "here's a regulator, with these settings for
> the boot_on and always_on flags".  If neither was set, they
> are obviously clear ... so the regulator shouldn't be enabled.

At the minute a zero initialised set of constraints means "don't touch
anything" - it doesn't grant any permissions to do anything, all that
can actually be done is inspect the state.  Some of the drivers use this
currently, having a block of regulator constraint data in a larger block
of platform data and registering all regulators on the chip
unconditionally.  Requiring boot_on or always_on to be set would mean
that these drivers would start powering everything off once the change
is merged unless the drivers are changed first.

If we are going to make this change it might be best to first spend a
release printing a big fat warning so it's harder for people to get
surprised by it, especially with stuff getting merged via platform
trees.

> > I did actually still consider adding code to power off the 
> > regulator but thought that there may also be situations where the state
> > really is unknown (eg, it depends on what the system booted from) and
> > it'd be useful to be able to punt to the consumers to figure it out.

The other use case I should've mentioned is for people who are reverse
engineering systems and initially want to fire things up and inspect the
state they get left with before they go figuring out what (if anything)
they want to do with it.  Even if you do know the design this can be
quite handy for testing that everything came up as expected, the kernel
provides a fairly convenient UI.

> The core problem with that thought is that if you try doing that,
> then consumers have exactly zero ways to fix the issue.  It's the
> scenario I listed above:  regulator is enabled, but refcount is
> zero.  So they're not allowed to disable.

It can do it by enabling (which is a noop) and then disabling - it's not
nice and wasn't really intentional but it gets the job done.

> That's in addition to the fact that "unknown" states are
> extremely error prone.  The state after initialization should
> fully known, without having to play such guessing games.

Yes, doing it via constriants is clearly better - I'm more thinking
about this in terms of "if you really want to do it this is how" than as
something I'd recommend people use.

> defined values.  Adding a second "always_on" flag makes for
> some confusion, since it only defines a third state, not a
> pair of states (it's not orthogonal).

We should just be able to remove always_on; it's equivalent to setting
boot_on and not enabling REGULATOR_CHANGE_STATUS.  I'll look into that
but it's got cross tree issues too.

> always have the consumer ops delegate to the real regulator.
> And have that real regulator's usecount set to one when it's
> enabled at boot time, so regulator_disable() will work then.

Clearly.  I'm wondering how that plays with multiple consumers, though.
Consumers will be able to disable regulators that were left on but
they'll need something to let them figure out why the device was left
on.  Or just not worry about supporting such users too strongly suggest
that they should be using something that gets added to the constraints.

Fancy kicking off a couple of new discussions on lkml?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc7-omap] OMAP1: ASoC buildfix for OSK

2009-03-11 Thread Mark Brown
On Wed, Mar 11, 2009 at 02:37:25AM -0800, David Brownell wrote:

> There is currently such an interface in mainline; but there shouldn't
> be ... which is why I'm sending this to the ASoC folk too (I think
> those clock updates will hit in the 2.6.30 window).

I've applied this, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ASoC: Complete Beagleboard support

2009-03-11 Thread Mark Brown
On Wed, Mar 11, 2009 at 06:50:18AM -0700, Steve Sakoman wrote:

> Any updates on the status of this patch?  Doesn't seem to have been applied 
> yet.

> I've seen lot's of folks on the Beagle mailing list and IRC channels
> who are getting bitten by this issue.

It's been applied:

http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commit;h=80c509fdd74f3b158267374cc55156965c8bf930
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

2009-03-12 Thread Mark Brown
On Wed, Mar 11, 2009 at 04:43:34PM -0800, David Brownell wrote:

> Buggy consumers could notice different bug symptoms.  The main
> example would be refcounting bugs; also, any (out-of-tree) users
> of the experimental regulator_set_optimum_mode() stuff which
> don't call it when they're done using a regulator.

I'm OK with this from a code point of view so

Acked-by: Mark Brown 

However any consumers that take advantage of this won't be able to
safely share a regulator without extra work since they have no way of
telling why a regulator is in the state that it's in without extra
stuff.  We should probably have something along the lines of a
regulator_get_exclusive() for them.  Previously the consumer counting
would have stopped them interfering with enables done by other
consumers.

There will be other consumers that can't safely share a regulator anyway
(eg, requiring additional code to notice and handle voltage changes) so
it'd be a good thing to have.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc7 regulator-next] regulator: init fixes

2009-03-12 Thread Mark Brown
On Wed, Mar 11, 2009 at 06:32:15PM -0800, David Brownell wrote:

> Make the regulator setup code cope more consistently with
> regulators undesirably left enabled by the bootloader.

First up I'd just like to make it absolutely clear that I agree that
this is a feature we should have - it's obviously useful.

>  * Unless the "boot_on" or "always_on" machine constraints
>were set, disable() the regulator.  This gives drivers
>a clean start state:  enable state matches usecount,
>regardless of boot_on/always_on flag state.

At the minute the regulator constraints have the property that if you
pass a zero-initialised set of constraints the regulator API will not do
anything other than allow you to read the state of the regulator - any
action taken by the regulator API must be explicitly permitted by code.
This change will mean that users will have to explicitly mark any
regulators that are needed as enabled or we'll do unfortunate things.  

It is a particular problem for multi-function devices like pcf50633
which not only register all their regulators by default but also embed
constraints within the general pcf50633 platform data.  If the user
simply turns on the regulator driver in their config they'll get this
behavior if they don't edit the code.  Even with regulator code I'd not
be surprised if people were bitten by this for things like the memory or
a CPU core without regulator based DVFS.

You've addressed some of the use cases for this by providing devmode but
it's still a big incompatible change, especially at this point in the
development cycle where we've got at most a couple of weeks to the merge
window.  We could do this without introducing the incompatibility by
adding a new flag (eg, boot_off) which machine constraints need to
explicitly set to enforce this behaviour or by having something machine
drivers can call to say "now power off any regulators that look idle".

I'm not 100% agaist doing things the way you suggest since there is 
appeal in having boot_on be more of a boolean but I do feel that if
we're going to go this way we should do it more gently.  For example, we
could merge a patch now which warns loudly that this will happen and
then after the 2.6.30 merge window actually implement it.  This would
reduce merge issues for machine support coming in via other trees; we're
rather late in the development cycle to be putting this in right now.
Not everyone will read a warning but there is some chance they might and
there's also a chance they'll test in -next.  We should also provide a
temporary Kconfig which actually enables the behaviour if people request
it.

>  * To help make some integration stages easier, add a new
>"devmode" machine constraint where state the bootloader
>left isn't touched, but enable state and usecount may
>not match.  (System boots but some drivers act odd ...
>debuggable.  System dies part way through booting ...
>often painful.)

This also addresses things like the reverse engineering use case where
people genuinely don't know what's going on and want to use the API to
inspect the state of the regulators.

I do wonder if we can't come up with a different way of expressing
devmode - a different name like dont_disable that more directly
expresses what the constraint does might be more obvious.  Something
that can enable this globally may also be convenient.

The code itself is OK.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

2009-03-12 Thread Mark Brown
On Thu, Mar 12, 2009 at 12:35:24PM -0800, David Brownell wrote:
> On Thursday 12 March 2009, Mark Brown wrote:

> > safely share a regulator without extra work since they have no way of
> > telling why a regulator is in the state that it's in without extra
> > stuff.

> Depends what you mean by "safely".  If they weren't buggy
> already, I don't see how they'd notice any difference.
> Having buggy consumers become non-buggy isn't exactly a
> job for the framework itself.

Previously the per-consumer reference count would've meant that they
couldn't interfere with other consumers - they could set their own
state but not reverse an enable something else had done.  Now there is
only one reference count but there's no way for a consumer to exclude
other consumers and nothing which protects against breakage.

> > We should probably have something along the lines of a 
> > regulator_get_exclusive() for them.  Previously the consumer counting
> > would have stopped them interfering with enables done by other
> > consumers.

> I'd like to see get()/put() match the design pattern used
> elsewhere in the kernel:  those calls signify refcount
> operations.

Aquiring a reference is obviously what we want in the rather common case
where the supply is shared.  We could name an operation that enforces
non shared supplies something else but at the end of the day it's going
to be the same operation.  The major purpose of adding an explicit call
for this is to document the requirement the consumer has for direct
control of what it's doing.

> Agreed that the "consumer" access model probably needs a few
> interface updates.  I'm not sure what they would be though;
> one notion would be to focus on the constraints they apply
> (including "enabled") instead of what they do now.

I'm not at all sure what you mean by this - constraint narrowing by the
consumers is pretty much exactly the model the existing code has.  We
need to do things like re-add the voltage handling that was removed pre
merge but that's already the programming model we have.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

2009-03-12 Thread Mark Brown
On Thu, Mar 12, 2009 at 03:02:55PM -0800, David Brownell wrote:
> On Thursday 12 March 2009, Mark Brown wrote:

> > Previously the per-consumer reference count would've meant that they
> > couldn't interfere with other consumers - they could set their own
> > state but not reverse an enable something else had done.

> They still can't ... *unless* they're already buggy.

As previously noted they could've worked happily via bouncing their
state to match the physical state first; it's not nice or a good idea
(which is why I am happy with your patch) but, well, not everyone pays
attention to warnings and errors :(

> And, as a new thing not currently addressed well in code, you
> are talking about "non-shared" handles.

You are missing my point here; this is mostly about documentation.  The
exclusive access issue is with devices that can't tolerate any
arbitration and need the regulator to go into the state they're
requesting - if the consumer is finding itself doing something like turn
off a regulator which it did not enable itself then it's clearly not
able to play nicely with other drivers sharing the same resource without
extra communication.  This may be because the driver is doing things
that aren't really appropriate and perhaps ought to be done via
constraints if at all; it may also be because the hardware that's being
controlled demands it.

If everyone is nice and careful about what they're doing it'll not make
any difference at all either way.  Especially in the hardware
requirements case I'd hope it never even comes up that it'd make a
difference.

> One could as easily have "handle" and "regulator" be the
> same ... so the get/put idioms could work like they do
> elsewhere in the kernel.

I really don't see that there is any meaningful difference here; from
the point of view of the consumer the fact that the thing it gets back
is a handle to a structure the core uses to keep track of the consumer
rather than the underlying hardware object is an implementation detail
that shouldn't make any difference to them.  In terms of the programming
model it seems like a layering violation to know the difference between
one opaque structure and another.

> See above.  Currently constraints are hidden for "consumers",
> behind functional accessors like regulator_set_voltage().
> There are no explicit constraint objects, as there are for
> the machines.

The current interface has been driven by the needs of the users: the
majority of consumers want to do one operation on a regular basis -
normally that's enable/disable, most devices are just powering
themselves up and down, though for some things voltage changes are much
more common (DVFS being the prime example).  Overall it's been fairly
similar to the clock API in terms of usage pattern.

In terms of looking at redesigning the API I feel we're getting ahead of
ourselves here and should be working more for stability until we run
into problems that force us to make big changes.  Right now it seems
better to focus on improving the support for real systems in mainline
and addressing any issues that they see so that we've got something to
learn from when doing any redesign and can minimise the amount of
integration churn that is created.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] SoC: Move headset jack registration to device initialization for SDP3430

2009-03-13 Thread Mark Brown
On Thu, Mar 12, 2009 at 09:45:27PM -0500, Lopez Cruz, Misael wrote:

> the card is registed. As a consequence of jack device registered
> properly, the jack is detected as an input device.

> Signed-off-by: Misael Lopez Cruz 

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Overo broken after recent mainline merge

2009-03-14 Thread Mark Brown
On Sat, Mar 14, 2009 at 12:08:30PM -0700, David Brownell wrote:

> The basic problem is that still-unfixed goofage in the regulator
> framework:  it seriously mis-handles regulators that bootloaders
> leave enabled.  This can be teased apart into at least two bugs,
> possibly as many as four.  The fix for one of them is queued in
> the regulator-next tree.

For clarity, what David is referring to here is that it's not currently
possible for the machine constraints to turn off a regulator that has
been left enabled when the kernel starts.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Overo broken after recent mainline merge

2009-03-14 Thread Mark Brown
On Sat, Mar 14, 2009 at 02:14:05PM -0700, David Brownell wrote:

> But thinking about how to handle the video support makes
> it clear that's not always the best solution.  One needs
> bootloaders to be able to throw a splash screen which
> stays active until the window system kicks in ... but
> turning off video regulators would clearly blacken the
> screen, which is the wrong model.  (And marking them as
> "always on" or "boot_on" is wrong for other reasons.)

Yes, this sounds like one of those cases where just leaving the
regulator alone and letting the drivers figure things out will probably
work best.

> The way clocks handle that is with a late disable,
> so drivers have a chance to start up.  You rejected

This behaviour is specific to the OMAP implementation of the clock API
(and is an optional feature of that from the looks of it).  It's
probably also worth rembembering that the clock API is working in a very
much more controlled domain (in that it mostly only covers well known
devices on a given architecture), requires little if any per-machine
setup and isn't an optional feature.  Most of the clock API setup is a
feature of the SoC CPU rather than of the board.

> the REGULATOR_DISABLE_UNUSED patch I sent, applying
> that model ... and that's why I started to think about
> other approaches, as in the "early disable" patch I
> sent earlier in this thread.

I didn't reject the concept - I asked for some changes in the interface
to make it be something that the machine drivers can control rather than
have it be a Kconfig option that's selected by end users and can't be
part of the power description that the machine has to have anyway.  As I
said at the time if it were something that could be enabled by the
machine driver, either per regulator or per system, then I'd be happy
with this approach.  I think that it is a better approach than the one
you're currently going for.

Unlike the clock API the machines are going to have to provide some
configuration for the regulators for this feature to be useful (to
specify the supplies that are actually in use) so there is little
additional cost to them in requesting this feature.  If it were only a
Kconfig option then people building the kernel have to know somehow how
well set up the regulators are for their system and some people will end
up with suboptimal configurations because they don't
know if they can turn the option on or not.

> A third approach (after applying that one patch that's
> in the regulator-next tree) would be

>   if (rdev->is_enabled() > 0)
>   rdev->use_count = 1;

That won't work with consumers that can share their supply - they can't
tell if the regulator was enabled because it was left on at startup or
if it was enabled by some other consumer.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc7 regulator-next] regulator: refcount fixes

2009-03-14 Thread Mark Brown
On Sat, Mar 14, 2009 at 02:29:24PM -0700, David Brownell wrote:

> You're not stepping back from the current interface, which is
> a prerequisite to understanding the points I was making:

I'm pretty sure I understand exactly what you're saying but we just
disagree on this one.  Probably best that we agree to disagree.

>  * Almost everywhere else in the kernel, there's only one
>handle (no per-client facet idiom), for which get/put
>works.

Looking at things from the point of view of the consumer I just don't
find that it makes any difference since as far as the consumer is
concerned it's all opaque objects manipulated via an API.  I certainly
don't experience any conceptual jar switching between this and things
like the clock API, the patterns in clients the same especially for a
basic consumer doing something along the lines of:

foo = foo_get(dev, name);
foo_enable(foo);
foo_disable(foo);
foo_put(foo);

Even once you start setting more properties in there I'm struggling to
see the difference being visible.

I feel that you are focusing too much on the implementation here, not
the interface, but like I say I think we're just going to have to agree
to disagree on this one.

>  * The thing that *is* per-client is basically a constraint
>set ... but it's called a "regulator", which again is
>confusing.

You could go for regulated_supply or something.  I think that at this
point it's just not worth the trouble to try to change the name, though.

If it helps think of the per client object as representing the
particular physical supply to the physical device.  We could represent
them internally as pass through regulators but since the implementation
should still be opaque to the consumers I don't think that it's worth
doing that unless it buys us something in the implementation.

I'm not overly concerned about the implementation right now since it's
not causing any problems and the opacity we have now for the consumers
supports later refactoring.  Things like the issues with working with
struct device for I2C and SPI devices seem to be causing far more
pressing problems in actual use.

> In the regulator-next tree you've now moved regulator_dev
> into the public interface ... that's the handle to the
> real hardware.  Sort of a hint that it can't really be
> hidden in the way you originally thought.

It's only exposed to the drivers for the regulator hardware; it's not
visible to consumers unless they go peering into the driver API.  The
drivers for the regulators have always had a direct handle on themselves
for obvious reasons - the only change here is that they now know a bit
more about the implementation.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc8 regulator-next] regulator: init fixes (v4)

2009-03-14 Thread Mark Brown
On Sat, Mar 14, 2009 at 05:25:35PM -0700, David Brownell wrote:

> + } else if (ops->is_enabled) {
> + /* ... if the bootloader left it on, drivers need a
> +  * nonzero enable count else it can't be disabled.
> +  */
> + ret = ops->is_enabled(rdev);
> + if (ret > 0)
> + rdev->use_count = 1;
> + ret = 0;

This means that drivers that do balanced enables and disables will never
be able to cause the regulator to actually be disabled since there will
always be this extra reference count there.  Without this patch what'll
happen with those drivers is that they'll do an enable then later on
when the last one disables its supply the reference count will fall to
zero and the regulator will be disabled.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] twl4030-regulator: expose VPLL2

2009-03-16 Thread Mark Brown
On Fri, Mar 13, 2009 at 05:54:54PM -0700, David Brownell wrote:
> From: David Brownell 
> 
> Add VPLL2 to the set of twl4030-family regulators exposed for
> use by various drivers.  It's commonly used to power the digital
> video outputs (e.g. LCD or DVI displays) on OMAP3 systems.
> 
> Signed-off-by: David Brownell 

Acked-by: Mark Brown 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc8 regulator-next] regulator: init fixes (v4)

2009-03-16 Thread Mark Brown
On Sat, Mar 14, 2009 at 09:05:29PM -0700, David Brownell wrote:

> was called.  It's not exactly hard to check if it was enabled, then
> act accordingly, in the typical "single consumer of the regulator"
> case.

How typical that is depends very much on the devices you're looking at.
Devices that need to do things like set voltages are fairly likely to
own the regulator but with devices that just need to ensure that they
have their supplies enabled it's much more likely that the supplies will
be shared.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc8 regulator-next] regulator: init fixes (v4)

2009-03-17 Thread Mark Brown
On Tue, Mar 17, 2009 at 11:15:06AM -0700, David Brownell wrote:
> On Monday 16 March 2009, Mark Brown wrote:

> > Devices that need to do things like set voltages are fairly likely to
> > own the regulator but with devices that just need to ensure that they
> > have their supplies enabled it's much more likely that the supplies will
> > be shared.

> Right.  Do you have a model how such shared supplies would
> coexist with the "enabled at boot time" model, and still
> support being disabled?

The drivers can essentially ignore the physical status of the regulator
when they start, enabling when they need them and disabling when they're
done.  So long as at least one device has the regulator enabled the
regulator will remain on but when the reference count drops to zero then
the regulator will be disabled.

This works well when the driver will be enabling the regulators at
startup and then disabling them later on.  It will also work well with a
late_initcall which disables any unreferenced regulators - that should
become the default at some future point (2.6.31 might be a good candiate
here, I posted a patch the other day providing an implementation which
warns if there are affected regulators and which machines can activate
if they want).

> My first thought is that the system designer should avoid
> such boot_on modes.  But that's not always going to work.

Yes, that's not something that can realistically be achieved in the
general case.  Machines should probably avoid it but often people want
to do things like bring LCDs up in the bootloader and providing graceful
handover to the actual driver helps the user experience.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc8 regulator-next] regulator: init fixes (v4)

2009-03-18 Thread Mark Brown
On Wed, Mar 18, 2009 at 12:25:11PM -0700, David Brownell wrote:
> On Tuesday 17 March 2009, Mark Brown wrote:

> > The drivers can essentially ignore the physical status of the regulator
> > when they start,

> That is, shared supplies should adopt a different model?

I think that's a bit strong, once we get past init the problem pretty
much goes away AFAICT.

> That approach can't be used with drivers, as for MMC slots,
> which need to ensure they start with a "power off" state as
> part of a clean reset/init sequence.

Pretty much.  They could cope if they used the enable/disable bounce
hack but if they urgently need to have a specific power state they won't
be able to cope with shared regulators anyway so it doesn't really make
any odds.

> Maybe "sharable" should be a regulator constraint flag, so
> the regulator framework can avoid committing nastiness like
> allocating multiple consumer handles for them.

Or vice versa - it's as much a property of the consumer driver that it
requires exclusivity.  From the point of view of the regulator API
there's very little difference between the two cases.

Note that for well behaved consumers that use mapped supply names we
already know about them all in advance anyway...

> > It will also work well with a 
> > late_initcall which disables any unreferenced regulators -

> The $SUBJECT patch will prevent such things from existing.

I sent a patch backing that specific change out along with the
late_initcall() patch.

> Also, regulator use that kicks in before that particular
> late_initcall will still see self-inconsistent state in
> the regulator framework ... of course, $SUBJECT patch (and
> its predecessors) is all about preventing self-inconsistency.

A driver that can cope with sharing the regulator is not going to be
able to observe anything here: it must always enable the regulator when
it is using it even if it is already enabled (since otherwise some other
consumer could cause it to be turned off) and it should expect that the
regulator might be enabled by something else.  It can't do an unbalanced
disable since otherwise it could be reversing an enable something else
did.  It's also not possible for it to inspect the use count.

If a consumer can't play with that model then I'm reasonably happy with
it having to state any additional requirements it has in some way - if
nothing else it gives us a bit of documentation about what's going on.

> That self-inconsistency doesn't seem to concern you much.

I think it's more that I'm viewing the use count as being useful
information which the API can take advantage of ("do any consumers
actually want this on right now?").  I think we should be able to handle
this without changing the use count and that this is preferable because
otherwise we make more work with shared consumers, which should be the
simplest case.

The trick is getting the non-shared regulators into sync with the
internal status, ideally without doing things like bouncing supplies
during init.  I think either using regulator_force_disable() or saying
that the consmer wants exclusive access on get and then bumping the use
count for it if the regulator is already enabled ought to cover it.
I've not thought the latter option through fully, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] ASoC: Declare Headset as Mic and Headphone widgets for SDP3430

2009-03-19 Thread Mark Brown
On Thu, Mar 19, 2009 at 01:07:34AM -0500, Lopez Cruz, Misael wrote:
> Headset was declared previously as a Headphone widget connecting
> HSMIC and HSOL/HSOR pins of TWL4030 codec in SDP430 machine driver.
> The capture path becomes invalid as the Headphone widget is not a
> valid input endpoint.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 2.6.29-rc8 regulator-next] regulator: init fixes (v4)

2009-03-19 Thread Mark Brown
On Wed, Mar 18, 2009 at 02:14:14PM -0700, David Brownell wrote:
> On Wednesday 18 March 2009, Mark Brown wrote:

> > I think it's more that I'm viewing the use count as being useful
> > information which the API can take advantage of ("do any consumers
> > actually want this on right now?").

> In that case, I don't understand why you didn't like
> the previous versions of this patch ... which just
> forced the regulator off (at init time) if that count
> was zero.

There are two issues which I raised in response to that patch.  One is
that this is a fairly substantial interface change which will affect
existing constraints without warning and will therefore break people
using the current code.  At the minute you can't rely on boot_on being
set for any regulator which is enabled when Linux starts.  This is the
most serious issue - a quick survey leads me to expect that turning off
regulators without boot_on would break most existing systems somehow.

The other is that I'm fairly sure that we'll end up running into systems
where the setup at startup is not fixed and forcing the regulator state
immediately on regulator registration is likely to cause problems
dealing gracefully with such systems.  You addressed that by adding
devmode, though ideally that should have a clearer name.

> > I think we should be able to handle 
> > this without changing the use count and that this is preferable because
> > otherwise we make more work with shared consumers, which should be the
> > simplest case.

> That's what the earlier versions of this patch did,
> but you rejected that approach ... patches against
> both mainline (which is where the bug hurts TODAY),
> and against regulator-next.

I have given you two suggestions which will allow your MMC driver to use
mainline without impacting other drivers.  I've also provided some
suggestions for how we could introduce changes in the regulator core to
support this better.

> Also, I don't see why you'd think a shared consumer
> would be the "simplest", given all the special rules
> that you've already noted as only needing to kick in
> for those cases.

Simplest for the consumers - they just need to do a get followed by
basic reference counting, they don't need to (and can't, really) worry
about anything else.

> > The trick is getting the non-shared regulators into sync with the
> > internal status,

> I don't see why that should need any tricks.  At
> init time you have that state and the regulator;
> and you know there are no consumers.  Put it into

Realistically we don't have that information at the minute.  For the
most part we have the physical state and we may also have some
constraint information but we can't rely on the constraint information
right now.  The fact that we can't rely on the constraint information
means that we can't tell if a regulator is enabled because something is
using it at the minute or if it's just been left on because that was the
default or similar.

> a self-consistent state at that time ... done.

> There are really only two ways to make that state
> self-consistent.  And you have rejected both.

Both of the approaches you have suggested change the interfaces and
cause problems for existing users with no warning to allow them to
transition.  Changing the reference count does avoid the problems with
powering things off but it causes other problems in doing so, ones that
look harder to resolve.

When looking at bringing the use count in sync with the physical state
during startup we have to deal with the fact that we can't currently
rely on having information about the desired state of the hardware at
the time the regulator is registered.  We need to make an API change of
some kind to get that information.

> > during init. ?I think either using regulator_force_disable() or saying

> The force_disable() thing looks to me like an API design
> botch.  As you said at one point, it has no users.  And
> since the entire point is to bypass the entire usecount
> scheme ... it'd be much better to remove it.

As the documentation says it was originally added for error handling
cases where there is a danger of hardware damage.  In that situation
you really do want to power off immediately without reference to any
other state.

> > that the consmer wants exclusive access on get and then bumping the use
> > count for it if the regulator is already enabled ought to cover it.
> > I've not thought the latter option through fully, though.

> The problem I have with your approach here is that you
> have rejected quite a few alternative bugfixes applying
> to a common (and simple!!) configuration, but you haven't
> provided an alternative that can work for MMC i

Re: [patch 2.6.29-rc8 regulator-next] regulator: init fixes (v4)

2009-03-19 Thread Mark Brown
On Wed, Mar 18, 2009 at 02:02:14PM -0700, David Brownell wrote:

> Huh?  $SUBJECT patch hasn't merged.  How could you
> have backed it out??

Sorry - another patch introducing a version of the same issue.  Your
patch:

http://git.kernel.org/?p=linux/kernel/git/lrg/voltage-2.6.git;a=commit;h=67130ef073299cc7299d3ffa344122959e97753c

bumped the refcount for boot_on regulators.  This patch:

http://git.kernel.org/?p=linux/kernel/git/lrg/voltage-2.6.git;a=commit;h=504e7d7e42fb3a58809946770ff5e07cc9d52dbf

backs out that change only.  This was my bad on the review.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [APPLIED] Overo broken after recent mainline merge

2009-03-23 Thread Mark Brown
On Fri, Mar 20, 2009 at 03:33:24PM -0700, David Brownell wrote:

> + dev_warn(dev, "APPLY REGULATOR HACK for 
> vmmc\n");

...

> + dev_warn(dev, "APPLY REGULATOR HACK "
> + "for vmmc_aux\n");

Please remove these hunks, it's not useful to the users to log this.  No
action is required on their part and this is expected behaviour.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to test regulator driver?

2009-03-24 Thread Mark Brown
On Tue, Mar 24, 2009 at 05:14:58PM +0530, Aggarwal, Anuj wrote:

> I am seriously confused on how to fetch the struct device * in my
> consumer driver, for an already registered regulator. I am planning to
> use this consumer driver as a part of my CPU freq/CPU idle framework
> but I don't know what to pass in regulator_get.

Your consumer driver should not be using the struct device for the
regulator when calling regulator_get().  They should never need to know
anything about the struct device of the regulator that is providing a
supply to them.  The consumer should use a fixed device name and the
struct device for itself, these will then be resolved to an actual
regulator by the core based on the machine constraints.

In the specific case of cpufreq where there is no struct device for the
consumer use NULL when calling regulator_get().
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

2009-03-30 Thread Mark Brown
The approach that's being taken by the mmc-twl4030 driver to disabling
regulators is a normal and supported one so there is no need to print
messages on the console warning about this - their system is functioning
normally.

Signed-off-by: Mark Brown 
---
 arch/arm/mach-omap2/mmc-twl4030.c |3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/mmc-twl4030.c 
b/arch/arm/mach-omap2/mmc-twl4030.c
index 8db1f29..b76a31a 100644
--- a/arch/arm/mach-omap2/mmc-twl4030.c
+++ b/arch/arm/mach-omap2/mmc-twl4030.c
@@ -136,14 +136,11 @@ static int twl_mmc_late_init(struct device *dev)
 * (which is safe for MMC, but not in general).
 */
if (regulator_is_enabled(hsmmc[i].vcc) > 0) {
-   dev_warn(dev, "APPLY REGULATOR HACK for 
vmmc\n");
regulator_enable(hsmmc[i].vcc);
regulator_disable(hsmmc[i].vcc);
}
if (hsmmc[i].vcc_aux) {
if (regulator_is_enabled(reg) > 0) {
-   dev_warn(dev, "APPLY REGULATOR HACK "
-   "for vmmc_aux\n");
regulator_enable(reg);
regulator_disable(reg);
}
-- 
1.6.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

2009-03-31 Thread Mark Brown
On Mon, Mar 30, 2009 at 01:53:43PM -0700, David Brownell wrote:

> So when are you going to fix the regulator docs to report that:

>   ALL regulator consumers must start by enabling and
>   then disabling the regulator.

The documention should not be changed to say that since only consumers
that need the regulator to be off at startup should do this, and then
probably only if they find that it is already enabled.

Other consumers do not need to do this.  Consumers that want to enable a
regulator at startup can do so directly.  Consumers that don't need a
specific state (for example, because they are able to share the
regulator and don't need it enabling) should just leave it alone.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems while designing TPS65023 regulator driver

2009-04-03 Thread Mark Brown
On Fri, Apr 03, 2009 at 01:33:58PM +0530, Aggarwal, Anuj wrote:

> I could not find the commit in linux-OMAP git where the init data is 
> passed as a parameter to the regulator_register(). I am dependent 
> on this commit for my TPS65023 regulator driver and could not push 
> my patch without the commit being in l-o.

> Any idea when this would be available in l-o?

I can't speak for the OMAP tree but the patch should appear in
2.6.30-rc1 so presumably it'll get merged into OMAP some time after that
is released.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

2009-04-03 Thread Mark Brown
On Fri, Apr 03, 2009 at 05:26:52PM -0700, Tony Lindgren wrote:

> Maybe you guys have gone back and forth on this option too.. But what
> if we have something in regulator_init_data that would tell the
> regulator to reset the regulator on init? That could be then be
> then disabled with some cmdline option if needed for debugging or
> while booting from other operating systems etc.

Yup, that works perfectly well - in fact, Jonathan Cameron has a patch
adding a new constraint doing exactly that which managed ought to have
been merged but ended up falling through the cracks for 2.6.30.  I'm
expecting him to do the required refresh early on in this cycle.  Since
it's an explicit constraint we can just go ahead and apply it without
needing an ability to override it.

There's also support for powering off any unused regulators in a
late_initcall() which did get merged but that doesn't help with things
like MMC since they'll be probing before late_initcall().
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP: Don't warn user about expected behaviour in mmc-twl4030

2009-04-06 Thread Mark Brown
On Fri, Apr 03, 2009 at 06:03:33PM -0700, David Brownell wrote:
> On Tuesday 31 March 2009, Mark Brown wrote:

> > The documention should not be changed to say that since only consumers
> > that need the regulator to be off at startup should do this, and then
> > probably only if they find that it is already enabled.

> "Probably" shows you still don't have a good answer...

No, it means that I can't think of any reason why they'd want to bounce
the supply but I'm open to the possibility that there could be one.

> I see that mainline now has ca7255614e0861e36480103f4a402a115803d7b5
> which is a variant of the first late-fixup patch I sent.  The isssue
> with that approach is that it leaves a huge window between regulator
> init and its late fixup ... during which driver probe() calls suffer
> the bad effects of the current self-inconsistent initialization.  So
> it doesn't really address the MMC cases.

That change was not intended to do anything for MMC, it's there for
other users like shared regulators and situations where no consumer
driver instantiates.

It is really important that you engage with the feedback you are
getting.  At this point we are all quite familiar with your views and we
do understand them, restating them is not what's needed.  Changes have
to be made with an understanding of the existing interface and how it
supports the use cases the API has, including users other than MMC.
Changes also need to be made with consideration given to merge issues.

> And in fact, that's exactly what I think folk should be doing with
> the recent additions to twl4030-power supporting explicit init of
> all the power resources ... as done with e.g. Beagle.  The benefit

...

> So there's an OMAP-specific workaround now in place, albeit not
> yet headed towards mainline.

With Jonathan's patch there will be support in the regulator API for
doing this with all regulators.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.29 and SDP3430

2009-04-09 Thread Mark Brown
On Thu, Apr 09, 2009 at 01:32:27PM -0500, Menon, Nishanth wrote:

> implementor 41 architecture 3 part 30 variant c rev 1
> regulator_init_complete: incomplete constraints, leaving VAUX3 on

That last message shouldn't be a problem - it means that if the machine
init code had requested it the regulator core would've powered off the
VAUX3 regulator since it has no users.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] trouble with alsa, wolfson, and TI OMAP35xx McBSP

2009-04-22 Thread Mark Brown
On Wed, Apr 22, 2009 at 01:58:01PM -0400, twebb wrote:

> Problem 1:
> The WM8978 sample rate PLL does not seem to be stable while DCVDD =
> DBVDD = 1.8V.  FCLK mean = 44.5KHz and jittering. Increasing DCVDD and
> DBVDD voltage above 2V increases PLL stability. We have a workaround
> for this for now.

Can I suggest that you contact Wolfson's applications support team
regarding this issue - there's a web form at:

http://www.wolfsonmicro.com/support/technical/

Looking at the datasheet the PLL is specified as requiring a minimum of
1.9V - see page 6 of:

http://www.wolfsonmicro.com/uploads/documents/en/WM8978_Rev4.3.pdf

> Problem 2:
> Test tone is being presented by the user application, providing a 1Khz
> tone sampled at 44.1Khz. The data are S16_LE, right channel only. Left
> channel is quiet. The data seems to slip back and forth from left to
> right channel. This is reproducable and verified with a scope trace.

> Anybody have any ideas what might be going wrong here?  Traces for
> codec reg dump and mcbsp are attached.

There have been some recent threads on this list regarding the
configuration of the McBSP port configuration for various formats - it's
probably worth checking the latest changes to the OMAP code in the
topic/asoc branch of:

git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git

That said, I2S with the CODEC as clock master is one of the most common
configurations for OMAP so I'd expect it to be well tested.  Have you
tried testing recording?

Also CCing in Peter and Jarkko, our resident OMAP audio experts.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/1] Activate VDD1, VDD2 and VPLL1 at startup

2009-04-23 Thread Mark Brown
On Wed, Apr 22, 2009 at 06:35:29PM +0300, Peter 'p2' De Schrijver wrote:

> This patch activates VDD1, VDD2 and VPLL1 when booting. This is necessary
> because these resources are in warm reset state after a reboot. This means
> their voltage levels cannot be modified so DVFS and smartreflex don't work.

> Signed-off-by: Peter 'p2' De Schrijver 

Since TWL4030 is in mainline now you really ought to submit this and
the other patches you posted immediately before this one to mainline.
It'd be best to CC David Brownell (added here) on TWL4030 regulator
changes too, he's the expert on the regulator features of the chip.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] trouble with alsa, wolfson, and TI OMAP35xx McBSP

2009-04-23 Thread Mark Brown
On Thu, Apr 23, 2009 at 11:50:25AM -0500, Alejandro Blanca G. wrote:

> Probably the issue we are seeing is related: we are having problems
> when doing playback and observe that the information corresponding to
> 2 channels is being sent only through the left channel. We might have
> some other problems too, since we are new to this kind of development,
> but maybe our observations or yours could be useful to stabilize the
> Beagle platform code.

Please check out the current ASoC branch - as previously mentioned there
has been quite a bit of work recently on the McBSP audio formatting:

git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] regulator core: fix double-free in regulator_register() error path

2009-04-26 Thread Mark Brown
On Sat, Apr 25, 2009 at 05:28:36AM -0600, Paul Walmsley wrote:

> During regulator registration, any error after device_register() will 
> cause a double-free on the struct regulator_dev 'rdev'.  The bug is in 
> drivers/regulator/core.c:regulator_register():

Looks good:

Acked-by: Mark Brown 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] regulator core: fix double-free in regulator_register() error path

2009-04-28 Thread Mark Brown
On Mon, Apr 27, 2009 at 08:08:50PM -0700, David Brownell wrote:
> On Saturday 25 April 2009, Paul Walmsley wrote:

> > For the 3430SDP users out there, this patch also fixes the boot hang after 
> > "regulator_init_complete: incomplete constraints, leaving VAUX3 on"
> > on that device.

> For the record, that "incomplete constraints" message is bogus.
> On that board, VAUX3 has a complete set of constraints:  it may
> only emit 2.8V.

It's not VAUX3 that it's saying has incomplete constraints, it's the
board as a whole - if the constraints for the board were fully specified
(and the core had been told about it) then it would power off VAUX3 at
that point.

> Mark and/or Liam ... you might want to fix that diagnostic, to
> avoid leading more developers astray!

Probably shove a "board has" in there or something I guess.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] regulator core: fix double-free in regulator_register() error path

2009-04-28 Thread Mark Brown
On Tue, Apr 28, 2009 at 02:32:56AM -0700, David Brownell wrote:
> On Tuesday 28 April 2009, Mark Brown wrote:

> > It's not VAUX3 that it's saying has incomplete constraints, it's the
> > board as a whole - if the constraints for the board were fully specified

> No; driver support != constraint.  Only one of the
> issues is packaged as a "constraint".

Driver support isn't particularly relevant here.  If there are devices
that don't have drivers or don't have drivers with regulator support but
need a regulator enabling then the board should mark the regulator as
always_on when setting full constraints; the always_on flag can be
removed when the required consumers are added.

> > > Mark and/or Liam ... you might want to fix that diagnostic, to
> > > avoid leading more developers astray!

> > Probably shove a "board has" in there or something I guess.

> How about:  "VAUX3 board support is incomplete".
> That's accurate.

No.  The constraints being complete is a property of the board as a
whole and not the particular regulator.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] regulator core: fix double-free in regulator_register() error path

2009-04-28 Thread Mark Brown
On Tue, Apr 28, 2009 at 04:47:52AM -0700, David Brownell wrote:
> On Tuesday 28 April 2009, Mark Brown wrote:
> > On Tue, Apr 28, 2009 at 02:32:56AM -0700, David Brownell wrote:

> > > > > What it lacks is something entirely different: ?driver support
> > > > > for the LCD which uses the regulator framework,

> > > > It's not VAUX3 that it's saying has incomplete constraints, it's the
> > > > board as a whole - if the constraints for the board were fully specified

> > > No; driver support != constraint.  Only one of the
> > > issues is packaged as a "constraint".

> > Driver support isn't particularly relevant here.

> It's the *entire* point.  The driver is talking directly
> to the regulator, bypassing this framework.  The constraints
> on that regulator are fully defined ... and then bypassed.

Ah, I see - I didn't realise that there was driver support that doesn't
use the regulator API, that had been dropped out of context by that
point in the discussion.  I guess the LCD driver will be converted to
use the API in due course, hopefully there aren't too many other such
drivers.

This is the sort of use case that made me not want to have the API
disable unused regulators by default - there's stuff going on that the
regulator API and the code using it doesn't know about (since it's going
through some other interface to do the job in this case) so the API
can't safely do anything to that regulator.

> > > How about:  "VAUX3 board support is incomplete".
> > > That's accurate.

> > No.  The constraints being complete is a property of the board as a
> > whole and not the particular regulator.

> Except ... that "constraint" isn't the issue, it's
> unexpected driver behavior.  And "board" is exactly

This is sufficiently unexpected that it's the first time I've seen
anything bypassing the regulator API; in any case, it's just a question
of where you see the problem coming from.

My terminology comes from the fact that as far as the core is concerned
the issue is that the board didn't say it had given the core complete
constraints allowing it to fully control the regulators that are visible
to it.  The reason the board didn't do that is that is that the LCD
driver is bypassing the regulator API and the regulator API doesn't have
any way to express that possibility.

> what I said, so I don't know why you're arguing.
> (For the "fun" of it?)

David, that is really not necessary or constructive.  This sort of
disparaging remark has been a constant feature of your interactions on
regulator API issues and it is not achieving anything useful.

> Board support includes full driver support, as well
> as board setup (constraints).  That's the common way
> to factor it, at any rate -- a "board support package"
> addresses both, and they need to work together.

s/board/machine driver/; in an ideal world for Linux there is no BSP,
it's all mainline.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Overo broken with current top of tree

2009-04-30 Thread Mark Brown
On Thu, Apr 30, 2009 at 12:31:47PM +0300, Grazvydas Ignotas wrote:
> On Wed, Apr 29, 2009 at 7:03 PM, Steve Sakoman  wrote:

> > set_machine_constraints: invalid 'VUSB1V5' voltage constraints

> I get the same on pandora, although it continues booting fine after
> that. Maybe regulator folks will comment about regulator errors.

I suspect this may be due to the buggy defaults that are provided when
no voltage constraints are given for a fixed voltage regulator.  There's
a patch on its way to mainline fixing this:

commit 14d32bb077f7cc6f78bd012e5b1489899dddf749
Author: Mark Brown 
Date:   Tue Apr 28 11:09:38 2009 +0100

regulator: Fix default constraints for fixed voltage regulators

Default voltage constraints were being provided for fixed voltage
regulator where board constraints were not provided but these constraints
used INT_MIN as the default minimum voltage which is not a valid value
since it is less than zero. Use 1uV instead.

Also set the default values we set in the constraints themselves since
otherwise the max_uV constraint we determine will not be stored in the
actual constraint strucutre and will therefore not be used.

Signed-off-by: Mark Brown 
Signed-off-by: Liam Girdwood 

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2f14c16..98c3a74 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -703,10 +703,13 @@ static int set_machine_constraints(struct regulator_dev 
*rdev,
int cmin = constraints->min_uV;
int cmax = constraints->max_uV;
 
-   /* it's safe to autoconfigure fixed-voltage supplies */
+   /* it's safe to autoconfigure fixed-voltage supplies
+  and the constraints are used by list_voltage. */
if (count == 1 && !cmin) {
-   cmin = INT_MIN;
+   cmin = 1;
cmax = INT_MAX;
+   constraints->min_uV = cmin;
+   constraints->max_uV = cmax;
}
 
/* voltage constraints are optional */
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] Beep sound in the end of audio file

2009-05-06 Thread Mark Brown
On Wed, May 06, 2009 at 07:30:03PM +0530, Aggarwal, Anuj wrote:

> > After a quick look I can not pin point the soc board file used with the
> > omap3evm board. Is it in the tree?

> [Aggarwal, Anuj] No, it is not. Reason being we already have beagle & 
> sdp3430 there, which are both similar to omap3evm, which I pushed. 
> So mine got rejected. We had some discussions on how to handle this
> scenario but nothing got finalized.

Not from the ALSA side it didn't...
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] Beep sound in the end of audio file

2009-05-07 Thread Mark Brown
On Thu, May 07, 2009 at 03:05:08PM +0530, Arun KS wrote:

> Since we are not going with the generic driver, are you going to push
> this patch:

> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg07166.html

I can't see anything wrong with it looking in the web archive but somene
should send a copy tested against current ASoC.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/1] ASoC: Added OMAP3 EVM support in ASoC.

2009-05-08 Thread Mark Brown
On Thu, May 07, 2009 at 09:38:47PM +0530, Anuj Aggarwal wrote:

> + if (!machine_is_omap3evm()) {
> + pr_debug("Not OMAP3 EVM!\n");
> + return -ENODEV;
> + }

Since you've said you'll be resubmitting perhaps this should be an error
rather than debug message?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] Support multiple PMICs in Voltage Regulator Framework

2009-05-08 Thread Mark Brown
On Fri, May 08, 2009 at 08:41:13PM +0530, Anuj Aggarwal wrote:
> Based on the discussion we had at:
> 
> http://marc.info/?l=linux-omap&m=124083364321017&w=2
> 
> I am sending the following patches which allows one to move the 
> board-dependent
> regulator-specific code to a newly created file drivers\regulator\pmic.c. This
> file will have the board specific information for different regulators and
> it will do the regulator initialization depending on one which is available.

You really need to submit changes like this to the relevant subsystem
maintainers and mailing lists rather than linux-omap - changes to kernel
subsystems will affect all users of the kernel, not just OMAP.

In this case your proposal should have been sent to myself, Liam
Girdwood (CCed) and linux-kernel.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Regulator: Add pmic.c file to regulator framework

2009-05-08 Thread Mark Brown
On Fri, May 08, 2009 at 08:42:06PM +0530, Anuj Aggarwal wrote:

> Added drivers\regulator\pmic.c file to the regulator framework which will

I suspect you mean / there :)

> have the board specific information for different regulators and will do the
> regulator initialization depending on one which is available.

> Signed-off-by: Anuj Aggarwal 

I'm really not sure what the abstraction that you're attempting to
introduce here is intended to do that can't handled by the existing
kernel features - could you go into more detail about the problem that
this patch is intended to solve, please?

The kernel already provides facilities for detecting the presence of
devices.  In cases where it is possible to do a generic check for a
device (for example, by reading back ID registers on the device and
checking the values) then the device driver can do so in its probe
routine and the board can unconditionally declare that the device is
there, allowing the probe to fail.  In cases where something board
specific needs to be done (such as checking for fit resistors or
information provided by the bootloader) then that'd need to be handled
in the board code anyway - it's not going to apply on generic systems.
There will also be cases where it's just not possible to determine
what's fitted at runtime, especially for simple devices with GPIO only
control.

If there is initialisation which can always be done for a chip no matter
what board it's on then I'd expect the driver to just handle that when
it is starting up without needing separate code - if there's something
preventing that then we should definitely fix that, probably in a way
that's not specific to the regulator framework since I'd expect other
subsystems to be running into similar issues.

Another thing here is that the code appears to assume that there will be
only one regulator device in the system which isn't always true.  Some
designs will use a series of discrete regulators rather than integrated
chips while others will use several integrated PMICs for reasons such as
ease of layout or power distribution.

Based on experience with the OMAP audio drivers I suspect the problem
the patch is trying to solve may be that there are a lot of designs out
there which are based on the reference platforms which TI have produced
and are therefore have very repetitive board code.  Perhaps this might
be best handled by having some utility routines in the OMAP code which
the machine drivers can use to say that they have the same PMIC setup as
a given reference design?

CCing in Liam so I've not deleted any of the text.

> ---
>  drivers/regulator/Makefile |2 +-
>  drivers/regulator/pmic.c   |  103 
> 
>  include/linux/regulator/pmic.h |   29 +++
>  3 files changed, 133 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/regulator/pmic.c
>  create mode 100644 include/linux/regulator/pmic.h
> 
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index bac133a..c0d87bf 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -3,7 +3,7 @@
>  #
> 
> 
> -obj-$(CONFIG_REGULATOR) += core.o
> +obj-$(CONFIG_REGULATOR) += core.o pmic.o
>  obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
>  obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
> 
> diff --git a/drivers/regulator/pmic.c b/drivers/regulator/pmic.c
> new file mode 100644
> index 000..36ed341
> --- /dev/null
> +++ b/drivers/regulator/pmic.c
> @@ -0,0 +1,103 @@
> +/*
> + * pmic.c
> + *
> + * Supports run-time detection of different Power Management ICs.
> + *
> + * Copyright (C) 2009 Texas Instrument Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or modify it 
> under
> + * the terms of the GNU General Public License as  published by the Free
> + * Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any kind,
> + * whether express or implied; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Definitions specific to TWL4030
> + */
> +
> +/*
> + * Definitions specific to TPS62350
> + */
> +
> +/*
> + * Definitions specific to TPS65023
> + */
> +
> +static int flag_pmic_twl4030;
> +static int flag_pmic_tps6235x;
> +static int flag_pmic_tps65023;
> +
> +/*
> + * Detect the current PMIC, set one of the flags
> + */
> +static inline int detect_pmic(void)
> +{
> + /* How? Any suggestions?? This is a temporary solution. */
> +#if defined(CONFIG_TWL4030_CORE)
> + flag_pmic_twl4030 = 1;
> +#endif
> +
> +#if defined(CONFIG_OMAP3EVM_TPS6235X)
> + flag_pmic_tps6235x = 1;
> +#endif
> +
> +#if defined(CONFIG_OMAP3EVM_TPS65023)
> + flag_pmic_tps65023 = 1;
> +#endif
> +
> + return 0;
> +}
> +
> +/* Functions to detect which PMIC is pre

Re: [PATCH 2/3] Regulator: Add TPS65023 Regulator Driver

2009-05-08 Thread Mark Brown
On Fri, May 08, 2009 at 08:42:12PM +0530, Anuj Aggarwal wrote:
> Added regulator driver for TPS65023 and modified the Kconfig and Makefile for
> the same.
> 
> Signed-off-by: Anuj Aggarwal 

This looks pretty good.  I've spotted a few issues, mostly to do with
error handling or nitpicky stuff that it's nice to have but which
shouldn't be a blocker to merge.  It would be good to get the initcall
ordering addressed, though.

CCing in Liam and not deleting any text as a result.

> ---
>  drivers/regulator/Kconfig  |8 +
>  drivers/regulator/Makefile |1 +
>  drivers/regulator/tps65023-regulator.c |  510 
> 
>  3 files changed, 519 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/regulator/tps65023-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index e58c0ce..28109e1 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -91,4 +91,12 @@ config REGULATOR_PCF50633
>Say Y here to support the voltage regulators and convertors
>on PCF50633
> 
> +config REGULATOR_TPS65023
> + tristate "TI TPS65023 Power regulators"
> + depends on OMAP3EVM_TPS65023
> + help
> +   This driver supports TPS65023 voltage regulator chips. TPS65023 
> provides
> +   three step-down converters and two general-purpose LDO voltage 
> regulators.
> +   It supports TI's software based Class-2 SmartReflex implementation.
> +
>  endif
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index c0d87bf..28235b9 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -13,5 +13,6 @@ obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>  obj-$(CONFIG_REGULATOR_DA903X)   += da903x.o
>  obj-$(CONFIG_REGULATOR_PCF50633) += pcf50633-regulator.o
> +obj-$(CONFIG_REGULATOR_TPS65023) += tps65023-regulator.o
> 
>  ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
> diff --git a/drivers/regulator/tps65023-regulator.c 
> b/drivers/regulator/tps65023-regulator.c
> new file mode 100644
> index 000..657c0c3
> --- /dev/null
> +++ b/drivers/regulator/tps65023-regulator.c
> @@ -0,0 +1,510 @@
> +/*
> + * tps65023-regulator.c -- Supports TPS65023 regulator
> + *
> + * Author : Anuj Aggarwal
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Register definitions */
> +#define  TPS65023_REG_VERSION0
> +#define  TPS65023_REG_PGOODZ 1
> +#define  TPS65023_REG_MASK   2
> +#define  TPS65023_REG_REG_CTRL   3
> +#define  TPS65023_REG_CON_CTRL   4
> +#define  TPS65023_REG_CON_CTRL2  5
> +#define  TPS65023_REG_DEF_CORE   6
> +#define  TPS65023_REG_DEFSLEW7
> +#define  TPS65023_REG_LDO_CTRL   8
> +
> +/* PGOODZ bitfields */
> +#define  TPS65023_PGOODZ_PWRFAILZBIT(7)
> +#define  TPS65023_PGOODZ_LOWBATTZBIT(6)
> +#define  TPS65023_PGOODZ_VDCDC1  BIT(5)
> +#define  TPS65023_PGOODZ_VDCDC2  BIT(4)
> +#define  TPS65023_PGOODZ_VDCDC3  BIT(3)
> +#define  TPS65023_PGOODZ_LDO2BIT(2)
> +#define  TPS65023_PGOODZ_LDO1BIT(1)
> +
> +/* MASK bitfields */
> +#define  TPS65023_MASK_PWRFAILZ  BIT(7)
> +#define  TPS65023_MASK_LOWBATTZ  BIT(6)
> +#define  TPS65023_MASK_VDCDC1BIT(5)
> +#define  TPS65023_MASK_VDCDC2BIT(4)
> +#define  TPS65023_MASK_VDCDC3BIT(3)
> +#define  TPS65023_MASK_LDO2  BIT(2)
> +#define  TPS65023_MASK_LDO1  BIT(1)
> +
> +/* REG_CTRL bitfields */
> +#define TPS65023_REG_CTRL_VDCDC1_EN  BIT(5)
> +#define TPS65023_REG_CTRL_VDCDC2_EN  BIT(4)
> +#define TPS65023_REG_CTRL_VDCDC3_EN  BIT(3)
> +#define TPS65023_REG_CTRL_LDO2_ENBIT(2)
> +#define TPS65023_REG_CTRL_LDO1_ENBIT(1)
> +
> +/* LDO_CTRL bitfields */
> +#define TPS65023_LDO_CTRL_LDOx_SHIFT 4
> +#define TPS65023_LDO_CTRL_LDOx_MASK(ldo_id)  (0xF0 >> (ldo_id*4))
> +
> +/* Number of step-down converters available */
> +#define TPS65023_NUM_DCDC3
> +/* Number of LDO voltage regulators  available */
> +#define TPS65023_NUM_LDO 2
> +/* Number of total regulators available */
> +#define TPS65023_NUM_REGULATOR   (TPS65023_NUM_DCDC + TPS65023_NUM_LDO)
> +
> +struct tps_info {
> + const char  *name;
> + unsignedmin_uV;
> + unsignedmax_uV;
> + boolfixed;
> + u8  ta

Re: [PATCH 3/3] Regulator: Added board-dependent code for TPS65023

2009-05-08 Thread Mark Brown
On Fri, May 08, 2009 at 08:42:16PM +0530, Anuj Aggarwal wrote:
> Added OMAP3 EVM specific code for TPS65023 in pmic.c file.
> 
> Signed-off-by: Anuj Aggarwal 

CCing in Liam again.

> ---
>  drivers/regulator/pmic.c |   92 
> ++
>  1 files changed, 92 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/regulator/pmic.c b/drivers/regulator/pmic.c
> index 36ed341..6e7276a 100644
> --- a/drivers/regulator/pmic.c
> +++ b/drivers/regulator/pmic.c
> @@ -29,6 +29,96 @@
>  /*
>   * Definitions specific to TPS65023
>   */
> +#if defined(CONFIG_OMAP3EVM_TPS65023)
> +/* MPU voltage regulator of DCDC type */
> +struct regulator_consumer_supply tps65023_mpu_consumers = {
> + .supply = "vdd1",
> +};

This comes back to my questions about pmic.c but I'd expect this all to
appear in the OMAP3 EVM code.

> +
> +/* CORE voltage regulator of DCDC type */
> +struct regulator_consumer_supply tps65023_core_consumers = {
> + .supply = "vdd2",
> +};
> +
> +/* SRAM/MEM/WKUP_BG voltage regulator of DCDC type */
> +struct regulator_consumer_supply tps65023_vdds_consumers = {
> + .supply = "vdds",
> +};
> +
> +/* DPLL voltage regulator of LDO type */
> +struct regulator_consumer_supply tps65023_dpll_consumers = {
> + .supply = "dpll",
> +};
> +
> +/* MMC voltage regulator of LDO type */
> +struct regulator_consumer_supply tps65023_mmc_consumers = {
> + .supply = "mmc",
> +};
> +
> +struct regulator_init_data tps65023_regulator_data[] = {
> + {
> + .constraints = {
> + .min_uV = 80,
> + .max_uV = 160,
> + .valid_ops_mask = (REGULATOR_CHANGE_VOLTAGE |
> + REGULATOR_CHANGE_STATUS),
> + .boot_on = 1,
> + },
> + .num_consumer_supplies  = 1,
> + .consumer_supplies  = &tps65023_mpu_consumers,
> + },
> + {
> + .constraints = {
> + .min_uV = 180,
> + .max_uV = 330,
> + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> + .boot_on = 1,
> + },
> + .num_consumer_supplies  = 1,
> + .consumer_supplies  = &tps65023_core_consumers,
> + },
> + {
> + .constraints = {
> + .min_uV = 180,
> + .max_uV = 330,
> + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> + .boot_on = 1,
> + },
> + .num_consumer_supplies  = 1,
> + .consumer_supplies  = &tps65023_vdds_consumers,
> + },
> + {
> + .constraints = {
> + .min_uV = 100,
> + .max_uV = 315,
> + .valid_ops_mask = (REGULATOR_CHANGE_VOLTAGE |
> + REGULATOR_CHANGE_STATUS),
> + .boot_on = 1,
> + },
> + .num_consumer_supplies  = 1,
> + .consumer_supplies  = &tps65023_dpll_consumers,
> + },
> + {
> + .constraints = {
> + .min_uV = 105,
> + .max_uV = 330,
> + .valid_ops_mask = (REGULATOR_CHANGE_VOLTAGE |
> + REGULATOR_CHANGE_STATUS),
> + .boot_on = 1,
> + },
> + .num_consumer_supplies  = 1,
> + .consumer_supplies  = &tps65023_mmc_consumers,
> + },
> +};
> +
> +static struct i2c_board_info __initdata board_tps65023_instances[] = {
> + {
> + I2C_BOARD_INFO("tps65023", 0x48),
> + .flags = I2C_CLIENT_WAKE,
> + .platform_data = &tps65023_regulator_data[0],
> + },
> +};
> +#endif
> 
>  static int flag_pmic_twl4030;
>  static int flag_pmic_tps6235x;
> @@ -96,6 +186,8 @@ int pmic_init(void)
> 
>  #if defined(CONFIG_OMAP3EVM_TPS65023)
>   /* do stuff specific to TPS65023 */
> + omap_register_i2c_bus(1, 400, board_tps65023_instances,
> + ARRAY_SIZE(board_tps65023_instances));
>  #endif
> 
>   return 0;
> --
> 1.6.2.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] ASoC: Added OMAP3 EVM support in ASoC.

2009-05-14 Thread Mark Brown
On Thu, May 14, 2009 at 01:59:19PM +0530, Anuj Aggarwal wrote:
> Resending the patch after fixing the minor issues.
> 
> Signed-off-by: Anuj Aggarwal 

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ASoC: Added OMAP3 EVM support in ASoC.

2009-05-14 Thread Mark Brown
On Thu, 2009-05-14 at 08:39 -0700, Tony Lindgren wrote:
> Are there still some problems from ASoC point of view to
> convert ASoC to do the platform_device_alloc() from
> board init files?

This is still something that needs to be done for cards - the driver is
always called soc-audio. It needs an update to allow the platform device
for the codec to call snd_soc_register_card() directly to support this
but there's not much demand at the minute, I'm more concerned with
getting the CODEC drivers converted to use the standard device model to
register first.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] Regulator: Add pmic.c file to regulator framework

2009-05-18 Thread Mark Brown
On Mon, May 18, 2009 at 05:50:31PM +0530, Aggarwal, Anuj wrote:

> > The kernel already provides facilities for detecting the presence of
> > devices.  In cases where it is possible to do a generic check for a

> [Aggarwal, Anuj] In my opinion, it would not be a good idea to probe 
> for several PMICs, on all the buses present in the system. It can be 
> simplified by probing only the ones which are relevant for this device.

The point here is that the kernel already provides for doing device
enumeration of all kinds, both static and dynamic, and that adding this
file doesn't add anything except copying of the code from arch/arm into
drivers/regulator.

> > Another thing here is that the code appears to assume that there will be
> > only one regulator device in the system which isn't always true.  Some
> > designs will use a series of discrete regulators rather than integrated
> > chips while others will use several integrated PMICs for reasons such as
> > ease of layout or power distribution.

> [Aggarwal, Anuj] Can you explain this, I didn't get that much?

Instead of having a single PMIC with multiple regulator outputs a system
may choose to have several regulators.  One way this can happen is that
some manufacturers produce simple discrete regulator chips which produce
only a single output - typically several of these would be needed in a
system.

Another approach which cause this to come up some systems use is to have
more than one PMIC, for example having one PMIC for the main application
processor and a second PMIC providing supplies used by a coprocessor.
This can avoid electrical issues by allowing shorter tracks for the
regulated supplies and simplifying routing.

> [Aggarwal, Anuj] You are right, the primary purpose of this patch is to 
> avoid duplication of PMIC code, common to many OMAP platforms and I believe 
> this is a simpler and cleaner approach.
> Instead of keeping this file in the drivers/regulator folder, we can move it 
> to arch/arm/mach-omap2 folder.

This sounds like a good approach.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/6] ARM: OMAP3: Initialize more devices for LDP

2009-05-25 Thread Mark Brown
On Mon, May 25, 2009 at 01:59:20PM +0100, Russell King - ARM Linux wrote:
> On Fri, May 22, 2009 at 03:45:41PM -0700, Tony Lindgren wrote:

> > +   /* link regulators to MMC adapters */
> > +   ldp_vmmc1_supply.dev = mmc[0].dev;

> This feels wrong - setup after registering devices.  All it takes is to
> have to move various device drivers to be earlier in the initialization
> ordering and things start breaking in unexpected ways.

> If MMC controls the Vmmc1 supply, then why does the Vmmc1 supply need
> the struct device pointer for the MMC controller?

Like the clock API the regulator API looks up clients for regulators
based on a combination of the struct device for the client driver and
the name of the supply.  Unfortunately with both the I2C and SPI APIs
the struct device is only available once the driver has probed so this
can't be set up with static data prior to probe, hence this sort of
bodge.

Long term I think we'll want to fix I2C and SPI to let us have static
data for this.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/8] ARM: OMAP3: mmc-twl4030 uses regulator framework

2009-05-26 Thread Mark Brown
On Mon, May 25, 2009 at 07:47:30PM -0300, Daniel Ribeiro wrote:

> This hack would look less ugly on twl4030reg_probe(). There you can
> disable the regulator without first enabling it.

That might cause issues when running on a platform where someone has
used the MMC regulators for some purpose other than MMC and expects them
to stay on during boot.  It can be done via the machine constraints if
required, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: "Dummy" regulator possible?

2009-05-28 Thread Mark Brown
On Thu, May 28, 2009 at 11:45:15AM +0300, Tomi Valkeinen wrote:

> Is it possible to create a dummy regulator, so that the device driver
> could always ask for Vcc supply, and get it, even when it's connected to
> a always-on regulator?

This is exactly the use case that the fixed voltage regulator is
intended for - drivers/regulator/fixed.c.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] ARM: OMAP3: Initialize regulators for Beagle and Overo

2009-05-28 Thread Mark Brown
On Thu, May 28, 2009 at 10:33:20AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux  [090528 09:20]:

> > I really hate it when I see this kind of initialization after registration.
> > It feels totally wrong and fragile.

> > At one point, I had clkdev matching using struct device pointer as well,
> > but it was realised that was far too limiting - you couldn't declare
> > clock entries without first having all devices setup, and then you run
> > into problems with ordering.

> > It looks like the regulator stuff is suffering this same problem - it
> > wants to match by struct device pointer.  That's fine if all your
> > struct device's are statically allocated, but as soon as you start
> > having dynamic ones, it gets _much_ harder to cope with.

> Yeah. I believe Mark is working on sorting out the regulator fwk issues
> regarding this.

Eh, no, not really.  It's the same as the clock API in this regard - if
you want the struct device can be NULL and you can do a name based
lookup only but then you have to pass the name around in platform data
to support configurability.  If the clock API implements some other
solutions we'll probably follow them but I'm not aware of any.

Otherwise it's more likely that if I personally do anything on this I'd
work on getting I2C and similar subsystems to make the struct device
more readily available to board code.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] ARM: OMAP3: Initialize regulators for Beagle and Overo

2009-05-29 Thread Mark Brown
On Thu, May 28, 2009 at 08:48:34PM +0100, Russell King - ARM Linux wrote:
> On Thu, May 28, 2009 at 07:00:13PM +0100, Mark Brown wrote:

> > Eh, no, not really.  It's the same as the clock API in this regard - if
> > you want the struct device can be NULL and you can do a name based
> > lookup only but then you have to pass the name around in platform data
> > to support configurability.  If the clock API implements some other
> > solutions we'll probably follow them but I'm not aware of any.

> That's not quite what we're talking about.  With clkdev, you specify
> device name itself, rather than address-of-struct-device.  Using the
> device name gives you independence from the initialization or creation
> of the struct device (if indeed the struct device is created at all.)

Hrm, that does seem like a neat approach for avoiding the problems with
buses like I2C - I'll look into adding support for it, but at this point
it's not likely to be in time for the next merge window.  Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] Please help in adding ams-delta support to ASoC

2009-06-02 Thread Mark Brown
On Tue, Jun 02, 2009 at 09:24:39AM +0200, Janusz Krzysztofik wrote:

> I have choosen to register the codec device with 
> platform_device_register_simple("cx20442-codec", ...) invoked form asoc 
> machine driver, is this ok?

Normally the platform device would be registered by the machine file
under arch/arm but either works.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] alsa: add beagleboard SoC configuration.

2008-10-22 Thread Mark Brown
On Wed, Oct 22, 2008 at 01:30:26PM -0700, Steve Sakoman wrote:

> At the time, the feedback from the alsa-devel list was that they were
> OK with these patches living in linux-omap until the twl4030 driver
> was mainline ready.

Indeed.  The only blocker was that TWL4030 codec support breaks the
build of SND_SOC_ALL_CODECS without the core support for the chip - I
gave it my ack otherwise already.  Since the core support has now been
merged by Linus this shouldn't be an issue once ALSA has synchronised
with mainline.

> I am happy to resubmit the patches, if that is what folks want.  I'm
> copying alsa-devel so they can weigh in on the decision.

Once mainline is merged up to the ASoC topic branch it should be fine to
go via ALSA.  I've got some other stuff waiting for that to happen
myself so if you want to send a patch queue I can queue that in there.
I was going to wait for rc1 before asking Takashi to merge up to the
ASoC branches.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] alsa: add beagleboard SoC configuration.

2008-10-30 Thread Mark Brown
On Wed, Oct 22, 2008 at 09:44:48PM +0100, Mark Brown wrote:
> On Wed, Oct 22, 2008 at 01:30:26PM -0700, Steve Sakoman wrote:

> > I am happy to resubmit the patches, if that is what folks want.  I'm
> > copying alsa-devel so they can weigh in on the decision.

> Once mainline is merged up to the ASoC topic branch it should be fine to
> go via ALSA.  I've got some other stuff waiting for that to happen
> myself so if you want to send a patch queue I can queue that in there.
> I was going to wait for rc1 before asking Takashi to merge up to the
> ASoC branches.

Ping?  I'd like to start pushing more ASoC v2 stuff soon - that means
more API churn so it'd be easier to get as much stuff merged as
possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] Jack insertion

2008-11-18 Thread Mark Brown
On Tue, Nov 18, 2008 at 12:16:18PM +0530, naveen krishna ch wrote:

> Now my issue is is there any work regarding the Headset JACK
> insertion/detection.

> Earlier there was a proposal for a jack insertion layer.

> Can anyone suggest me in this regard.

The generic ALSA part of the API has been merged already - see
include/sound/jack.h.

For ASoC drivers you need to implement the actual detection in your
machine driver.  Normally this would just hook into a GPIO.  You can
then update the DAPM widget for the jack to mark it as in use or not in
use (some existing drivers do this) and also hook into the generic
userspace API (that's not been implemented by anyone yet).  Plan is to
get something more generic into 2.6.30.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Old OMAP mailing list

2008-11-18 Thread Mark Brown
Please don't CC any ASoC patches to the linux-omap-open-source list -
my understanding is that it should no longer be used, having been
replaced by [EMAIL PROTECTED]  Any posts CCed to it result in
backscatter like this:

On Tue, Nov 18, 2008 at 07:57:55AM -0600, [EMAIL PROTECTED] wrote:
> You are not allowed to post to this mailing list, and your message has
> been automatically rejected.  If you think that your messages are
> being rejected in error, contact the mailing list owner at
> [EMAIL PROTECTED]

which seems a bit hostile.  It'd be nice if this bounce mentioned the
new list, FWIW.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH 1/1] ASoC: Add support for omap2evm board

2008-11-19 Thread Mark Brown
On Wed, Nov 19, 2008 at 05:45:19PM +0530, Arun KS wrote:
> This patch adds twl4030 audio support on omap2evm
> 
> Signed-off-by: Arun KS <[EMAIL PROTECTED]>

Acked-by: Mark Brown <[EMAIL PROTECTED]>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] omap3evm asoc

2008-11-19 Thread Mark Brown
On Wed, Nov 19, 2008 at 05:57:55PM +0530, naveen krishna ch wrote:

> Is there any previous work on audio ASOC for OMAP3EVM.
> I have started working on OMAP3EVM + TWL4030 combination and audio is
> working.

Not that I'm aware of, though Arun just submitted a patch for the
omap2evm.  CCing in linux-omap who are more likely to know.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] ASoC: Add support for OMAP3 EVM

2008-11-25 Thread Mark Brown
On Tue, Nov 25, 2008 at 01:21:20PM +0200, Jarkko Nikula wrote:

> Or if those EVM's & SDP's can route TWL4030 audio connections more
> flexible than Beagle but somewhat similar manner, then probably have one
> single machine driver for all EVM's?

> Otherwise it doesn't make very much sense to have n similar machine
> drivers where only functions and variable names differ.

Yes, it looks like a lot of these drivers could be redone along the
lines of s3c24xx_uda134x.c with platform data specifying the differences
between the boards.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] ASoC: Fix TW4030 Kconfig dependency

2008-11-25 Thread Mark Brown
On Mon, Nov 24, 2008 at 08:44:13PM -0800, David Brownell wrote:
> On Monday 24 November 2008, [EMAIL PROTECTED] wrote:

> > Fixes Kconfig dependency of TWL4030 audio codec driver
> > with TWL4030 core driver on both overo and omap2evm
> > boards

> > Signed-off-by: Arun KS <[EMAIL PROTECTED]>

> Good idea.  :)

> Acked-by: David Brownell <[EMAIL PROTECTED]>

Applied.

Might it be an idea to have the relevant machine drivers select the
TWL4030 support?  This would prevent them disabling it but the chip is
such a basic part of these systems that that doesn't seem unreasonable.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] ASoC: Fix TW4030 Kconfig dependency

2008-11-25 Thread Mark Brown
On Tue, Nov 25, 2008 at 10:54:28AM -0800, David Brownell wrote:

> I'm not sure what you mean by "machine driver" ... there seem
> to be two:  arch/arm/mach-omap2/board-*.c for everything except
> ALSA, and then sound/soc/omap/*.c for the ASoC bits.

I meant the arch/arm one.

> As a rule, it's worth avoiding "select" in Kconfig; it doesn't
> work well, since it won't even report missing dependencies much
> less fix the trivial ones.

Right, I'm just thinking that for something like this it might not be
worth turning off the TWL4030 core support given that these sorts of
southbridgeish chips tend to be pretty key to the system.  It was just
an idle thought, anyway.

BTW, it'd also be really handy if you could remove the OMAP dependencies
from TWL4030_CORE - I know it currently needs them to actually be useful
at runtime but it'd mean that it's possible to build things like the
codec driver on other configs which helps when working on subsystems.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP general SOC driver.

2008-11-26 Thread Mark Brown
On Wed, Nov 26, 2008 at 06:47:58PM +0800, Stanley.Miao wrote:
> Add a shared omap SoC driver to avoid reduplicate code among omap soc drivers.

It's really good to see work on this but I've got some concerns with the
approach being taken here.

> +
> +config SND_OMAP_SOC_OVERO
> + tristate "SoC Audio support for Gumstix Overo"
> + depends on SND_OMAP_SOC && MACH_OVERO

This won't apply - this and several of the other machines are already
present.

> +#ifdef CONFIG_SND_OMAP_SOC_N810
> +#include "../codecs/tlv320aic3x.h"
> +#define CODEC_SYS_CLOCK  1200
> +#define CODEC_NAME   "TLV320AIC33"
> +#define STREAM_NAME  "AIC33"
> +#define CODEC_DEV(&soc_codec_dev_aic3x)
> +#define CODEC_DAI(&aic3x_dai)
> +#define CODEC_SETUP_DATA (&&n810_aic33_setup)
> +#define DAI_LINK_INIT(n810_aic33_init)
> +#define SOC_OPS_STARTUP  (n810_startup)
> +#define SOC_OPS_SHUTDOWN (n810_shutdown)

Please change this to use a platform data based approach rather than
these ifdefs: it's not a good approach for readability and it prevents
building multiple board drivers in one kernel which isn't good for
distributions.  The s3c24xx_uda134x.c provides an example of how this
can be done.  It may be more sensible to have a separate driver per
codec.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [PATCH] OMAP general SOC driver.

2008-11-26 Thread Mark Brown
On Wed, Nov 26, 2008 at 09:24:03AM -0800, Steve Sakoman wrote:

> I'm all for factoring out common code, but I think that this approach
> will become a maintenance nightmare over time.

I had formed the impression that an awful lot of the boards had very
similar audio subsystems and that even with DAPM support it'd be fairly
straightforward to do standard drivers for certain classes of system.
If that's not the case then doing this sort of shared machine driver
probably isn't worth it at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP general SOC driver.

2008-11-26 Thread Mark Brown
On Wed, Nov 26, 2008 at 10:34:41AM -0800, David Brownell wrote:

> Re TWL4030 codec configuration ... I count three microphone
> input channels not used on Beagle, plus a speaker output
> and some other stuff.  So it's obvious that something which
> works on Beagle won't handle more interesting configurations
> of that audio support.  

Can someone post an overview of what the hardware configurations of
these boards are, please?  It's starting to sound like they're not very
similar at all.

> I'm told that the ASoC stuff "should" go in a separate ASoC
> area for some reason.  That still makes no good sense to me,
> so if there's a brief explanation as to why it's done that
> way, please fling me a URL.  :)

The move is in the direction you want but we're not there yet.  The fact
that things are now decomposed enough for us to be able to think about
it is a good sign here.

The biggest part of it is that the degree of coupling between the
various ASoC components is high - this is particularly obvious with v1
where the entire card is probed at once.  This includes coupling between
the drivers and the core where the degree of churn is very high right
now due to v2 merging.  It doesn't feel right to give architectures an
API that they can't rely on from one merge window to the next yet,
partly from the point of view of isolating the code for review purposes
and also due to the merge issues which would doubtless crop up.

Another part of it is that some machine drivers can get involved enough
to sit on or cross the boundary from platform data to being drivers for
substantial distinct hardware but that's very much not the case for
everything.

I've been spending time this week working on getting the ability to load
platform and codec drivers independantly merged.  That will at least
allow the instantiation of the ASoC drivers for those to be pushed out
into the architecture code, which is a start and should substantially
reduce the size of most machine drivers.  At the point where that's been
done it's probably worth looking again at the simpler machine drivers.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP general SOC driver.

2008-11-26 Thread Mark Brown
On Wed, Nov 26, 2008 at 12:33:12PM -0800, David Brownell wrote:

> And maybe more; that's just me summarizing unconnected pins,
> not the datasheet.  There are also cost-reduced parts with
> less audio capability.

...

> I make no claim about audio expertise -- the nearest I come
> to it is observing a few years back that there was a huge
> framework hole, which ASoC seems to fill -- but I wonder if
> it shouldn't suffice just to provide a mask of capabilities
> that are wired up on a given board.

It's possible - if the differences can be handled by marking some pins
as connected or disconnected and possible specifying a different system
clock rate to the chip then that approach was what I was expecting.
If there is more substantial differences such as having the clocking
arranged differently then separate drivers start to make more sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP general SOC driver.

2008-11-26 Thread Mark Brown
On Wed, Nov 26, 2008 at 12:44:33PM -0800, David Brownell wrote:

> Let me insert a minor plug from the PM perspective that it would be
> good to find a way that the codecs can sit in the proper places in
> the driver model tree.  Example with twl4030:  it's an I2C device
> and thus should be a child of something like

This is exactly what I'm saying about allowing the machine, codec and
platform drivers to probe separately.  It's the major benefit of v2.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] OMAP general SOC driver.

2008-11-27 Thread Mark Brown
On Thu, Nov 27, 2008 at 12:42:12PM +0200, Grazvydas Ignotas wrote:

> Pandora board uses different master/slave configuration for McBSP bus,
> compared to most other boards. It also has another audio path (in
> addition to TWL4030) - another McBSP link connected to external DAC.
> The DAC needs 256*Fs clock from TWL4030 to function, and is used for
> main audio output. TWL4030 is to be used for 2 MIC + line inputs only,
> and has no outputs connected.

This definitely needs to be a separate machine driver.

> I have main audio path working (McBSP2 to DAC), but not inputs (McBSP4
> to TWL4030). How can this be implemented using current framework?
> Should I register 2 platform devices? Sorry for possible derail or
> stupid questions.

This should all be one ASoC device, especially given that the fact that
all the paths use the TWL4030 somehow.  Have a look at how, eg, the
neo1973 driver handles the bluetooth.  For now there's not
*particularly* sane support for multiple codecs.  Does the DAC have any
software control?  If not it should be fairly straightforward to handle.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   4   5   6   7   8   9   10   >