| Message ID | 20170521204233.6745-2-wsa@the-dreams.de |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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!
> > 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? The bh1770 seems to be used on the N950: http://elinux.org/N950 I couldn't find a user of the apds990x.
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
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%)