[PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Matti Vaittinen]

The ROHM BD718x7 and BD71828 drivers support setting HW state
specific voltages from device-tree. This is used also by various
in-tree DTS files.

These drivers do incorrectly try to compose bit-map using enum
values. By a chance this works for first two valid levels having
values 1 and 2 - but setting values for the rest of the levels
do indicate capbility of setting values for first levels as
well. Luckily the regulators which support settin values for
SUSPEND/LPSR do usually also support setting values for RUN
and IDLE too - thus this has not been such a fatal issue.

Fix this by defining the old enum values as bits and using
new enum in parsing code. This allows keeping existing IC
specific drivers intact and only adding the defines and
slightly changing the rohm-regulator.c

Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common and bd718x7 specific parts")
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/regulator/rohm-regulator.c |  8 ++++----
 include/linux/mfd/rohm-generic.h   | 22 ++++++++++++++++------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/rohm-regulator.c b/drivers/regulator/rohm-regulator.c
index 399002383b28..96caae7dbef4 100644
--- a/drivers/regulator/rohm-regulator.c
+++ b/drivers/regulator/rohm-regulator.c
@@ -55,25 +55,25 @@ int rohm_regulator_set_dvs_levels(const struct rohm_dvs_config *dvs,
 	for (i = 0; i < ROHM_DVS_LEVEL_MAX && !ret; i++) {
 		if (dvs->level_map & (1 << i)) {
 			switch (i + 1) {
-			case ROHM_DVS_LEVEL_RUN:
+			case _ROHM_DVS_LEVEL_RUN:
 				prop = "rohm,dvs-run-voltage";
 				reg = dvs->run_reg;
 				mask = dvs->run_mask;
 				omask = dvs->run_on_mask;
 				break;
-			case ROHM_DVS_LEVEL_IDLE:
+			case _ROHM_DVS_LEVEL_IDLE:
 				prop = "rohm,dvs-idle-voltage";
 				reg = dvs->idle_reg;
 				mask = dvs->idle_mask;
 				omask = dvs->idle_on_mask;
 				break;
-			case ROHM_DVS_LEVEL_SUSPEND:
+			case _ROHM_DVS_LEVEL_SUSPEND:
 				prop = "rohm,dvs-suspend-voltage";
 				reg = dvs->suspend_reg;
 				mask = dvs->suspend_mask;
 				omask = dvs->suspend_on_mask;
 				break;
-			case ROHM_DVS_LEVEL_LPSR:
+			case _ROHM_DVS_LEVEL_LPSR:
 				prop = "rohm,dvs-lpsr-voltage";
 				reg = dvs->lpsr_reg;
 				mask = dvs->lpsr_mask;
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 4283b5b33e04..a557988831d7 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -20,15 +20,25 @@ struct rohm_regmap_dev {
 	struct regmap *regmap;
 };
 
+/*
+ * Do not use these in IC specific driver - the bit map should be created using
+ * defines without the underscore. These should be used only in rohm-regulator.c
+ */
 enum {
-	ROHM_DVS_LEVEL_UNKNOWN,
-	ROHM_DVS_LEVEL_RUN,
-	ROHM_DVS_LEVEL_IDLE,
-	ROHM_DVS_LEVEL_SUSPEND,
-	ROHM_DVS_LEVEL_LPSR,
-	ROHM_DVS_LEVEL_MAX = ROHM_DVS_LEVEL_LPSR,
+	_ROHM_DVS_LEVEL_UNKNOWN,
+	_ROHM_DVS_LEVEL_RUN,
+	_ROHM_DVS_LEVEL_IDLE,
+	_ROHM_DVS_LEVEL_SUSPEND,
+	_ROHM_DVS_LEVEL_LPSR,
+	ROHM_DVS_LEVEL_MAX = _ROHM_DVS_LEVEL_LPSR,
 };
 
+#define ROHM_DVS_LEVEL_UNKNOWN	(1 << _ROHM_DVS_LEVEL_UNKNOWN)
+#define ROHM_DVS_LEVEL_RUN	(1 << _ROHM_DVS_LEVEL_RUN)
+#define ROHM_DVS_LEVEL_IDLE	(1 << _ROHM_DVS_LEVEL_IDLE)
+#define ROHM_DVS_LEVEL_SUSPEND	(1 << _ROHM_DVS_LEVEL_SUSPEND)
+#define ROHM_DVS_LEVEL_LPSR	(1 << _ROHM_DVS_LEVEL_LPSR)
+
 /**
  * struct rohm_dvs_config - dynamic voltage scaling register descriptions
  *

base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

Re: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Lee Jones]

On Fri, 15 Jan 2021, Matti Vaittinen wrote:

> The ROHM BD718x7 and BD71828 drivers support setting HW state
> specific voltages from device-tree. This is used also by various
> in-tree DTS files.
> 
> These drivers do incorrectly try to compose bit-map using enum
> values. By a chance this works for first two valid levels having
> values 1 and 2 - but setting values for the rest of the levels
> do indicate capbility of setting values for first levels as
> well. Luckily the regulators which support settin values for
> SUSPEND/LPSR do usually also support setting values for RUN
> and IDLE too - thus this has not been such a fatal issue.
> 
> Fix this by defining the old enum values as bits and using
> new enum in parsing code. This allows keeping existing IC
> specific drivers intact and only adding the defines and
> slightly changing the rohm-regulator.c
> 
> Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common and bd718x7 specific parts")
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
>  drivers/regulator/rohm-regulator.c |  8 ++++----
>  include/linux/mfd/rohm-generic.h   | 22 ++++++++++++++++------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/regulator/rohm-regulator.c b/drivers/regulator/rohm-regulator.c
> index 399002383b28..96caae7dbef4 100644
> --- a/drivers/regulator/rohm-regulator.c
> +++ b/drivers/regulator/rohm-regulator.c
> @@ -55,25 +55,25 @@ int rohm_regulator_set_dvs_levels(const struct rohm_dvs_config *dvs,
>  	for (i = 0; i < ROHM_DVS_LEVEL_MAX && !ret; i++) {
>  		if (dvs->level_map & (1 << i)) {
>  			switch (i + 1) {
> -			case ROHM_DVS_LEVEL_RUN:
> +			case _ROHM_DVS_LEVEL_RUN:
>  				prop = "rohm,dvs-run-voltage";
>  				reg = dvs->run_reg;
>  				mask = dvs->run_mask;
>  				omask = dvs->run_on_mask;
>  				break;
> -			case ROHM_DVS_LEVEL_IDLE:
> +			case _ROHM_DVS_LEVEL_IDLE:
>  				prop = "rohm,dvs-idle-voltage";
>  				reg = dvs->idle_reg;
>  				mask = dvs->idle_mask;
>  				omask = dvs->idle_on_mask;
>  				break;
> -			case ROHM_DVS_LEVEL_SUSPEND:
> +			case _ROHM_DVS_LEVEL_SUSPEND:
>  				prop = "rohm,dvs-suspend-voltage";
>  				reg = dvs->suspend_reg;
>  				mask = dvs->suspend_mask;
>  				omask = dvs->suspend_on_mask;
>  				break;
> -			case ROHM_DVS_LEVEL_LPSR:
> +			case _ROHM_DVS_LEVEL_LPSR:
>  				prop = "rohm,dvs-lpsr-voltage";
>  				reg = dvs->lpsr_reg;
>  				mask = dvs->lpsr_mask;
> diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
> index 4283b5b33e04..a557988831d7 100644
> --- a/include/linux/mfd/rohm-generic.h
> +++ b/include/linux/mfd/rohm-generic.h
> @@ -20,15 +20,25 @@ struct rohm_regmap_dev {
>  	struct regmap *regmap;
>  };
>  
> +/*
> + * Do not use these in IC specific driver - the bit map should be created using
> + * defines without the underscore. These should be used only in rohm-regulator.c
> + */
>  enum {
> -	ROHM_DVS_LEVEL_UNKNOWN,
> -	ROHM_DVS_LEVEL_RUN,
> -	ROHM_DVS_LEVEL_IDLE,
> -	ROHM_DVS_LEVEL_SUSPEND,
> -	ROHM_DVS_LEVEL_LPSR,
> -	ROHM_DVS_LEVEL_MAX = ROHM_DVS_LEVEL_LPSR,
> +	_ROHM_DVS_LEVEL_UNKNOWN,
> +	_ROHM_DVS_LEVEL_RUN,
> +	_ROHM_DVS_LEVEL_IDLE,
> +	_ROHM_DVS_LEVEL_SUSPEND,
> +	_ROHM_DVS_LEVEL_LPSR,
> +	ROHM_DVS_LEVEL_MAX = _ROHM_DVS_LEVEL_LPSR,

I don't understand how this is still not broken.

I think you either need to change the for() loop that consumes this to
use "<=" or push the MAX enum to the last line (on its own).

The latter would be my personal preference.

>  };
>  
> +#define ROHM_DVS_LEVEL_UNKNOWN	(1 << _ROHM_DVS_LEVEL_UNKNOWN)
> +#define ROHM_DVS_LEVEL_RUN	(1 << _ROHM_DVS_LEVEL_RUN)
> +#define ROHM_DVS_LEVEL_IDLE	(1 << _ROHM_DVS_LEVEL_IDLE)
> +#define ROHM_DVS_LEVEL_SUSPEND	(1 << _ROHM_DVS_LEVEL_SUSPEND)
> +#define ROHM_DVS_LEVEL_LPSR	(1 << _ROHM_DVS_LEVEL_LPSR)
> +
>  /**
>   * struct rohm_dvs_config - dynamic voltage scaling register descriptions
>   *
> 
> base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Vaittinen, Matti]

On Fri, 2021-01-15 at 13:47 +0000, Lee Jones wrote:
> On Fri, 15 Jan 2021, Matti Vaittinen wrote:
> 
> > The ROHM BD718x7 and BD71828 drivers support setting HW state
> > specific voltages from device-tree. This is used also by various
> > in-tree DTS files.
> > 
> > These drivers do incorrectly try to compose bit-map using enum
> > values. By a chance this works for first two valid levels having
> > values 1 and 2 - but setting values for the rest of the levels
> > do indicate capbility of setting values for first levels as
> > well. Luckily the regulators which support settin values for
> > SUSPEND/LPSR do usually also support setting values for RUN
> > and IDLE too - thus this has not been such a fatal issue.
> > 
> > Fix this by defining the old enum values as bits and using
> > new enum in parsing code. This allows keeping existing IC
> > specific drivers intact and only adding the defines and
> > slightly changing the rohm-regulator.c
> > 
> > Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common
> > and bd718x7 specific parts")
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> >  drivers/regulator/rohm-regulator.c |  8 ++++----
> >  include/linux/mfd/rohm-generic.h   | 22 ++++++++++++++++------
> >  2 files changed, 20 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/regulator/rohm-regulator.c
> > b/drivers/regulator/rohm-regulator.c
> > index 399002383b28..96caae7dbef4 100644
> > --- a/drivers/regulator/rohm-regulator.c
> > +++ b/drivers/regulator/rohm-regulator.c
> > @@ -55,25 +55,25 @@ int rohm_regulator_set_dvs_levels(const struct
> > rohm_dvs_config *dvs,
> >  	for (i = 0; i < ROHM_DVS_LEVEL_MAX && !ret; i++) {
> >  		if (dvs->level_map & (1 << i)) {
> >  			switch (i + 1) {
> > -			case ROHM_DVS_LEVEL_RUN:
> > +			case _ROHM_DVS_LEVEL_RUN:
> >  				prop = "rohm,dvs-run-voltage";
> >  				reg = dvs->run_reg;
> >  				mask = dvs->run_mask;
> >  				omask = dvs->run_on_mask;
> >  				break;
> > -			case ROHM_DVS_LEVEL_IDLE:
> > +			case _ROHM_DVS_LEVEL_IDLE:
> >  				prop = "rohm,dvs-idle-voltage";
> >  				reg = dvs->idle_reg;
> >  				mask = dvs->idle_mask;
> >  				omask = dvs->idle_on_mask;
> >  				break;
> > -			case ROHM_DVS_LEVEL_SUSPEND:
> > +			case _ROHM_DVS_LEVEL_SUSPEND:
> >  				prop = "rohm,dvs-suspend-voltage";
> >  				reg = dvs->suspend_reg;
> >  				mask = dvs->suspend_mask;
> >  				omask = dvs->suspend_on_mask;
> >  				break;
> > -			case ROHM_DVS_LEVEL_LPSR:
> > +			case _ROHM_DVS_LEVEL_LPSR:
> >  				prop = "rohm,dvs-lpsr-voltage";
> >  				reg = dvs->lpsr_reg;
> >  				mask = dvs->lpsr_mask;
> > diff --git a/include/linux/mfd/rohm-generic.h
> > b/include/linux/mfd/rohm-generic.h
> > index 4283b5b33e04..a557988831d7 100644
> > --- a/include/linux/mfd/rohm-generic.h
> > +++ b/include/linux/mfd/rohm-generic.h
> > @@ -20,15 +20,25 @@ struct rohm_regmap_dev {
> >  	struct regmap *regmap;
> >  };
> >  
> > +/*
> > + * Do not use these in IC specific driver - the bit map should be
> > created using
> > + * defines without the underscore. These should be used only in
> > rohm-regulator.c
> > + */
> >  enum {
> > -	ROHM_DVS_LEVEL_UNKNOWN,
> > -	ROHM_DVS_LEVEL_RUN,
> > -	ROHM_DVS_LEVEL_IDLE,
> > -	ROHM_DVS_LEVEL_SUSPEND,
> > -	ROHM_DVS_LEVEL_LPSR,
> > -	ROHM_DVS_LEVEL_MAX = ROHM_DVS_LEVEL_LPSR,
> > +	_ROHM_DVS_LEVEL_UNKNOWN,
> > +	_ROHM_DVS_LEVEL_RUN,
> > +	_ROHM_DVS_LEVEL_IDLE,
> > +	_ROHM_DVS_LEVEL_SUSPEND,
> > +	_ROHM_DVS_LEVEL_LPSR,
> > +	ROHM_DVS_LEVEL_MAX = _ROHM_DVS_LEVEL_LPSR,
> 
> I don't understand how this is still not broken.
> 
> I think you either need to change the for() loop that consumes this
> to
> use "<=" or push the MAX enum to the last line (on its own).
> 
> The latter would be my personal preference.

Bah. Occasionally getting it right is just hard. The for loop condition
is allright now as it does as it was originally intended and scans the
amount of valid values. Starting at 0, using condition i <
ROHM_DVS_LEVEL_MAX guarantees we process all valid values _knowing_
that value 0 is 'invalid'.

But now I made a new error in following if-condition. I didn't take
into account the fact that left-sifting the 1 by _ROHM_DVS_LEVEL_RUN
for first valid value (ROHM_DVS_LEVEL_RUN) makes the first valid
bitmask to be 2 not 1. So the if-condition

if (dvs->level_map & (1 << i))

following the for loop is now broken. It would probably work if it was
2 << i. Anyways - we're now wasting one bit out of 32 for invalid mask.

I think you're correct though. It needs a change. This is quite good
indication that this loop/check is overly complex :) So please forget
this patch - I will do better at next week when I get to the office and
can also test this with a break-out board. 

Sorry for wasting the time and thanks for the review again.

Best Regards
	Matti Vaittinen

[PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Matti Vaittinen]

The ROHM BD718x7 and BD71828 drivers support setting HW state
specific voltages from device-tree. This is used also by various
in-tree DTS files.

These drivers do incorrectly try to compose bit-map using enum
values. By a chance this works for first two valid levels having
values 1 and 2 - but setting values for the rest of the levels
do indicate capbility of setting values for first levels as
well. Luckily the regulators which support settin values for
SUSPEND/LPSR do usually also support setting values for RUN
and IDLE too - thus this has not been such a fatal issue.

Fix this by defining the old enum values as bits and using
new enum in parsing code. This allows keeping existing IC
specific drivers intact and only adding the defines and
slightly changing the rohm-regulator.c

Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common and bd718x7 specific parts")
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---

One more attempt today. I did test the driver is not causing a crash
when load but no further tests concluded as I don't have BD71837/47/50
at home. This looks now trivial though so I decided to give it a go.
Sorry for all the trouble this far - and also for the mistakes to come.

 drivers/regulator/rohm-regulator.c |  9 ++++++---
 include/linux/mfd/rohm-generic.h   | 14 ++++++--------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/rohm-regulator.c b/drivers/regulator/rohm-regulator.c
index 399002383b28..5c558b153d55 100644
--- a/drivers/regulator/rohm-regulator.c
+++ b/drivers/regulator/rohm-regulator.c
@@ -52,9 +52,12 @@ int rohm_regulator_set_dvs_levels(const struct rohm_dvs_config *dvs,
 	char *prop;
 	unsigned int reg, mask, omask, oreg = desc->enable_reg;
 
-	for (i = 0; i < ROHM_DVS_LEVEL_MAX && !ret; i++) {
-		if (dvs->level_map & (1 << i)) {
-			switch (i + 1) {
+	for (i = 0; i < ROHM_DVS_LEVEL_VALID_AMOUNT && !ret; i++) {
+		int bit;
+
+		bit = BIT(i);
+		if (dvs->level_map & bit) {
+			switch (bit) {
 			case ROHM_DVS_LEVEL_RUN:
 				prop = "rohm,dvs-run-voltage";
 				reg = dvs->run_reg;
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 4283b5b33e04..2b85b9deb03a 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -20,14 +20,12 @@ struct rohm_regmap_dev {
 	struct regmap *regmap;
 };
 
-enum {
-	ROHM_DVS_LEVEL_UNKNOWN,
-	ROHM_DVS_LEVEL_RUN,
-	ROHM_DVS_LEVEL_IDLE,
-	ROHM_DVS_LEVEL_SUSPEND,
-	ROHM_DVS_LEVEL_LPSR,
-	ROHM_DVS_LEVEL_MAX = ROHM_DVS_LEVEL_LPSR,
-};
+#define ROHM_DVS_LEVEL_RUN		BIT(0)
+#define ROHM_DVS_LEVEL_IDLE		BIT(1)
+#define ROHM_DVS_LEVEL_SUSPEND		BIT(2)
+#define ROHM_DVS_LEVEL_LPSR		BIT(3)
+#define ROHM_DVS_LEVEL_VALID_AMOUNT	4
+#define ROHM_DVS_LEVEL_UNKNOWN		0
 
 /**
  * struct rohm_dvs_config - dynamic voltage scaling register descriptions

base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

Re: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Lee Jones]

On Fri, 15 Jan 2021, Matti Vaittinen wrote:

> The ROHM BD718x7 and BD71828 drivers support setting HW state
> specific voltages from device-tree. This is used also by various
> in-tree DTS files.
> 
> These drivers do incorrectly try to compose bit-map using enum
> values. By a chance this works for first two valid levels having
> values 1 and 2 - but setting values for the rest of the levels
> do indicate capbility of setting values for first levels as
> well. Luckily the regulators which support settin values for
> SUSPEND/LPSR do usually also support setting values for RUN
> and IDLE too - thus this has not been such a fatal issue.
> 
> Fix this by defining the old enum values as bits and using
> new enum in parsing code. This allows keeping existing IC
> specific drivers intact and only adding the defines and
> slightly changing the rohm-regulator.c
> 
> Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common and bd718x7 specific parts")
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> 
> One more attempt today. I did test the driver is not causing a crash
> when load but no further tests concluded as I don't have BD71837/47/50
> at home. This looks now trivial though so I decided to give it a go.
> Sorry for all the trouble this far - and also for the mistakes to come.

Why don't you wait until next week when you can run this on real h/w
with some pretty debug to ensure it does the right thing?

>  drivers/regulator/rohm-regulator.c |  9 ++++++---
>  include/linux/mfd/rohm-generic.h   | 14 ++++++--------
>  2 files changed, 12 insertions(+), 11 deletions(-)

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels

[Vaittinen, Matti]

On Fri, 2021-01-15 at 14:41 +0000, Lee Jones wrote:
> On Fri, 15 Jan 2021, Matti Vaittinen wrote:
> 
> > The ROHM BD718x7 and BD71828 drivers support setting HW state
> > specific voltages from device-tree. This is used also by various
> > in-tree DTS files.
> > 
> > These drivers do incorrectly try to compose bit-map using enum
> > values. By a chance this works for first two valid levels having
> > values 1 and 2 - but setting values for the rest of the levels
> > do indicate capbility of setting values for first levels as
> > well. Luckily the regulators which support settin values for
> > SUSPEND/LPSR do usually also support setting values for RUN
> > and IDLE too - thus this has not been such a fatal issue.
> > 
> > Fix this by defining the old enum values as bits and using
> > new enum in parsing code. This allows keeping existing IC
> > specific drivers intact and only adding the defines and
> > slightly changing the rohm-regulator.c
> > 
> > Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common
> > and bd718x7 specific parts")
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > 
> > One more attempt today. I did test the driver is not causing a
> > crash
> > when load but no further tests concluded as I don't have
> > BD71837/47/50
> > at home. This looks now trivial though so I decided to give it a
> > go.
> > Sorry for all the trouble this far - and also for the mistakes to
> > come.
> 
> Why don't you wait until next week when you can run this on real h/w
> with some pretty debug to ensure it does the right thing?

I first thought I would. Then I didn't wait because - as I said - this
looks pretty trivial to me now - and because I thought it might get
merged quickly. If you see it risky, then please don't merge. I will
test this next week and can resend after that if necessary.

Sorry for the trouble again.

Best Regards
--Matti