2013-04-04 10:31:10

by Paul Bolle

[permalink] [raw]
Subject: [PATCH] Blackfin: bf537: rename "CONFIG_ADT75"

In v3.2 the Analog Devices ADT75 temperature sensor driver was removed
as an IIO driver and support for it was added to the LM75 HWMON driver.
But it was apparently overlooked to rename one reference to CONFIG_ADT75
to CONFIG_SENSORS_LM75. Do so now. Use the IS_ENABLED() macro, while
we're at it.

Signed-off-by: Paul Bolle <[email protected]>
---
0) Also untested.

1) See commits e96f9d89e6213c7630a3323cd0c754e7f2619564 ("hwmon: (lm75)
Add support for Analog Devices ADT75") and
cdea0bec8d37f2943ae500512b0c178bc76de6e3 ("iio: adc: remove ADT75 driver
- hwmon/lm75 will take over ADT75 support") for the two patches involved
in this operation.

arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/blackfin/mach-bf537/boards/stamp.c b/arch/blackfin/mach-bf537/boards/stamp.c
index b40b849..9735345 100644
--- a/arch/blackfin/mach-bf537/boards/stamp.c
+++ b/arch/blackfin/mach-bf537/boards/stamp.c
@@ -2221,7 +2221,7 @@ static struct i2c_board_info __initdata bfin_i2c_board_info[] = {
},
#endif

-#if defined(CONFIG_ADT75) || defined(CONFIG_ADT75_MODULE)
+#if IS_ENABLED(CONFIG_SENSORS_LM75)
{
I2C_BOARD_INFO("adt75", 0x9),
.irq = IRQ_PG5,
--
1.7.11.7


2013-04-04 11:40:28

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH] Blackfin: bf537: rename "CONFIG_ADT75"

On Thu, 04 Apr 2013 12:31:06 +0200, Paul Bolle wrote:
> In v3.2 the Analog Devices ADT75 temperature sensor driver was removed
> as an IIO driver and support for it was added to the LM75 HWMON driver.
> But it was apparently overlooked to rename one reference to CONFIG_ADT75
> to CONFIG_SENSORS_LM75. Do so now. Use the IS_ENABLED() macro, while
> we're at it.
>
> Signed-off-by: Paul Bolle <[email protected]>
> ---
> 0) Also untested.
>
> 1) See commits e96f9d89e6213c7630a3323cd0c754e7f2619564 ("hwmon: (lm75)
> Add support for Analog Devices ADT75") and
> cdea0bec8d37f2943ae500512b0c178bc76de6e3 ("iio: adc: remove ADT75 driver
> - hwmon/lm75 will take over ADT75 support") for the two patches involved
> in this operation.
>
> arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

This breakage is telling us something about the weird approach taken in
this file, methinks.

> diff --git a/arch/blackfin/mach-bf537/boards/stamp.c b/arch/blackfin/mach-bf537/boards/stamp.c
> index b40b849..9735345 100644
> --- a/arch/blackfin/mach-bf537/boards/stamp.c
> +++ b/arch/blackfin/mach-bf537/boards/stamp.c
> @@ -2221,7 +2221,7 @@ static struct i2c_board_info __initdata bfin_i2c_board_info[] = {
> },
> #endif
>
> -#if defined(CONFIG_ADT75) || defined(CONFIG_ADT75_MODULE)
> +#if IS_ENABLED(CONFIG_SENSORS_LM75)
> {
> I2C_BOARD_INFO("adt75", 0x9),

Unrelated to this patch, but this is very odd, as the ADT75 can't have
slave address 0x09. This address is almost exclusively used by battery
chargers AFAIK.

> .irq = IRQ_PG5,

The patch itself looks good.

Reviewed-by: Jean Delvare <[email protected]>

--
Jean Delvare

2013-04-04 12:07:26

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] Blackfin: bf537: rename "CONFIG_ADT75"

On Thu, Apr 04, 2013 at 01:40:03PM +0200, Jean Delvare wrote:
> On Thu, 04 Apr 2013 12:31:06 +0200, Paul Bolle wrote:
> > In v3.2 the Analog Devices ADT75 temperature sensor driver was removed
> > as an IIO driver and support for it was added to the LM75 HWMON driver.
> > But it was apparently overlooked to rename one reference to CONFIG_ADT75
> > to CONFIG_SENSORS_LM75. Do so now. Use the IS_ENABLED() macro, while
> > we're at it.
> >
> > Signed-off-by: Paul Bolle <[email protected]>
> > ---
> > 0) Also untested.
> >
> > 1) See commits e96f9d89e6213c7630a3323cd0c754e7f2619564 ("hwmon: (lm75)
> > Add support for Analog Devices ADT75") and
> > cdea0bec8d37f2943ae500512b0c178bc76de6e3 ("iio: adc: remove ADT75 driver
> > - hwmon/lm75 will take over ADT75 support") for the two patches involved
> > in this operation.
> >
> > arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> This breakage is telling us something about the weird approach taken in
> this file, methinks.
>
Yes, they have persistent problems with renamed or moved drivers. I somewhat
understood the need for SPI chips, but even there it seemed to me there should
be a better solution.

> > diff --git a/arch/blackfin/mach-bf537/boards/stamp.c b/arch/blackfin/mach-bf537/boards/stamp.c
> > index b40b849..9735345 100644
> > --- a/arch/blackfin/mach-bf537/boards/stamp.c
> > +++ b/arch/blackfin/mach-bf537/boards/stamp.c
> > @@ -2221,7 +2221,7 @@ static struct i2c_board_info __initdata bfin_i2c_board_info[] = {
> > },
> > #endif
> >
> > -#if defined(CONFIG_ADT75) || defined(CONFIG_ADT75_MODULE)
> > +#if IS_ENABLED(CONFIG_SENSORS_LM75)
> > {
> > I2C_BOARD_INFO("adt75", 0x9),
>
> Unrelated to this patch, but this is very odd, as the ADT75 can't have
> slave address 0x09. This address is almost exclusively used by battery
> chargers AFAIK.
>
Either not really an ADT75, or it should have been 0x49.

Couple of lines down, CONFIG_ADT7410 has the same problem.

Guenter

> > .irq = IRQ_PG5,
>
> The patch itself looks good.
>
> Reviewed-by: Jean Delvare <[email protected]>
>
> --
> Jean Delvare
>

2013-04-04 13:00:30

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] Blackfin: bf537: rename "CONFIG_ADT75"

On Thu, 2013-04-04 at 05:07 -0700, Guenter Roeck wrote:
> On Thu, Apr 04, 2013 at 01:40:03PM +0200, Jean Delvare wrote:
> > This breakage is telling us something about the weird approach taken in
> > this file, methinks.
> >
> Yes, they have persistent problems with renamed or moved drivers. I somewhat
> understood the need for SPI chips, but even there it seemed to me there should
> be a better solution.

The output of the (local) scripts I use mention about a dozen more
(possible) problems in both Blackfin's stamp.c files. I'm roughly
working my way back in git history, slowly, so those problems are
probably older.

Since I know from previous cleaning up efforts that the Blackfin people
know they have non-working code (eg, code not actually buildable since
it has no working Kconfig support) I do not give Blackfin much priority.
But this patch (and the other two related to the v3.2 release) were
obvious oversights that I could as well fix right away.


Paul Bolle

2013-04-04 13:02:29

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] Blackfin: bf537: rename "CONFIG_ADT75"

On Thu, Apr 04, 2013 at 02:59:43PM +0200, Paul Bolle wrote:
> On Thu, 2013-04-04 at 05:07 -0700, Guenter Roeck wrote:
> > On Thu, Apr 04, 2013 at 01:40:03PM +0200, Jean Delvare wrote:
> > > This breakage is telling us something about the weird approach taken in
> > > this file, methinks.
> > >
> > Yes, they have persistent problems with renamed or moved drivers. I somewhat
> > understood the need for SPI chips, but even there it seemed to me there should
> > be a better solution.
>
> The output of the (local) scripts I use mention about a dozen more
> (possible) problems in both Blackfin's stamp.c files. I'm roughly
> working my way back in git history, slowly, so those problems are
> probably older.
>
> Since I know from previous cleaning up efforts that the Blackfin people
> know they have non-working code (eg, code not actually buildable since
> it has no working Kconfig support) I do not give Blackfin much priority.
> But this patch (and the other two related to the v3.2 release) were
> obvious oversights that I could as well fix right away.
>
Makes me shiver ...

Guenter

2014-02-13 10:15:53

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] Blackfin: bf537: rename "CONFIG_ADT75"

