All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Looijmans <mike.looijmans@topic.nl>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>,
	linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
	cjb@laptop.org, gdjakov@mm-sol.com
Subject: Re: [PATCH] sdhci: Forward EPROBE_DEFER on vmmc and vqmmc regulators
Date: Mon, 7 Apr 2014 15:11:17 +0200	[thread overview]
Message-ID: <5342A3F5.606@topic.nl> (raw)
In-Reply-To: <4902841.krZFXFDZqb@wuerfel>

On 04/07/2014 02:51 PM, Arnd Bergmann wrote:
> On Monday 07 April 2014 14:32:20 Mike Looijmans wrote:
>> On 04/07/2014 02:25 PM, Arnd Bergmann wrote:
>>
>> Judging from the kernel output, regulator_get_optional returns -ENODEV if the
>> supply wasn't found.
>>
>> Maybe the API is confusing (or wrong?) here.
>>
>> If you change the code as per your suggestion, the SD will not work unless you
>> explicitly assign supplies. And judging from what I've seen so far, I am the
>> first to have ever attached a power supply to this controller...
>
> It's certainly not very "optional" if it returns an error here.
>
> You could try to special-case the "-ENODEV" return here, but I think it
> would be better to change the regulator interface to be less confusing.
>

Judging from the code:
http://lxr.free-electrons.com/source/drivers/regulator/core.c#L1476

The behaviour is supposed to be something like "regulator_get_optional" will 
return an error if the supply wasn't found, but "regulator_get" will create a 
dummy supply for you instead of returning an error code.

So "regulator_get_optional" means: "I handle my own problems, just gimme the 
bad news". But "regulator_get" appears to imply: "I will shoot the messenger, 
so do whatever you can to get me something to say 'enable' to".

In this case, the IS_NULL part is wrong indeed, it will never return NULL.

I still think it's unrelated to my patch and should be submitted separately.

Mike.


Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

Visit us at the Hannover Messe 7 - 11 April 2014 - Hall 002/D10 (Dutch Pavillion)
http://www.hannovermesse.de/exhibitor/topic-embedded-products/V229623


WARNING: multiple messages have this Message-ID (diff)
From: Mike Looijmans <mike.looijmans@topic.nl>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>,
	<linux-mmc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<cjb@laptop.org>, <gdjakov@mm-sol.com>
Subject: Re: [PATCH] sdhci: Forward EPROBE_DEFER on vmmc and vqmmc regulators
Date: Mon, 7 Apr 2014 15:11:17 +0200	[thread overview]
Message-ID: <5342A3F5.606@topic.nl> (raw)
In-Reply-To: <4902841.krZFXFDZqb@wuerfel>

On 04/07/2014 02:51 PM, Arnd Bergmann wrote:
> On Monday 07 April 2014 14:32:20 Mike Looijmans wrote:
>> On 04/07/2014 02:25 PM, Arnd Bergmann wrote:
>>
>> Judging from the kernel output, regulator_get_optional returns -ENODEV if the
>> supply wasn't found.
>>
>> Maybe the API is confusing (or wrong?) here.
>>
>> If you change the code as per your suggestion, the SD will not work unless you
>> explicitly assign supplies. And judging from what I've seen so far, I am the
>> first to have ever attached a power supply to this controller...
>
> It's certainly not very "optional" if it returns an error here.
>
> You could try to special-case the "-ENODEV" return here, but I think it
> would be better to change the regulator interface to be less confusing.
>

Judging from the code:
http://lxr.free-electrons.com/source/drivers/regulator/core.c#L1476

The behaviour is supposed to be something like "regulator_get_optional" will 
return an error if the supply wasn't found, but "regulator_get" will create a 
dummy supply for you instead of returning an error code.

So "regulator_get_optional" means: "I handle my own problems, just gimme the 
bad news". But "regulator_get" appears to imply: "I will shoot the messenger, 
so do whatever you can to get me something to say 'enable' to".

In this case, the IS_NULL part is wrong indeed, it will never return NULL.

I still think it's unrelated to my patch and should be submitted separately.

Mike.


Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

Visit us at the Hannover Messe 7 - 11 April 2014 - Hall 002/D10 (Dutch Pavillion)
http://www.hannovermesse.de/exhibitor/topic-embedded-products/V229623


  reply	other threads:[~2014-04-07 13:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-07 14:18 [PATCH] sdhci: Forward EPROBE_DEFER on vmmc and vqmmc regulators Mike Looijmans
2014-03-26 15:09 ` Georgi Djakov
2014-03-27 17:47   ` Mike Looijmans
2014-03-27 21:13     ` Georgi Djakov
2014-03-28  7:30 ` Mike Looijmans
2014-03-28 10:20   ` Ulf Hansson
2014-04-07  6:38   ` Mike Looijmans
2014-04-07  6:45     ` Mike Looijmans
2014-04-07  8:11     ` Arnd Bergmann
2014-04-07 12:09       ` Mike Looijmans
2014-04-07 12:09         ` Mike Looijmans
2014-04-07 12:16         ` Ben Dooks
2014-04-07 12:18           ` Ben Dooks
2014-04-07 12:25             ` Arnd Bergmann
2014-04-07 12:32               ` Mike Looijmans
2014-04-07 12:32                 ` Mike Looijmans
2014-04-07 12:51                 ` Arnd Bergmann
2014-04-07 13:11                   ` Mike Looijmans [this message]
2014-04-07 13:11                     ` Mike Looijmans
2014-04-16 20:15                     ` Andrew Bresticker
2014-04-16 20:47                       ` Mark Brown

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=5342A3F5.606@topic.nl \
    --to=mike.looijmans@topic.nl \
    --cc=arnd@arndb.de \
    --cc=ben.dooks@codethink.co.uk \
    --cc=cjb@laptop.org \
    --cc=gdjakov@mm-sol.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    /path/to/YOUR_REPLY

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

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