diff mbox

Revert "Input: bma150 - avoid binding to bma180 if IIO bma180 driver present" and "Input: bma150 - extend chip detection for bma180"

Message ID 631685e1205d8b1b45d61e57070310d89d535b74.1491057884.git.hns@goldelico.com (mailing list archive)
State Accepted
Headers show

Commit Message

H. Nikolaus Schaller April 1, 2017, 2:44 p.m. UTC
This reverts commit baf28d91e7b1 ("Input: bma150 - avoid binding to bma180 if IIO bma180 driver present")
and commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180")

Rationale: initially (2012) the GTA04 device using a bma180 chip simply refereced
the bma150 platform driver in its board file [1] which happened to work in all
scenarios that were tested.

When conversion to DT started (2014), we needed to make the driver be still
recognised. Hence we introduced the compatibility to the bma180 chip in
Linux 3.15-rc5 [2] without further checks if it is really 100% compatible. This
worked flawlessly for years with the GTA04 device.

Recently (2016), Hans de Goede pointed out that the chips are not as similar
as they appeared and the driver works with the bma180 for the GTA04 only
by good luck. He proposed to remove the bma180 support completely [3], but
we still did need it until we have a replacement. Thus, a condifional compile
was added.

We have now developed a generic iio-input-bridge which works with any 2 or
3 axis iio based accelerometer. It has been tested on GTA04 and Pyra and
works as expected. Therefore we can remove the bma180 support from this
driver completely. User-space API compatibility can be restored by using the
iio-input-bridge.

Maybe it is time to convert the bma150 driver to iio as well and retire the
accelerometer input drivers completely but this is a different story and task.

[1]: https://github.com/neilbrown/linux/blob/gta04/3.2.y/arch/arm/mach-omap2/board-omap3gta04.c#L976
[2]: https://patchwork.kernel.org/patch/3961171/
[3]: https://patchwork.kernel.org/patch/9325481/

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/input/misc/bma150.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

Comments

Hans de Goede April 1, 2017, 2:59 p.m. UTC | #1
Hi,

