| Message ID | 20200804192654.12783-14-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk/watchdog/ARM: Cleanup of various S3C bits | expand |
Quoting Krzysztof Kozlowski (2020-08-04 12:26:54) > The s3c_camif_gpio_put() declaration in > include/media/drv-intf/s3c_camif.h header was different than definition. > Fixing this allows to include that header to also fix the W=1 compile > warnings: > > arch/arm/mach-s3c24xx/setup-camif.c:28:5: warning: no previous prototype for 's3c_camif_gpio_get' [-Wmissing-prototypes] > 28 | int s3c_camif_gpio_get(void) > arch/arm/mach-s3c24xx/setup-camif.c:56:6: warning: no previous prototype for 's3c_camif_gpio_put' [-Wmissing-prototypes] > 56 | void s3c_camif_gpio_put(void) Maybe it should have been void all along?
On Wed, Aug 12, 2020 at 9:59 AM Stephen Boyd <sboyd@kernel.org> wrote: > Quoting Krzysztof Kozlowski (2020-08-04 12:26:54) > > The s3c_camif_gpio_put() declaration in > > include/media/drv-intf/s3c_camif.h header was different than definition. > > Fixing this allows to include that header to also fix the W=1 compile > > warnings: > > > > arch/arm/mach-s3c24xx/setup-camif.c:28:5: warning: no previous prototype for 's3c_camif_gpio_get' [-Wmissing-prototypes] > > 28 | int s3c_camif_gpio_get(void) > > arch/arm/mach-s3c24xx/setup-camif.c:56:6: warning: no previous prototype for 's3c_camif_gpio_put' [-Wmissing-prototypes] > > 56 | void s3c_camif_gpio_put(void) > > Maybe it should have been void all along? It seems there have never been any callers and the entire file can just be removed, with the rest of that platform_data header file moved to drivers/media/platform/s3c-camif/. Arnd
On 12.08.2020 11:14, Arnd Bergmann wrote: > On Wed, Aug 12, 2020 at 9:59 AM Stephen Boyd <sboyd@kernel.org> wrote: >> Quoting Krzysztof Kozlowski (2020-08-04 12:26:54) >>> The s3c_camif_gpio_put() declaration in >>> include/media/drv-intf/s3c_camif.h header was different than definition. >>> Fixing this allows to include that header to also fix the W=1 compile >>> warnings: >>> >>> arch/arm/mach-s3c24xx/setup-camif.c:28:5: warning: no previous prototype for 's3c_camif_gpio_get' [-Wmissing-prototypes] >>> 28 | int s3c_camif_gpio_get(void) >>> arch/arm/mach-s3c24xx/setup-camif.c:56:6: warning: no previous prototype for 's3c_camif_gpio_put' [-Wmissing-prototypes] >>> 56 | void s3c_camif_gpio_put(void) >> >> Maybe it should have been void all along? > > It seems there have never been any callers and the entire file > can just be removed, with the rest of that platform_data header > file moved to drivers/media/platform/s3c-camif/. Yes, it seems that patch never made it to mainline: https://git.linuxtv.org/snawrocki/media.git/commit/?h=testing/s3c-camif&id=355cbf835aff2aabf78390931cbbaa608af77967 I doubt there are still users of camera on the s3c2440 boards with current mainline kernels, if any at all, there are much better HW alternatives right now.
On Wed, Aug 12, 2020 at 12:46 PM Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 12.08.2020 11:14, Arnd Bergmann wrote: > > > > It seems there have never been any callers and the entire file > > can just be removed, with the rest of that platform_data header > > file moved to drivers/media/platform/s3c-camif/. > > Yes, it seems that patch never made it to mainline: > https://git.linuxtv.org/snawrocki/media.git/commit/?h=testing/s3c-camif&id=355cbf835aff2aabf78390931cbbaa608af77967 I suppose it would still apply if anyone was interested, but I see your point. > I doubt there are still users of camera on the s3c2440 boards > with current mainline kernels, if any at all, there are much > better HW alternatives right now. I see two board files (and no DT) instantiate the camif device: NexVision Nexcoder 2440 and the FriendlyARM mini2440. Can you say whether the camif on those would actually work at all without your patch? If not, we know that there are no users of that driver and could either drop it completely or move it to staging for a release or two. Arnd
On 12.08.2020 13:28, Arnd Bergmann wrote: > On Wed, Aug 12, 2020 at 12:46 PM Sylwester Nawrocki > <s.nawrocki@samsung.com> wrote: >> On 12.08.2020 11:14, Arnd Bergmann wrote: >>> >>> It seems there have never been any callers and the entire file >>> can just be removed, with the rest of that platform_data header >>> file moved to drivers/media/platform/s3c-camif/. >> >> Yes, it seems that patch never made it to mainline: >> https://protect2.fireeye.com/v1/url?k=abe5f73a-f6293cfe-abe47c75-0cc47a314e9a-7fafe832d055d852&q=1&e=2596ffb6-c4cb-492a-8c6f-a0e261567842&u=https%3A%2F%2Fgit.linuxtv.org%2Fsnawrocki%2Fmedia.git%2Fcommit%2F%3Fh%3Dtesting%2Fs3c-camif%26id%3D355cbf835aff2aabf78390931cbbaa608af77967 > > I suppose it would still apply if anyone was interested, but I see your > point. > >> I doubt there are still users of camera on the s3c2440 boards >> with current mainline kernels, if any at all, there are much >> better HW alternatives right now. > > I see two board files (and no DT) instantiate the camif device: > NexVision Nexcoder 2440 and the FriendlyARM mini2440. > > Can you say whether the camif on those would actually work > at all without your patch? If not, we know that there are no > users of that driver and could either drop it completely or move > it to staging for a release or two. Without additional patches the camif will not work, the driver needs an instance of struct s3c_camif_plat_data which specifies what image sensor is attached. I think we can drop the driver, together with the s3c_camif_device platform device definitions. It can always be added again if anyone ever needs it or converts the platform to DT. IMO all non-DT code in arch/arm/mach-s3c24xx is a candidate for removal, it just adds to the maintenance effort and I seriously doubt there are now any users of it.
On Wed, Aug 12, 2020 at 03:11:41PM +0200, Sylwester Nawrocki wrote: > On 12.08.2020 13:28, Arnd Bergmann wrote: > > On Wed, Aug 12, 2020 at 12:46 PM Sylwester Nawrocki > > <s.nawrocki@samsung.com> wrote: > >> On 12.08.2020 11:14, Arnd Bergmann wrote: > >>> > >>> It seems there have never been any callers and the entire file > >>> can just be removed, with the rest of that platform_data header > >>> file moved to drivers/media/platform/s3c-camif/. > >> > >> Yes, it seems that patch never made it to mainline: > >> https://protect2.fireeye.com/v1/url?k=abe5f73a-f6293cfe-abe47c75-0cc47a314e9a-7fafe832d055d852&q=1&e=2596ffb6-c4cb-492a-8c6f-a0e261567842&u=https%3A%2F%2Fgit.linuxtv.org%2Fsnawrocki%2Fmedia.git%2Fcommit%2F%3Fh%3Dtesting%2Fs3c-camif%26id%3D355cbf835aff2aabf78390931cbbaa608af77967 > > > > I suppose it would still apply if anyone was interested, but I see your > > point. > > > >> I doubt there are still users of camera on the s3c2440 boards > >> with current mainline kernels, if any at all, there are much > >> better HW alternatives right now. > > > > I see two board files (and no DT) instantiate the camif device: > > NexVision Nexcoder 2440 and the FriendlyARM mini2440. > > > > Can you say whether the camif on those would actually work > > at all without your patch? If not, we know that there are no > > users of that driver and could either drop it completely or move > > it to staging for a release or two. > > Without additional patches the camif will not work, the driver > needs an instance of struct s3c_camif_plat_data which specifies > what image sensor is attached. > > I think we can drop the driver, together with the s3c_camif_device > platform device definitions. It can always be added again if anyone > ever needs it or converts the platform to DT. Since the header was in /include/media I assumed there might be some user-space tools using it. But if it is not the case, I'll drop the code then. > IMO all non-DT code in arch/arm/mach-s3c24xx is a candidate for > removal, it just adds to the maintenance effort and I seriously > doubt there are now any users of it. That is quite tricky... I really do not know whether there are any real world users of S3C24xx and S3C64xx platforms. Evalkits are mostly not available for buying so I do not expect new designs. However still existing ones might be somewhere... Few years ago, back in Samsung, I mentioned removing them. That time I think Marek or you Sylwester, said that there are industrial applications using S3C24xx. I believe, why not. The trouble is - how to find such users? How to get in touch for testing or at least for bug reports if something is broken? Or even more important - is it worth to spend effort and time on this? If there is no single production system using recent Linux kernel, the answer should be negative... Best regards, Krzysztof
On 12.08.2020 15:31, Krzysztof Kozlowski wrote: > On Wed, Aug 12, 2020 at 03:11:41PM +0200, Sylwester Nawrocki wrote: >> On 12.08.2020 13:28, Arnd Bergmann wrote: >>> On Wed, Aug 12, 2020 at 12:46 PM Sylwester Nawrocki >>> <s.nawrocki@samsung.com> wrote: >>>> On 12.08.2020 11:14, Arnd Bergmann wrote: >>> I see two board files (and no DT) instantiate the camif device: >>> NexVision Nexcoder 2440 and the FriendlyARM mini2440. >>> >>> Can you say whether the camif on those would actually work >>> at all without your patch? If not, we know that there are no >>> users of that driver and could either drop it completely or move >>> it to staging for a release or two. >> >> Without additional patches the camif will not work, the driver >> needs an instance of struct s3c_camif_plat_data which specifies >> what image sensor is attached. >> >> I think we can drop the driver, together with the s3c_camif_device >> platform device definitions. It can always be added again if anyone >> ever needs it or converts the platform to DT. > > Since the header was in /include/media I assumed there might be some > user-space tools using it. But if it is not the case, I'll drop the code > then. That's a kernel internal header, only for board files, it should really have been added to include/linux/platform_data. >> IMO all non-DT code in arch/arm/mach-s3c24xx is a candidate for >> removal, it just adds to the maintenance effort and I seriously >> doubt there are now any users of it. > > That is quite tricky... I really do not know whether there are any real > world users of S3C24xx and S3C64xx platforms. Evalkits are mostly not > available for buying so I do not expect new designs. However still > existing ones might be somewhere... Few years ago, back in Samsung, I > mentioned removing them. That time I think Marek or you Sylwester, said > that there are industrial applications using S3C24xx. I believe, why > not. The trouble is - how to find such users? How to get in touch for > testing or at least for bug reports if something is broken? I believe if there any such applications of the S3C24XX SoCs still existing somewhere their long term support doesn't include updating to new kernels. I used to keep a running S3C2440 SoC based board just for the purpose of testing patches touching the common code, but I stopped it, I think it is not worth to waste time and health on it any more. For example support for the OSELAS.BSP-Pengutronix-Mini2440 BSP I used for tests ended 5 years ago [1]. > Or even more important - is it worth to spend effort and time on this? > If there is no single production system using recent Linux kernel, the > answer should be negative... I suspect nobody cares about that code (non-DT s3c24xx) any more for other than sentimental reasons. [1] https://git.pengutronix.de/cgit/OSELAS.BSP-Pengutronix-Mini2440
On Wed, Aug 12, 2020 at 05:58:52PM +0200, Sylwester Nawrocki wrote: > On 12.08.2020 15:31, Krzysztof Kozlowski wrote: > > On Wed, Aug 12, 2020 at 03:11:41PM +0200, Sylwester Nawrocki wrote: > >> On 12.08.2020 13:28, Arnd Bergmann wrote: > >>> On Wed, Aug 12, 2020 at 12:46 PM Sylwester Nawrocki > >>> <s.nawrocki@samsung.com> wrote: > >>>> On 12.08.2020 11:14, Arnd Bergmann wrote: > > >>> I see two board files (and no DT) instantiate the camif device: > >>> NexVision Nexcoder 2440 and the FriendlyARM mini2440. > >>> > >>> Can you say whether the camif on those would actually work > >>> at all without your patch? If not, we know that there are no > >>> users of that driver and could either drop it completely or move > >>> it to staging for a release or two. > >> > >> Without additional patches the camif will not work, the driver > >> needs an instance of struct s3c_camif_plat_data which specifies > >> what image sensor is attached. > >> > >> I think we can drop the driver, together with the s3c_camif_device > >> platform device definitions. It can always be added again if anyone > >> ever needs it or converts the platform to DT. > > > > Since the header was in /include/media I assumed there might be some > > user-space tools using it. But if it is not the case, I'll drop the code > > then. > > That's a kernel internal header, only for board files, it should really > have been added to include/linux/platform_data. > > >> IMO all non-DT code in arch/arm/mach-s3c24xx is a candidate for > >> removal, it just adds to the maintenance effort and I seriously > >> doubt there are now any users of it. > > > > That is quite tricky... I really do not know whether there are any real > > world users of S3C24xx and S3C64xx platforms. Evalkits are mostly not > > available for buying so I do not expect new designs. However still > > existing ones might be somewhere... Few years ago, back in Samsung, I > > mentioned removing them. That time I think Marek or you Sylwester, said > > that there are industrial applications using S3C24xx. I believe, why > > not. The trouble is - how to find such users? How to get in touch for > > testing or at least for bug reports if something is broken? > > I believe if there any such applications of the S3C24XX SoCs still existing > somewhere their long term support doesn't include updating to new kernels. > I used to keep a running S3C2440 SoC based board just for the purpose of > testing patches touching the common code, but I stopped it, I think it is > not worth to waste time and health on it any more. For example support for > the OSELAS.BSP-Pengutronix-Mini2440 BSP I used for tests ended 5 years ago > [1]. > > > Or even more important - is it worth to spend effort and time on this? > > If there is no single production system using recent Linux kernel, the > > answer should be negative... > > I suspect nobody cares about that code (non-DT s3c24xx) any more for other > than sentimental reasons. I'll start then with S3C camif driver. :) I guess drivers/media/platform/s3c-camif/ still should be left? Best regards, Krzysztof
On 12.08.2020 19:13, Krzysztof Kozlowski wrote: > I'll start then with S3C camif driver. :) I guess > drivers/media/platform/s3c-camif/ still should be left? No, if you want to remove the driver then this whole directory should be removed. The driver also supports the S3C6410 CAMIF but there is no s3c6410-camif devices instantiated in arch/arm/mach-s3c64xx either.
On 8/12/20 3:31 PM, Krzysztof Kozlowski wrote: > Or even more important - is it worth to spend effort and time on this? > If there is no single production system using recent Linux kernel, the > answer should be negative... Well, I have a server running on mini2440 with a not-too-young but not-too-old kernel. I don't have much time to test recent kernels though so I guess that doesn't count.
On Thu, 27 Aug 2020 at 22:51, Cedric Roux <sed@free.fr> wrote: > > On 8/12/20 3:31 PM, Krzysztof Kozlowski wrote: > > Or even more important - is it worth to spend effort and time on this? > > If there is no single production system using recent Linux kernel, the > > answer should be negative... > > Well, I have a server running on mini2440 with a not-too-young > but not-too-old kernel. I don't have much time to test recent > kernels though so I guess that doesn't count. Actually good to hear. It counts a little bit :) Best regards, Krzysztof
diff --git a/arch/arm/mach-s3c24xx/setup-camif.c b/arch/arm/mach-s3c24xx/setup-camif.c index 2b262fae3f61..4046afaad645 100644 --- a/arch/arm/mach-s3c24xx/setup-camif.c +++ b/arch/arm/mach-s3c24xx/setup-camif.c @@ -7,6 +7,7 @@ #include <linux/gpio.h> #include <plat/gpio-cfg.h> #include <mach/gpio-samsung.h> +#include <media/drv-intf/s3c_camif.h> /* Number of camera port pins, without FIELD */ #define S3C_CAMIF_NUM_GPIOS 13 @@ -53,7 +54,7 @@ int s3c_camif_gpio_get(void) return 0; } -void s3c_camif_gpio_put(void) +int s3c_camif_gpio_put(void) { int i, gpio_start, gpio_reset; @@ -64,4 +65,6 @@ void s3c_camif_gpio_put(void) if (gpio != gpio_reset) gpio_free(gpio); } + + return 0; }
The s3c_camif_gpio_put() declaration in include/media/drv-intf/s3c_camif.h header was different than definition. Fixing this allows to include that header to also fix the W=1 compile warnings: arch/arm/mach-s3c24xx/setup-camif.c:28:5: warning: no previous prototype for 's3c_camif_gpio_get' [-Wmissing-prototypes] 28 | int s3c_camif_gpio_get(void) arch/arm/mach-s3c24xx/setup-camif.c:56:6: warning: no previous prototype for 's3c_camif_gpio_put' [-Wmissing-prototypes] 56 | void s3c_camif_gpio_put(void) Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- Changes since v1: 1. New patch --- arch/arm/mach-s3c24xx/setup-camif.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)