On Sunday 17 February 2013 19:17:59 Mark Brown wrote:
> On Sun, Feb 17, 2013 at 05:40:41PM +0100, Pali Rohár wrote:
> > $ dmesg | grep lis3
> > [ 110.454345] lis3lv02d_i2c 3-001d: Failed to get supply
> > 'Vdd': -517 [ 110.457397] i2c 3-001d: Driver lis3lv02d_i2c
> > requests probe deferral [ 111.507415] lis3lv02d_i2c
> > 3-001d: Failed to get supply 'Vdd': -517 [ 111.510528] i2c
> > 3-001d: Driver lis3lv02d_i2c requests probe deferral [
> > 126.720458] lis3lv02d_i2c 3-001d: Failed to get supply
> > 'Vdd': -517 [ 126.728149] i2c 3-001d: Driver lis3lv02d_i2c
> > requests probe deferral [ 135.842803] lis3lv02d_i2c
> > 3-001d: Failed to get supply 'Vdd': -517 [ 135.856292] i2c
> > 3-001d: Driver lis3lv02d_i2c requests probe deferral
>
> This just looks like a basic error in the board hookup - the
> supplies are not mapped for the device. Add the supply
> mappings and you should be fine. This should have been done
> when the device was added, the commit doing that looks like
> it was never tested in mainline as the driver problem was
> fixed in October 2011 but the RX51 didn't start using the
> device until May 2012.
There is no regulator 'Vdd' supply in board rx51 files. Can you
look at code how it can be fixed?
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-omap2/board-rx51-peripherals.c#n86
--
Pali Rohár
[email protected]
On Sun, Mar 24, 2013 at 03:50:42PM +0100, Pali Roh?r wrote:
> There is no regulator 'Vdd' supply in board rx51 files. Can you
> look at code how it can be fixed?
You need to look at the schematics rather than the code here - the code
just needs to say what regulator supplies Vdd on the device.
On Sunday 24 March 2013 22:21:49 Mark Brown wrote:
> On Sun, Mar 24, 2013 at 03:50:42PM +0100, Pali Rohár wrote:
> > There is no regulator 'Vdd' supply in board rx51 files. Can
> > you look at code how it can be fixed?
>
> You need to look at the schematics rather than the code here -
> the code just needs to say what regulator supplies Vdd on the
> device.
This is problem, there is no info about regulator supplies for
this device...
--
Pali Rohár
[email protected]
On Sun, Mar 24, 2013 at 10:27:32PM +0100, Pali Roh?r wrote:
> On Sunday 24 March 2013 22:21:49 Mark Brown wrote:
> > You need to look at the schematics rather than the code here -
> > the code just needs to say what regulator supplies Vdd on the
> > device.
> This is problem, there is no info about regulator supplies for
> this device...
Well, you should seek support from the board vendor then. Or try
things like continuity tests (the regulator will have external
components, most likely there's at least a decoupling cap you can probe
at the device end too).
On Sunday 24 March 2013 23:14:46 Mark Brown wrote:
> On Sun, Mar 24, 2013 at 10:27:32PM +0100, Pali Rohár wrote:
> > On Sunday 24 March 2013 22:21:49 Mark Brown wrote:
> > > You need to look at the schematics rather than the code
> > > here - the code just needs to say what regulator supplies
> > > Vdd on the device.
> >
> > This is problem, there is no info about regulator supplies
> > for this device...
>
> Well, you should seek support from the board vendor then.
Not possible. Nokia is already using Windows Phones and life
cycle for Nokia N900 phone is at the end. And Nokia never
released any HW documentations to community...
> Or try things like continuity tests (the regulator will have
> external components, most likely there's at least a
> decoupling cap you can probe at the device end too).
Disassembling this mobile phone is not simple and I dubt that it
will help me...
Another question: what was reason for that commit
ec400c9fab99d16a491cea17d27d0c6a5780b97c
"lis3lv02d: make regulator API usage unconditional" ?
I think that for N900 support is reverting above commit needed, I
do not see other solution...
--
Pali Rohár
[email protected]
On Sun, Mar 24, 2013 at 11:44:59PM +0100, Pali Roh?r wrote:
> On Sunday 24 March 2013 23:14:46 Mark Brown wrote:
> > Well, you should seek support from the board vendor then.
> Not possible. Nokia is already using Windows Phones and life
> cycle for Nokia N900 phone is at the end. And Nokia never
> released any HW documentations to community...
There may well still be people with the required information, though in
general this sort of thing is always going to be a hazard when working
on undocumented hardware.
> Another question: what was reason for that commit
> ec400c9fab99d16a491cea17d27d0c6a5780b97c
> "lis3lv02d: make regulator API usage unconditional" ?
Failing to provide power to a device is typically a very serious
problem for device operation but the code that was there handles such
errors by ignoring them. This isn't a robust way forwards and such code
should never have been merged in the first place, as the changelog says
the regulator core provides a number of facilities for stubbing itself
out when it is not required which boards should use.
Handling the possibility that supplies may not be there not only creates
needless repetitive complexity in device drivers but also decreases the
robustness of the system since error handling for access to powered down
devices often isn't very pretty and other drivers or the core may
disrupt the operation of the device by for example powering it down due
to not thinking it's in use.
> I think that for N900 support is reverting above commit needed, I
> do not see other solution...
If you're convinced that the regulator is kept on for some reason you
could always just provide a fake supply, though obviously it would be
better to hook up the real regulator since this may break if at some
point the kernel decides that whatever is actually providing the supply
is unused and can be turned off.
On Monday 25 March 2013 00:04:37 Mark Brown wrote:
> On Sun, Mar 24, 2013 at 11:44:59PM +0100, Pali Rohár wrote:
> > On Sunday 24 March 2013 23:14:46 Mark Brown wrote:
> > > Well, you should seek support from the board vendor then.
> >
> > Not possible. Nokia is already using Windows Phones and life
> > cycle for Nokia N900 phone is at the end. And Nokia never
> > released any HW documentations to community...
>
> There may well still be people with the required information,
> though in general this sort of thing is always going to be a
> hazard when working on undocumented hardware.
>
> > Another question: what was reason for that commit
> > ec400c9fab99d16a491cea17d27d0c6a5780b97c
> > "lis3lv02d: make regulator API usage unconditional" ?
>
> Failing to provide power to a device is typically a very
> serious problem for device operation but the code that was
> there handles such errors by ignoring them. This isn't a
> robust way forwards and such code should never have been
> merged in the first place, as the changelog says the
> regulator core provides a number of facilities for stubbing
> itself out when it is not required which boards should use.
>
> Handling the possibility that supplies may not be there not
> only creates needless repetitive complexity in device drivers
> but also decreases the robustness of the system since error
> handling for access to powered down devices often isn't very
> pretty and other drivers or the core may disrupt the
> operation of the device by for example powering it down due
> to not thinking it's in use.
>
> > I think that for N900 support is reverting above commit
> > needed, I do not see other solution...
>
> If you're convinced that the regulator is kept on for some
> reason you could always just provide a fake supply, though
> obviously it would be better to hook up the real regulator
> since this may break if at some point the kernel decides that
> whatever is actually providing the supply is unused and can
> be turned off.
CCing Aaro and Tony. Look at this thread on:
https://lkml.org/lkml/2013/2/16/152
What do you think how to fix this problem? I do not know about any
HW regulator for n900 accelerometer and possible solutions could
be revert that commit or adding fake regulator to board code...
--
Pali Rohár
[email protected]
On Tue, Mar 26, 2013 at 04:02:39PM +0100, Pali Roh?r wrote:
> What do you think how to fix this problem? I do not know about any
> HW regulator for n900 accelerometer and possible solutions could
> be revert that commit or adding fake regulator to board code...
Reverting the fix is not an option, this is a clear bug in the platform.
There will be a regulator here, the device needs power.
On Tue, Mar 26, 2013 at 8:44 AM, Mark Brown
<[email protected]> wrote:
> On Tue, Mar 26, 2013 at 04:02:39PM +0100, Pali Rohár wrote:
>
>> What do you think how to fix this problem? I do not know about any
>> HW regulator for n900 accelerometer and possible solutions could
>> be revert that commit or adding fake regulator to board code...
>
> Reverting the fix is not an option, this is a clear bug in the platform.
>
> There will be a regulator here, the device needs power.
Bullshit.
This is a regression, and it needs to be fixed. The "device needs
power" crap is just that - crap. Nobody cares. OF COURSE all devices
need power, but that is totally irrelevant for a driver. The SSD in my
system "needs power", but I have absolutely zero regulator information
for it NOR DO I F*CKING NEED ANY!
Claiming that we need to know the power regulator for an accelerometer
is total utter idiocy and crap.
I'm going to revert that commit unless you can fix it some other way
(dummy regulators when you can't find a real one or whatever). The
notion that you have to have regulator information in order to use
some random device is insanity. I don't understand how you can even
start to make excuses like that. It's so obviously bogus that it's not
even funny.
Why do I have to explain the "no regressions" to long-time kernel
maintainers EVERY SINGLE RELEASE? What the f*ck is *wrong* with you
people? Seriously?
Linus
On Tue, Mar 26, 2013 at 09:08:08AM -0700, Linus Torvalds wrote:
> This is a regression, and it needs to be fixed. The "device needs
This isn't a regression, the commit they're complaining about (ec400c)
was added in October 2011 with a note about all the in-tree users having
been looked after and then the commit adding the registration of the
device (3b511201) was added in May 2012 - the device has just never,
ever worked in mainline on this board as far as I can tell. It's not
like this was even two commits going in in the same release cycle.
As far as I can tell what's happened here is that someone got round to
testing what was sent upstream for the board and noticed that it never
worked.
> power" crap is just that - crap. Nobody cares. OF COURSE all devices
> need power, but that is totally irrelevant for a driver. The SSD in my
> system "needs power", but I have absolutely zero regulator information
> for it NOR DO I F*CKING NEED ANY!
Right, and this is why the core provides facilities for stubbing things
out. There's no point in every single driver going and implementing the
same code for handling this stuff, it's repetitive at best.
> I'm going to revert that commit unless you can fix it some other way
> (dummy regulators when you can't find a real one or whatever). The
That's pretty much what I'm telling them to do - put the stub regulators
in if they don't know how things are connected on their board (there's
helpers to make this easier).
There is also an existing Kconfig option to just stub out any missing
regulator which people can use but it's not recommended for production
since it confuses things that may really have optional supplies and want
to handle that.
> notion that you have to have regulator information in order to use
> some random device is insanity. I don't understand how you can even
> start to make excuses like that. It's so obviously bogus that it's not
> even funny.
There's definitely room for improving the stubs, I would like to see
something which lets boards say "these specific regulators are mapped,
these supplies don't exist and stub anything else out with a dummy", but
users like rx51 should be well enough catered for at the minute.
Hi,
On Tue, Mar 26, 2013 at 04:02:39PM +0100, Pali Roh?r wrote:
> CCing Aaro and Tony. Look at this thread on:
> https://lkml.org/lkml/2013/2/16/152
>
> What do you think how to fix this problem? I do not know about any
> HW regulator for n900 accelerometer and possible solutions could
> be revert that commit or adding fake regulator to board code...
I think the following should work:
...
From: Aaro Koskinen <[email protected]>
Date: Tue, 26 Mar 2013 21:34:22 +0200
Subject: [PATCH] OMAP: RX-51: add missing regulator supply definitions for lis3lv02d
Add missing regulator definitions for lis3lv02d accelerometer. Fixes
the following probe issue:
[ 57.737518] lis3lv02d_i2c 3-001d: Failed to get supply 'Vdd': -517
[ 57.747100] i2c 3-001d: Driver lis3lv02d_i2c requests probe deferral
Signed-off-by: Aaro Koskinen <[email protected]>
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 3a077df..1a88467 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -547,12 +547,16 @@ static struct regulator_consumer_supply rx51_vio_supplies[] = {
REGULATOR_SUPPLY("DVDD", "2-0019"),
/* Si4713 IO supply */
REGULATOR_SUPPLY("vio", "2-0063"),
+ /* lis3lv02d */
+ REGULATOR_SUPPLY("Vdd_IO", "3-001d"),
};
static struct regulator_consumer_supply rx51_vaux1_consumers[] = {
REGULATOR_SUPPLY("vdds_sdi", "omapdss"),
/* Si4713 supply */
REGULATOR_SUPPLY("vdd", "2-0063"),
+ /* lis3lv02d */
+ REGULATOR_SUPPLY("Vdd", "3-001d"),
};
static struct regulator_init_data rx51_vaux1 = {
--
1.7.10.4
A.
On Tue, Mar 26, 2013 at 10:53:19PM +0200, Aaro Koskinen wrote:
> From: Aaro Koskinen <[email protected]>
> Date: Tue, 26 Mar 2013 21:34:22 +0200
> Subject: [PATCH] OMAP: RX-51: add missing regulator supply definitions for lis3lv02d
>
> Add missing regulator definitions for lis3lv02d accelerometer. Fixes
> the following probe issue:
Reviewed-by: Mark Brown <[email protected]>
This is exactly the sort of thing I'd expect to see the board doing.
On Tuesday 26 March 2013 21:53:19 Aaro Koskinen wrote:
> Hi,
>
> On Tue, Mar 26, 2013 at 04:02:39PM +0100, Pali Rohár wrote:
> > CCing Aaro and Tony. Look at this thread on:
> > https://lkml.org/lkml/2013/2/16/152
> >
> > What do you think how to fix this problem? I do not know
> > about any HW regulator for n900 accelerometer and possible
> > solutions could be revert that commit or adding fake
> > regulator to board code...
>
> I think the following should work:
>
> ...
>
> From: Aaro Koskinen <[email protected]>
> Date: Tue, 26 Mar 2013 21:34:22 +0200
> Subject: [PATCH] OMAP: RX-51: add missing regulator supply
> definitions for lis3lv02d
>
> Add missing regulator definitions for lis3lv02d accelerometer.
> Fixes the following probe issue:
>
> [ 57.737518] lis3lv02d_i2c 3-001d: Failed to get supply
> 'Vdd': -517 [ 57.747100] i2c 3-001d: Driver lis3lv02d_i2c
> requests probe deferral
>
> Signed-off-by: Aaro Koskinen <[email protected]>
> ---
> arch/arm/mach-omap2/board-rx51-peripherals.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
> b/arch/arm/mach-omap2/board-rx51-peripherals.c index
> 3a077df..1a88467 100644
> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> @@ -547,12 +547,16 @@ static struct regulator_consumer_supply
> rx51_vio_supplies[] = { REGULATOR_SUPPLY("DVDD", "2-0019"),
> /* Si4713 IO supply */
> REGULATOR_SUPPLY("vio", "2-0063"),
> + /* lis3lv02d */
> + REGULATOR_SUPPLY("Vdd_IO", "3-001d"),
> };
>
> static struct regulator_consumer_supply
> rx51_vaux1_consumers[] = { REGULATOR_SUPPLY("vdds_sdi",
> "omapdss"),
> /* Si4713 supply */
> REGULATOR_SUPPLY("vdd", "2-0063"),
> + /* lis3lv02d */
> + REGULATOR_SUPPLY("Vdd", "3-001d"),
> };
>
> static struct regulator_init_data rx51_vaux1 = {
Hi, on #maemo IRC channel we have found some accelerometer
regulators and I wrote this patch which is working fine with 3.8:
@@ -728,12 +730,16 @@ static struct regulator_consumer_supply
rx51_vio_supplies[] = {
REGULATOR_SUPPLY("DVDD", "2-0019"),
/* Si4713 IO supply */
REGULATOR_SUPPLY("vio", "2-0063"),
+ /* lis3lv02d IO supply */
+ REGULATOR_SUPPLY("Vdd_IO", "3-001d"),
};
static struct regulator_consumer_supply rx51_vaux1_consumers[] =
{
REGULATOR_SUPPLY("vdds_sdi", "omapdss"),
/* Si4713 supply */
REGULATOR_SUPPLY("vdd", "2-0063"),
+ /* lis3lv02d supply */
+ REGULATOR_SUPPLY("Vdd", "3-001d"),
};
static struct regulator_init_data rx51_vaux1 = {
Now I see that patch is same as Aaro's :-) so please include it.
--
Pali Rohár
[email protected]