On 01-04-17 16:44, H. Nikolaus Schaller wrote:
> This reverts commit baf28d91e7b1 ("Input: bma150 - avoid binding to bma180 if IIO bma180 driver present")
> and commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180")
>
> Rationale: initially (2012) the GTA04 device using a bma180 chip simply refereced
> the bma150 platform driver in its board file [1] which happened to work in all
> scenarios that were tested.
>
> When conversion to DT started (2014), we needed to make the driver be still
> recognised. Hence we introduced the compatibility to the bma180 chip in
> Linux 3.15-rc5 [2] without further checks if it is really 100% compatible. This
> worked flawlessly for years with the GTA04 device.
>
> Recently (2016), Hans de Goede pointed out that the chips are not as similar
> as they appeared and the driver works with the bma180 for the GTA04 only
> by good luck. He proposed to remove the bma180 support completely [3], but
> we still did need it until we have a replacement. Thus, a condifional compile
> was added.
>
> We have now developed a generic iio-input-bridge which works with any 2 or
> 3 axis iio based accelerometer. It has been tested on GTA04 and Pyra and
> works as expected. Therefore we can remove the bma180 support from this
> driver completely. User-space API compatibility can be restored by using the
> iio-input-bridge.
>
> Maybe it is time to convert the bma150 driver to iio as well and retire the
> accelerometer input drivers completely but this is a different story and task.
>
> [1]: https://github.com/neilbrown/linux/blob/gta04/3.2.y/arch/arm/mach-omap2/board-omap3gta04.c#L976
> [2]: https://patchwork.kernel.org/patch/3961171/
> [3]: https://patchwork.kernel.org/patch/9325481/
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/input/misc/bma150.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
> index 1fa8537..1efcfdf 100644
> --- a/drivers/input/misc/bma150.c
> +++ b/drivers/input/misc/bma150.c
> @@ -70,7 +70,6 @@
>  #define BMA150_CFG_5_REG	0x11
>
>  #define BMA150_CHIP_ID		2
> -#define BMA180_CHIP_ID		3
>  #define BMA150_CHIP_ID_REG	BMA150_DATA_0_REG
>
>  #define BMA150_ACC_X_LSB_REG	BMA150_DATA_2_REG
> @@ -538,13 +537,8 @@ static int bma150_probe(struct i2c_client *client,
>  		return -EIO;
>  	}
>
> -	/*
> -	 * Note if the IIO CONFIG_BMA180 driver is enabled we want to fail
> -	 * the probe for the bma180 as the iio driver is preferred.
> -	 */
>  	chip_id = i2c_smbus_read_byte_data(client, BMA150_CHIP_ID_REG);
> -	if (chip_id != BMA150_CHIP_ID &&
> -	    (IS_ENABLED(CONFIG_BMA180) || chip_id != BMA180_CHIP_ID)) {
> +	if (chip_id != BMA150_CHIP_ID) {
>  		dev_err(&client->dev, "BMA150 chip id error: %d\n", chip_id);
>  		return -EINVAL;
>  	}
> @@ -648,9 +642,6 @@ static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
>
>  static const struct i2c_device_id bma150_id[] = {
>  	{ "bma150", 0 },
> -#if !IS_ENABLED(CONFIG_BMA180)
> -	{ "bma180", 0 },
> -#endif
>  	{ "smb380", 0 },
>  	{ "bma023", 0 },
>  	{ }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov April 1, 2017, 4:46 p.m. UTC | #2
On Sat, Apr 01, 2017 at 04:59:26PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 01-04-17 16:44, H. Nikolaus Schaller wrote:
> >This reverts commit baf28d91e7b1 ("Input: bma150 - avoid binding to bma180 if IIO bma180 driver present")
> >and commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180")
> >
> >Rationale: initially (2012) the GTA04 device using a bma180 chip simply refereced
> >the bma150 platform driver in its board file [1] which happened to work in all
> >scenarios that were tested.
> >
> >When conversion to DT started (2014), we needed to make the driver be still
> >recognised. Hence we introduced the compatibility to the bma180 chip in
> >Linux 3.15-rc5 [2] without further checks if it is really 100% compatible. This
> >worked flawlessly for years with the GTA04 device.
> >
> >Recently (2016), Hans de Goede pointed out that the chips are not as similar
> >as they appeared and the driver works with the bma180 for the GTA04 only
> >by good luck. He proposed to remove the bma180 support completely [3], but
> >we still did need it until we have a replacement. Thus, a condifional compile
> >was added.
> >
> >We have now developed a generic iio-input-bridge which works with any 2 or
> >3 axis iio based accelerometer. It has been tested on GTA04 and Pyra and
> >works as expected. Therefore we can remove the bma180 support from this
> >driver completely. User-space API compatibility can be restored by using the
> >iio-input-bridge.
> >
> >Maybe it is time to convert the bma150 driver to iio as well and retire the
> >accelerometer input drivers completely but this is a different story and task.
> >
> >[1]: https://github.com/neilbrown/linux/blob/gta04/3.2.y/arch/arm/mach-omap2/board-omap3gta04.c#L976
> >[2]: https://patchwork.kernel.org/patch/3961171/
> >[3]: https://patchwork.kernel.org/patch/9325481/
> >
> >Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> 
> Looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Changed subject slightly (we are not reverting because patches were
defective, we are removing support for bma180), and applied.

Thanks.
H. Nikolaus Schaller April 5, 2017, 7:16 p.m. UTC | #3
> Am 01.04.2017 um 18:46 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> 
> On Sat, Apr 01, 2017 at 04:59:26PM +0200, Hans de Goede wrote:
>> Hi,
>> 
>> On 01-04-17 16:44, H. Nikolaus Schaller wrote:
>>> This reverts commit baf28d91e7b1 ("Input: bma150 - avoid binding to bma180 if IIO bma180 driver present")
>>> and commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180")
>>> 
>>> Rationale: initially (2012) the GTA04 device using a bma180 chip simply refereced
>>> the bma150 platform driver in its board file [1] which happened to work in all
>>> scenarios that were tested.
>>> 
>>> When conversion to DT started (2014), we needed to make the driver be still
>>> recognised. Hence we introduced the compatibility to the bma180 chip in
>>> Linux 3.15-rc5 [2] without further checks if it is really 100% compatible. This
>>> worked flawlessly for years with the GTA04 device.
>>> 
>>> Recently (2016), Hans de Goede pointed out that the chips are not as similar
>>> as they appeared and the driver works with the bma180 for the GTA04 only
>>> by good luck. He proposed to remove the bma180 support completely [3], but
>>> we still did need it until we have a replacement. Thus, a condifional compile
>>> was added.
>>> 
>>> We have now developed a generic iio-input-bridge which works with any 2 or
>>> 3 axis iio based accelerometer. It has been tested on GTA04 and Pyra and
>>> works as expected. Therefore we can remove the bma180 support from this
>>> driver completely. User-space API compatibility can be restored by using the
>>> iio-input-bridge.
>>> 
>>> Maybe it is time to convert the bma150 driver to iio as well and retire the
>>> accelerometer input drivers completely but this is a different story and task.
>>> 
>>> [1]: https://github.com/neilbrown/linux/blob/gta04/3.2.y/arch/arm/mach-omap2/board-omap3gta04.c#L976
>>> [2]: https://patchwork.kernel.org/patch/3961171/
>>> [3]: https://patchwork.kernel.org/patch/9325481/
>>> 
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> 
>> Looks good to me:
>> 
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Changed subject slightly (we are not reverting because patches were
> defective, we are removing support for bma180), and applied.

Just noticed: the subject in linux-next has a typo:
Should IMHO be "Input: bma150 - remove support for bma180"

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=f069b5a0b27ad4a87e9351e54fbcab3d3f8a9fd5

thanks,
Nikolaus--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov April 5, 2017, 9:37 p.m. UTC | #4
On Wed, Apr 05, 2017 at 09:16:28PM +0200, H. Nikolaus Schaller wrote:
> 
> > Am 01.04.2017 um 18:46 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > 
> > On Sat, Apr 01, 2017 at 04:59:26PM +0200, Hans de Goede wrote:
> >> Hi,
> >> 
> >> On 01-04-17 16:44, H. Nikolaus Schaller wrote:
> >>> This reverts commit baf28d91e7b1 ("Input: bma150 - avoid binding to bma180 if IIO bma180 driver present")
> >>> and commit ef3714fdbc8d ("Input: bma150 - extend chip detection for bma180")
> >>> 
> >>> Rationale: initially (2012) the GTA04 device using a bma180 chip simply refereced
> >>> the bma150 platform driver in its board file [1] which happened to work in all
> >>> scenarios that were tested.
> >>> 
> >>> When conversion to DT started (2014), we needed to make the driver be still
> >>> recognised. Hence we introduced the compatibility to the bma180 chip in
> >>> Linux 3.15-rc5 [2] without further checks if it is really 100% compatible. This
> >>> worked flawlessly for years with the GTA04 device.
> >>> 
> >>> Recently (2016), Hans de Goede pointed out that the chips are not as similar
> >>> as they appeared and the driver works with the bma180 for the GTA04 only
> >>> by good luck. He proposed to remove the bma180 support completely [3], but
> >>> we still did need it until we have a replacement. Thus, a condifional compile
> >>> was added.
> >>> 
> >>> We have now developed a generic iio-input-bridge which works with any 2 or
> >>> 3 axis iio based accelerometer. It has been tested on GTA04 and Pyra and
> >>> works as expected. Therefore we can remove the bma180 support from this
> >>> driver completely. User-space API compatibility can be restored by using the
> >>> iio-input-bridge.
> >>> 
> >>> Maybe it is time to convert the bma150 driver to iio as well and retire the
> >>> accelerometer input drivers completely but this is a different story and task.
> >>> 
> >>> [1]: https://github.com/neilbrown/linux/blob/gta04/3.2.y/arch/arm/mach-omap2/board-omap3gta04.c#L976
> >>> [2]: https://patchwork.kernel.org/patch/3961171/
> >>> [3]: https://patchwork.kernel.org/patch/9325481/
> >>> 
> >>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> >> 
> >> Looks good to me:
> >> 
> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Changed subject slightly (we are not reverting because patches were
> > defective, we are removing support for bma180), and applied.
> 
> Just noticed: the subject in linux-next has a typo:
> Should IMHO be "Input: bma150 - remove support for bma180"
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=f069b5a0b27ad4a87e9351e54fbcab3d3f8a9fd5

Yeah, I screwed up there. Unfortunately it is buries too deep now.

Thanks.
diff mbox

Patch

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index 1fa8537..1efcfdf 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -70,7 +70,6 @@ 
 #define BMA150_CFG_5_REG	0x11
 
 #define BMA150_CHIP_ID		2
-#define BMA180_CHIP_ID		3
 #define BMA150_CHIP_ID_REG	BMA150_DATA_0_REG
 
 #define BMA150_ACC_X_LSB_REG	BMA150_DATA_2_REG
@@ -538,13 +537,8 @@  static int bma150_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
-	/*
-	 * Note if the IIO CONFIG_BMA180 driver is enabled we want to fail
-	 * the probe for the bma180 as the iio driver is preferred.
-	 */
 	chip_id = i2c_smbus_read_byte_data(client, BMA150_CHIP_ID_REG);
-	if (chip_id != BMA150_CHIP_ID &&
-	    (IS_ENABLED(CONFIG_BMA180) || chip_id != BMA180_CHIP_ID)) {
+	if (chip_id != BMA150_CHIP_ID) {
 		dev_err(&client->dev, "BMA150 chip id error: %d\n", chip_id);
 		return -EINVAL;
 	}
@@ -648,9 +642,6 @@  static UNIVERSAL_DEV_PM_OPS(bma150_pm, bma150_suspend, bma150_resume, NULL);
 
 static const struct i2c_device_id bma150_id[] = {
 	{ "bma150", 0 },
-#if !IS_ENABLED(CONFIG_BMA180)
-	{ "bma180", 0 },
-#endif
 	{ "smb380", 0 },
 	{ "bma023", 0 },
 	{ }