* [Question] orphan platform data header @ 2019-07-20 3:25 Masahiro Yamada 2019-07-20 13:54 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Masahiro Yamada @ 2019-07-20 3:25 UTC (permalink / raw) To: Linux Kernel Mailing List Cc: Arnd Bergmann, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel, masahiroy Hi. I see several platform-data headers that are not used in upstream. For instance, please look at this driver: drivers/leds/leds-netxbig.c If I understood it correctly, this driver supports both device tree and legacy board-file. I grepped 'netxbig_led_platform_data', but I only found the driver and platform_data header. No board-file in upstream. masahiro@grover:~/ref/linux$ git grep netxbig_led_platform_data drivers/leds/leds-netxbig.c: struct netxbig_led_platform_data *pdata, drivers/leds/leds-netxbig.c: struct netxbig_led_platform_data *pdata) drivers/leds/leds-netxbig.c: struct netxbig_led_platform_data *pdata) drivers/leds/leds-netxbig.c: struct netxbig_led_platform_data *pdata = dev_get_platdata(&pdev->dev); include/linux/platform_data/leds-kirkwood-netxbig.h:struct netxbig_led_platform_data { So, what shall we do? Drop the board-file support? Or, keep it in case somebody is still using their board-files in downstream? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-20 3:25 [Question] orphan platform data header Masahiro Yamada @ 2019-07-20 13:54 ` Arnd Bergmann 2019-07-21 3:44 ` Masahiro Yamada 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2019-07-20 13:54 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel, masahiroy On Sat, Jul 20, 2019 at 5:26 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > masahiro@grover:~/ref/linux$ git grep netxbig_led_platform_data > drivers/leds/leds-netxbig.c: struct > netxbig_led_platform_data *pdata, > drivers/leds/leds-netxbig.c: struct > netxbig_led_platform_data *pdata) > drivers/leds/leds-netxbig.c: struct > netxbig_led_platform_data *pdata) > drivers/leds/leds-netxbig.c: struct netxbig_led_platform_data > *pdata = dev_get_platdata(&pdev->dev); > include/linux/platform_data/leds-kirkwood-netxbig.h:struct > netxbig_led_platform_data { > > > > So, what shall we do? > > Drop the board-file support? Or, keep it > in case somebody is still using their board-files > in downstream? Generally speaking, I'd remove the board file support in another case like this, but it's worth looking at when it was last used and by what. For this file, all boards got converted to DT, and the old setup code removed in commit ebc278f15759 ("ARM: mvebu: remove static LED setup for netxbig boards"), four years ago, so it's a fairly easy decision to make it DT only. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-20 13:54 ` Arnd Bergmann @ 2019-07-21 3:44 ` Masahiro Yamada 2019-07-21 9:09 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Masahiro Yamada @ 2019-07-21 3:44 UTC (permalink / raw) To: Arnd Bergmann Cc: Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel Hi Arnd, On Sat, Jul 20, 2019 at 10:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sat, Jul 20, 2019 at 5:26 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > masahiro@grover:~/ref/linux$ git grep netxbig_led_platform_data > > drivers/leds/leds-netxbig.c: struct > > netxbig_led_platform_data *pdata, > > drivers/leds/leds-netxbig.c: struct > > netxbig_led_platform_data *pdata) > > drivers/leds/leds-netxbig.c: struct > > netxbig_led_platform_data *pdata) > > drivers/leds/leds-netxbig.c: struct netxbig_led_platform_data > > *pdata = dev_get_platdata(&pdev->dev); > > include/linux/platform_data/leds-kirkwood-netxbig.h:struct > > netxbig_led_platform_data { > > > > > > > > So, what shall we do? > > > > Drop the board-file support? Or, keep it > > in case somebody is still using their board-files > > in downstream? > > Generally speaking, I'd remove the board file support in another > case like this, but it's worth looking at when it was last used and by > what. > > For this file, all boards got converted to DT, and the old setup > code removed in commit ebc278f15759 ("ARM: mvebu: remove static > LED setup for netxbig boards"), four years ago, so it's a fairly > easy decision to make it DT only. Thanks. I see another case, which is difficult to make a decision. For example, drivers/spi/spi-tle62x0.c This driver supports only board-file, but the board-file is not found in upstream. Unless I am terribly missing something, there is no one who passes tle62x0_pdata to this driver. $ git grep tle62x0_pdata drivers/spi/spi-tle62x0.c: struct tle62x0_pdata *pdata; include/linux/spi/tle62x0.h:struct tle62x0_pdata { But, removing board-file support makes this driver completely useless... -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-21 3:44 ` Masahiro Yamada @ 2019-07-21 9:09 ` Arnd Bergmann 2019-07-21 12:12 ` Masahiro Yamada 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2019-07-21 9:09 UTC (permalink / raw) To: Masahiro Yamada, Ben Dooks Cc: Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel On Sun, Jul 21, 2019 at 5:45 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > On Sat, Jul 20, 2019 at 10:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sat, Jul 20, 2019 at 5:26 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > > So, what shall we do? > > > > > > Drop the board-file support? Or, keep it > > > in case somebody is still using their board-files > > > in downstream? >> > > For this file, all boards got converted to DT, and the old setup > > code removed in commit ebc278f15759 ("ARM: mvebu: remove static > > LED setup for netxbig boards"), four years ago, so it's a fairly > > easy decision to make it DT only. > > I see another case, which is difficult > to make a decision. > > For example, drivers/spi/spi-tle62x0.c > > This driver supports only board-file, but the board-file > is not found in upstream. > > Unless I am terribly missing something, > there is no one who passes tle62x0_pdata > to this driver. > > $ git grep tle62x0_pdata > drivers/spi/spi-tle62x0.c: struct tle62x0_pdata *pdata; > include/linux/spi/tle62x0.h:struct tle62x0_pdata { > > But, removing board-file support > makes this driver completely useless... Adding Ben Dooks to Cc. I suspect this driver is completely obsolete and should be removed. For some reason, it's not an SPI controller driver like all the other files in that directory, but implements low-level access to the state of a particular SPI device. However, there should not really be a low-level driver for it that just exports the pins to user space. It should either be a gpiolib driver to let other drivers talk to the pins, or a high-level driver that exposes the intended functionality (watchdog, regulator, ...) to those respective subsystems. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-21 9:09 ` Arnd Bergmann @ 2019-07-21 12:12 ` Masahiro Yamada 2019-07-21 14:15 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Masahiro Yamada @ 2019-07-21 12:12 UTC (permalink / raw) To: Arnd Bergmann Cc: Ben Dooks, Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel On Sun, Jul 21, 2019 at 6:10 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sun, Jul 21, 2019 at 5:45 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > On Sat, Jul 20, 2019 at 10:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Sat, Jul 20, 2019 at 5:26 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > > > So, what shall we do? > > > > > > > > Drop the board-file support? Or, keep it > > > > in case somebody is still using their board-files > > > > in downstream? > >> > > > For this file, all boards got converted to DT, and the old setup > > > code removed in commit ebc278f15759 ("ARM: mvebu: remove static > > > LED setup for netxbig boards"), four years ago, so it's a fairly > > > easy decision to make it DT only. > > > > I see another case, which is difficult > > to make a decision. > > > > For example, drivers/spi/spi-tle62x0.c > > > > This driver supports only board-file, but the board-file > > is not found in upstream. > > > > Unless I am terribly missing something, > > there is no one who passes tle62x0_pdata > > to this driver. > > > > $ git grep tle62x0_pdata > > drivers/spi/spi-tle62x0.c: struct tle62x0_pdata *pdata; > > include/linux/spi/tle62x0.h:struct tle62x0_pdata { > > > > But, removing board-file support > > makes this driver completely useless... > > Adding Ben Dooks to Cc. > > I suspect this driver is completely obsolete and should be removed. > > For some reason, it's not an SPI controller driver like all the other > files in that directory, but implements low-level access to the state > of a particular SPI device. > > However, there should not really be a low-level driver for it that > just exports the pins to user space. It should either be a gpiolib > driver to let other drivers talk to the pins, or a high-level driver that > exposes the intended functionality (watchdog, regulator, ...) > to those respective subsystems. > > Arnd Another example that I have no idea how it works: drivers/net/hamradio/yam.c yam_ioctl() reads data from user-space, but the data structures for ioctl are defined in include/linux/yam.h This header is not exported to user-space since it is outside of the uapi directory. I dug the git history, but it has never exported to user-space in the past. I do not know how user-space programs can pass-in data to the kernel. If we want to fix this, we could move it to include/uapi/linux/yam.h But, if nobody has reported any problem about this, it might be a good proof that nobody is using this driver. Maybe, can we simply drop odd drivers?? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-21 12:12 ` Masahiro Yamada @ 2019-07-21 14:15 ` Arnd Bergmann 2019-07-21 14:58 ` Masahiro Yamada 2019-07-22 6:46 ` Enrico Weigelt, metux IT consult 0 siblings, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2019-07-21 14:15 UTC (permalink / raw) To: Masahiro Yamada Cc: Ben Dooks, Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel On Sun, Jul 21, 2019 at 2:13 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > On Sun, Jul 21, 2019 at 6:10 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sun, Jul 21, 2019 at 5:45 AM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > On Sat, Jul 20, 2019 at 10:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Sat, Jul 20, 2019 at 5:26 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > > Another example that I have no idea > how it works: > > drivers/net/hamradio/yam.c > > yam_ioctl() reads data from user-space, > but the data structures for ioctl are > defined in include/linux/yam.h That is different: the hardware attaches to a serial port and may well be usable, and the user space side just contains a copy of the header, see https://github.com/nwdigitalradio/ax25-tools/tree/master/yamdrv > If we want to fix this, we could move it > to include/uapi/linux/yam.h We could do that, or just leave it alone, as nobody would tell the difference. > But, if nobody has reported any problem about this, > it might be a good proof that nobody is using this driver. > > Maybe, can we simply drop odd drivers?? Both the kernel driver and the user space side have a maintainer, and I see no indication that it is actually broken. The driver is clearly a relic from old times (earlier than 2.4) and we would not merge this driver today. It seems more useful to keep looking for drivers with a platform_data header file that is no longer included by any platform for candidates that may be obsolete. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-21 14:15 ` Arnd Bergmann @ 2019-07-21 14:58 ` Masahiro Yamada 2019-07-22 6:46 ` Enrico Weigelt, metux IT consult 1 sibling, 0 replies; 9+ messages in thread From: Masahiro Yamada @ 2019-07-21 14:58 UTC (permalink / raw) To: Arnd Bergmann Cc: Ben Dooks, Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel On Sun, Jul 21, 2019 at 11:15 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sun, Jul 21, 2019 at 2:13 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > On Sun, Jul 21, 2019 at 6:10 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Sun, Jul 21, 2019 at 5:45 AM Masahiro Yamada > > > <yamada.masahiro@socionext.com> wrote: > > > > On Sat, Jul 20, 2019 at 10:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > On Sat, Jul 20, 2019 at 5:26 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > > > > > Another example that I have no idea > > how it works: > > > > drivers/net/hamradio/yam.c > > > > yam_ioctl() reads data from user-space, > > but the data structures for ioctl are > > defined in include/linux/yam.h > > That is different: the hardware attaches to a serial port and may well > be usable, and the user space side just contains a copy of the header, > see https://github.com/nwdigitalradio/ax25-tools/tree/master/yamdrv Oh, I did not know that user-space had a copy of that. > > If we want to fix this, we could move it > > to include/uapi/linux/yam.h > > We could do that, or just leave it alone, as nobody would > tell the difference. When we are changing structures in uapi, it is very likely a red alert. On the other hand, we can change code outside of uapi more safely. One benefit of uapi is we can catch the compatibility level from the directory path. > > > But, if nobody has reported any problem about this, > > it might be a good proof that nobody is using this driver. > > > > Maybe, can we simply drop odd drivers?? > > Both the kernel driver and the user space side have a maintainer, and > I see no indication that it is actually broken. The driver is clearly > a relic from old times (earlier than 2.4) and we would not merge > this driver today. > > It seems more useful to keep looking for drivers with a platform_data > header file that is no longer included by any platform for candidates > that may be obsolete. OK. Thanks. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-21 14:15 ` Arnd Bergmann 2019-07-21 14:58 ` Masahiro Yamada @ 2019-07-22 6:46 ` Enrico Weigelt, metux IT consult 2019-07-22 7:24 ` Arnd Bergmann 1 sibling, 1 reply; 9+ messages in thread From: Enrico Weigelt, metux IT consult @ 2019-07-22 6:46 UTC (permalink / raw) To: Arnd Bergmann, Masahiro Yamada Cc: Ben Dooks, Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel On 21.07.19 16:15, Arnd Bergmann wrote: > That is different: the hardware attaches to a serial port and may well > be usable, and the user space side just contains a copy of the header, > see https://github.com/nwdigitalradio/ax25-tools/tree/master/yamdrv I believe that such header copies in userland applications are conceptionally wrong. Whenever something changes, both sides need to be kept in sync. Maybe we should talk to the hamradio folks to get this cleaned up. IMHO, this header should go to uapi. > It seems more useful to keep looking for drivers with a platform_data > header file that is no longer included by any platform for candidates > that may be obsolete. Some folks see platform_data old legacy that should be removed, but I don't aggree. For example w/ apu2 board driver (and corresponding amd-fch-gpio driver) I even had to introduce a pdata struct, so the board driver could configure the gpio driver. Certainly, I would have preferred doing everything via DT, but that's not available on x86/acpi targets (if anybody knows a way to inject a DT snippet just for one driver in such a scenario, please let me know). --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] orphan platform data header 2019-07-22 6:46 ` Enrico Weigelt, metux IT consult @ 2019-07-22 7:24 ` Arnd Bergmann 0 siblings, 0 replies; 9+ messages in thread From: Arnd Bergmann @ 2019-07-22 7:24 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult Cc: Masahiro Yamada, Ben Dooks, Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman, DTML, linux-arm-kernel On Mon, Jul 22, 2019 at 8:46 AM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 21.07.19 16:15, Arnd Bergmann wrote: > > > That is different: the hardware attaches to a serial port and may well > > be usable, and the user space side just contains a copy of the header, > > see https://github.com/nwdigitalradio/ax25-tools/tree/master/yamdrv > > I believe that such header copies in userland applications are > conceptionally wrong. Whenever something changes, both sides need > to be kept in sync. > > Maybe we should talk to the hamradio folks to get this cleaned up. > IMHO, this header should go to uapi. Having copies of driver specific uapi headers is rather common, and you won't have much success trying to get everyone to change that. The reasons why those applications do it are: - The kernel already gives ABI guarantees so anything built with an old header file is expected to keep working indefinitely. - Using a new header file won't help unless the application also knows about the added interfaces - If an application uses more recent additions to the kernel headers, it either has to have a matching version of that header, or use a long series of #ifdef checks to deal with arbitrary versions. > > It seems more useful to keep looking for drivers with a platform_data > > header file that is no longer included by any platform for candidates > > that may be obsolete. > > Some folks see platform_data old legacy that should be removed, but I > don't aggree. For example w/ apu2 board driver (and corresponding > amd-fch-gpio driver) I even had to introduce a pdata struct, so the > board driver could configure the gpio driver. The case that we are interested in is drivers that previously used platform data in legacy board files that have since been replaced with dtb based boots. > Certainly, I would have > preferred doing everything via DT, but that's not available on x86/acpi > targets (if anybody knows a way to inject a DT snippet just for one > driver in such a scenario, please let me know). It's been done before, but usually with overlays that don't necessarily make it any better than platform data. If you have a set of drivers where one of them creates a platform device for the other driver, then platform data is usually the easiest way, and I'm not aware of any move to get rid of that. As an alternative, you can use the generalized property support from include/linux/property.h that works on top of DT, ACPI or plain platform devices. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-07-22 7:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-20 3:25 [Question] orphan platform data header Masahiro Yamada 2019-07-20 13:54 ` Arnd Bergmann 2019-07-21 3:44 ` Masahiro Yamada 2019-07-21 9:09 ` Arnd Bergmann 2019-07-21 12:12 ` Masahiro Yamada 2019-07-21 14:15 ` Arnd Bergmann 2019-07-21 14:58 ` Masahiro Yamada 2019-07-22 6:46 ` Enrico Weigelt, metux IT consult 2019-07-22 7:24 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).