[Replaced previous maintainer and list with current maintainer and
list.]

On Thu, 2013-04-04 at 13:40 +0200, Jean Delvare wrote:
> On Thu, 04 Apr 2013 12:31:06 +0200, Paul Bolle wrote:
> > In v3.2 the Analog Devices ADT75 temperature sensor driver was removed
> > as an IIO driver and support for it was added to the LM75 HWMON driver.
> > But it was apparently overlooked to rename one reference to CONFIG_ADT75
> > to CONFIG_SENSORS_LM75. Do so now. Use the IS_ENABLED() macro, while
> > we're at it.
> >
> > Signed-off-by: Paul Bolle <[email protected]>
> > ---
> > 0) Also untested.
> >
> > 1) See commits e96f9d89e6213c7630a3323cd0c754e7f2619564 ("hwmon: (lm75)
> > Add support for Analog Devices ADT75") and
> > cdea0bec8d37f2943ae500512b0c178bc76de6e3 ("iio: adc: remove ADT75 driver
> > - hwmon/lm75 will take over ADT75 support") for the two patches involved
> > in this operation.
> >
> > arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> This breakage is telling us something about the weird approach taken in
> this file, methinks.
>
> > diff --git a/arch/blackfin/mach-bf537/boards/stamp.c b/arch/blackfin/mach-bf537/boards/stamp.c
> > index b40b849..9735345 100644
> > --- a/arch/blackfin/mach-bf537/boards/stamp.c
> > +++ b/arch/blackfin/mach-bf537/boards/stamp.c
> > @@ -2221,7 +2221,7 @@ static struct i2c_board_info __initdata bfin_i2c_board_info[] = {
> > },
> > #endif
> >
> > -#if defined(CONFIG_ADT75) || defined(CONFIG_ADT75_MODULE)
> > +#if IS_ENABLED(CONFIG_SENSORS_LM75)
> > {
> > I2C_BOARD_INFO("adt75", 0x9),
>
> Unrelated to this patch, but this is very odd, as the ADT75 can't have
> slave address 0x09. This address is almost exclusively used by battery
> chargers AFAIK.
>
> > .irq = IRQ_PG5,
>
> The patch itself looks good.
>
> Reviewed-by: Jean Delvare <[email protected]>

After which nothing seems to have happened. Can someone please have a
look at this patch?


Paul Bolle