Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1646441 > unrolled thread

[PATCH 1/2] misc: apds990x: move header file out of I2C realm

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] misc: apds990x: move header file out of I2C realm Wolfram Sang <wsa@the-dreams.de> - 2017-05-21 22:50 +0200
    Re: [PATCH 1/2] misc: apds990x: move header file out of I2C realm Arnd Bergmann <arnd@arndb.de> - 2017-05-22 16:30 +0200
      Re: [PATCH 1/2] misc: apds990x: move header file out of I2C realm Wolfram Sang <wsa@the-dreams.de> - 2017-05-22 18:00 +0200
        Re: [PATCH 1/2] misc: apds990x: move header file out of I2C realm Arnd Bergmann <arnd@arndb.de> - 2017-05-23 00:00 +0200
          Re: [PATCH 1/2] misc: apds990x: move header file out of I2C realm Linus Walleij <linus.walleij@linaro.org> - 2017-05-23 00:40 +0200
          Re: [PATCH 1/2] misc: apds990x: move header file out of I2C realm Wolfram Sang <wsa@the-dreams.de> - 2017-05-23 08:30 +0200

#1646441 — [PATCH 1/2] misc: apds990x: move header file out of I2C realm

include/linux/i2c is not for client devices. Move the header file to a
more appropriate location.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 drivers/misc/apds990x.c                         | 2 +-
 include/linux/{i2c => platform_data}/apds990x.h | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename include/linux/{i2c => platform_data}/apds990x.h (100%)

diff --git a/drivers/misc/apds990x.c b/drivers/misc/apds990x.c
index dfb72ecfa60461..c341164edaad01 100644
--- a/drivers/misc/apds990x.c
+++ b/drivers/misc/apds990x.c
@@ -32,7 +32,7 @@
 #include <linux/delay.h>
 #include <linux/wait.h>
 #include <linux/slab.h>
-#include <linux/i2c/apds990x.h>
+#include <linux/platform_data/apds990x.h>
 
 /* Register map */
 #define APDS990X_ENABLE	 0x00 /* Enable of states and interrupts */
diff --git a/include/linux/i2c/apds990x.h b/include/linux/platform_data/apds990x.h
similarity index 100%
rename from include/linux/i2c/apds990x.h
rename to include/linux/platform_data/apds990x.h
-- 
2.11.0

[toc] | [next] | [standalone]

#1646975

On Sun, May 21, 2017 at 10:42 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> include/linux/i2c is not for client devices. Move the header file to a
> more appropriate location.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
>  drivers/misc/apds990x.c                         | 2 +-
>  include/linux/{i2c => platform_data}/apds990x.h | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename include/linux/{i2c => platform_data}/apds990x.h (100%)
>
> diff --git a/drivers/misc/apds990x.c b/drivers/misc/apds990x.c
> index dfb72ecfa60461..c341164edaad01 100644
> --- a/drivers/misc/apds990x.c
> +++ b/drivers/misc/apds990x.c
> @@ -32,7 +32,7 @@
>  #include <linux/delay.h>
>  #include <linux/wait.h>
>  #include <linux/slab.h>
> -#include <linux/i2c/apds990x.h>
> +#include <linux/platform_data/apds990x.h>

The new location is clearly better than the old, but I notice that in both
patches, there is not a single definition for the platform_data structure
in the kernel and both drivers refuse to load when they do not get
passed valid platform_data.

Should we also remove the drivers or move them into staging?

       Arnd

[toc] | [prev] | [next] | [standalone]

#1647065

[Multipart message — attachments visible in raw view] — view raw

Hi Arnd,

> > -#include <linux/i2c/apds990x.h>
> > +#include <linux/platform_data/apds990x.h>
> 
> The new location is clearly better than the old, but I notice that in both
> patches, there is not a single definition for the platform_data structure
> in the kernel and both drivers refuse to load when they do not get
> passed valid platform_data.

Yes, this is true for quite some drivers I am moving around. I think
there are two reasons: a) the board code never made it upstream
b) DT conversion happened and platform_data is now cruft. As mentioned
in the cover-letter, I didn't dive deeper for all the drivers.

> Should we also remove the drivers or move them into staging?

I'd prefer to keep them. If they are needed again, DT conversion is
likely and easier than restarting from scratch. Guenter Roeck also
prefers to not unnecessarily annoy people who might have out-of-tree
board code. So, as long as they are not painful, let's keep them?

Regards,

   Wolfram

[toc] | [prev] | [next] | [standalone]

#1647431

On Mon, May 22, 2017 at 5:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> Should we also remove the drivers or move them into staging?
>
> I'd prefer to keep them. If they are needed again, DT conversion is
> likely and easier than restarting from scratch. Guenter Roeck also
> prefers to not unnecessarily annoy people who might have out-of-tree
> board code. So, as long as they are not painful, let's keep them?

The one reason I can think of for removing them is that we don't
want ambient light sensor drivers in drivers/misc any more and
instead of adding DT probing code would also expect new users to
migrate to drivers/iio/light/, which already has drivers for
apds9300 and apds9960 but not apds990x, as well as bh1750 and
bh1780 but not bh1770.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1647451

On Mon, May 22, 2017 at 11:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, May 22, 2017 at 5:56 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>>> Should we also remove the drivers or move them into staging?
>>
>> I'd prefer to keep them. If they are needed again, DT conversion is
>> likely and easier than restarting from scratch. Guenter Roeck also
>> prefers to not unnecessarily annoy people who might have out-of-tree
>> board code. So, as long as they are not painful, let's keep them?
>
> The one reason I can think of for removing them is that we don't
> want ambient light sensor drivers in drivers/misc any more and
> instead of adding DT probing code would also expect new users to
> migrate to drivers/iio/light/, which already has drivers for
> apds9300 and apds9960 but not apds990x, as well as bh1750 and
> bh1780 but not bh1770.

This (apds990x) and bh1770 were added by Samu Onkalo
for Nokia's upstreaming efforts.

Samu, what are the hardware targets using this? Something that
has a userspace etc that we can test?

Like Nokia 900 or so?

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1647669

[Multipart message — attachments visible in raw view] — view raw

> The one reason I can think of for removing them is that we don't
> want ambient light sensor drivers in drivers/misc any more and
> instead of adding DT probing code would also expect new users to
> migrate to drivers/iio/light/, which already has drivers for
> apds9300 and apds9960 but not apds990x, as well as bh1750 and
> bh1780 but not bh1770.

Yes, I totally agree!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web