Re: [PATCH 2/2] [media] include/media: move platform driver headers to a separate dir

2015-11-13 Thread Mauro Carvalho Chehab 
Em Wed, 11 Nov 2015 21:26:31 +0100
Arnd Bergmann  escreveu:

> On Wednesday 11 November 2015 15:14:48 Mauro Carvalho Chehab wrote:
> >  rename include/media/{ => platform}/exynos-fimc.h (100%)
> >  rename include/media/{ => platform}/mmp-camera.h (100%)
> >  rename include/media/{ => platform}/omap1_camera.h (100%)
> >  rename include/media/{ => platform}/omap4iss.h (100%)
> >  rename include/media/{ => platform}/s3c_camif.h (100%)
> >  rename include/media/{ => platform}/s5p_hdmi.h (100%)
> >  rename include/media/{ => platform}/sh_mobile_ceu.h (100%)
> >  rename include/media/{ => platform}/sh_mobile_csi2.h (100%)
> >  rename include/media/{ => platform}/sh_vou.h (100%)
> >  rename include/media/{ => platform}/sii9234.h (100%)
> >  rename include/media/{ => platform}/soc_camera.h (100%)
> >  rename include/media/{ => platform}/soc_camera_platform.h (98%)
> >  rename include/media/{ => platform}/soc_mediabus.h (100%)
> 
> This still seems to be a mix of various things. Some of these are interfaces
> between drivers, while others declare a foo_platform_data structure that
> is used to interface between platform code and the driver.

True. What about calling putting those driver interfaces under
include/media/drv-intf? That also helps moving the headers for other
non-platform drivers too.

> 
> I think the latter should go into include/linux/platform_data/media/*.h 
> instead.

Agreed.

Please see the enclosed patch:


Subject: [PATCH] [media] include/media: move platform driver headers to a
 separate dirs

Let's not mix headers used by the core with those headers that
are needed by some specific platform drivers or by platform data.

This patch was made via this script:
mkdir include/media/platform mkdir include/media/platform_data
(cd include/media/; git mv $(grep -l platform_data *.h|grep -v v4l2) 
platform_data/)
for i in include/media/*.h; do n=`basename $i`;  (for j in $(git grep 
-l $n); do dirname $j; done)|sort|uniq|grep -ve '^.$' > list; num=$(wc -l 
list|cut -d' ' -f1); if [ $num == 1 ]; then if [ "`grep platform list`" != "" 
]; then git mv $i include/media/drv-intf; fi; fi; done
git mv include/media/exynos* include/media/soc_* include/media/sh_* 
include/media/drv-intf/

And some headers were manually adjusted. Then, this script fixed the
address for those new headers:

for i in $(find include/media/ -type f); do n=`basename $i`; git grep 
-l $n; done|sort|uniq >files && (echo "for i in \$(cat files); do cat \$i | 
\\"; cd include/media; for j in platform/ platform_data/; do for i in $(ls $j); 
do echo "perl -ne 's,(include [\\\"\\<]media/)($i)([\\\"\\>]),\1$j\2\3,; print 
\$_' |\\"; done; done; echo "cat > a && mv a \$i; done") >script&& . ./script

Signed-off-by: Mauro Carvalho Chehab 

---

 arch/arm/mach-imx/mach-imx27_visstrim_m10.c  | 2 +-
 arch/arm/mach-imx/mach-mx27_3ds.c| 2 +-
 arch/arm/mach-imx/mach-mx31_3ds.c| 2 +-
 arch/arm/mach-imx/mach-mx35_3ds.c| 2 +-
 arch/arm/mach-imx/mach-pcm037.c  | 2 +-
 arch/arm/mach-imx/mx31moboard-marxbot.c  | 2 +-
 arch/arm/mach-imx/mx31moboard-smartbot.c | 2 +-
 arch/arm/mach-omap1/board-ams-delta.c| 2 +-
 arch/arm/mach-omap1/include/mach/camera.h| 2 +-
 arch/arm/mach-omap2/board-rx51-peripherals.c | 4 ++--
 arch/arm/mach-pxa/em-x270.c  | 2 +-
 arch/arm/mach-pxa/ezx.c  | 2 +-
 arch/arm/mach-pxa/mioa701.c  | 2 +-
 arch/arm/mach-pxa/palmz72.c  | 2 +-
 arch/arm/mach-pxa/pcm990-baseboard.c | 2 +-
 arch/arm/mach-shmobile/board-bockw.c | 2 +-
 arch/arm/plat-samsung/devs.c | 2 +-
 arch/sh/boards/mach-ap325rxa/setup.c | 6 +++---
 arch/sh/boards/mach-ecovec24/setup.c | 6 +++---
 arch/sh/boards/mach-kfr2r09/setup.c  | 4 ++--
 arch/sh/boards/mach-migor/setup.c| 4 ++--
 arch/sh/boards/mach-se/7724/setup.c  | 4 ++--
 drivers/media/common/cx2341x.c   | 2 +-
 drivers/media/common/saa7146/saa7146_core.c  | 2 +-
 drivers/media/common/saa7146/saa7146_fops.c  | 2 +-
 drivers/media/common/saa7146/saa7146_hlp.c   | 2 +-
 drivers/media/common/saa7146/saa7146_i2c.c   | 2 +-
 drivers/media/common/saa7146/saa7146_vbi.c   | 2 +-
 drivers/media/common/saa7146/saa7146_video.c | 2 +-
 drivers/media/i2c/cx25840/cx25840-audio.c| 2 +-
 drivers/media/i2c/cx25840/cx25840-core.c | 2 +-
 drivers/media/i2c/cx25840/cx25840-firmware.c | 2 +-
 drivers/media/i2c/cx25840/cx25840-ir.c   | 2 +-

Re: [PATCH 2/2] [media] include/media: move platform driver headers to a separate dir

2015-11-13 Thread Mauro Carvalho Chehab 
Em Fri, 13 Nov 2015 22:31:15 +0100
Arnd Bergmann  escreveu:

> On Friday 13 November 2015 17:13:41 Mauro Carvalho Chehab wrote:
> > Em Wed, 11 Nov 2015 21:26:31 +0100
> > Arnd Bergmann  escreveu:
> > 
> 
> >  include/media/{ => drv-intf}/cx2341x.h   | 0
> >  include/media/{ => drv-intf}/cx25840.h   | 0
> >  include/media/{ => drv-intf}/exynos-fimc.h   | 0
> >  include/media/{ => drv-intf}/msp3400.h   | 0
> >  include/media/{ => drv-intf}/s3c_camif.h | 0
> >  include/media/{ => drv-intf}/saa7146.h   | 0
> >  include/media/{ => drv-intf}/saa7146_vv.h| 2 +-
> >  include/media/{ => drv-intf}/sh_mobile_ceu.h | 0
> >  include/media/{ => drv-intf}/sh_mobile_csi2.h| 0
> >  include/media/{ => drv-intf}/sh_vou.h| 0
> >  include/media/{ => drv-intf}/si476x.h| 0
> >  include/media/{ => drv-intf}/soc_mediabus.h  | 0
> >  include/media/{ => drv-intf}/tea575x.h   | 0
> >  include/media/i2c/tw9910.h   | 2 +-
> >  include/media/{ => platform_data}/gpio-ir-recv.h | 0
> >  include/media/{ => platform_data}/ir-rx51.h  | 0
> >  include/media/{ => platform_data}/mmp-camera.h   | 0
> >  include/media/{ => platform_data}/omap1_camera.h | 0
> >  include/media/{ => platform_data}/omap4iss.h | 0
> >  include/media/{ => platform_data}/s5p_hdmi.h | 0
> >  include/media/{ => platform_data}/si4713.h   | 0
> >  include/media/{ => platform_data}/sii9234.h  | 0
> >  include/media/{ => platform_data}/smiapp.h   | 0
> >  include/media/{ => platform_data}/soc_camera.h   | 0
> >  include/media/{ => platform_data}/soc_camera_platform.h  | 2 +-
> >  include/media/{ => platform_data}/timb_radio.h   | 0
> >  include/media/{ => platform_data}/timb_video.h   | 0
> >  sound/pci/es1968.c   | 2 +-
> >  sound/pci/fm801.c| 2 +-
> >  155 files changed, 158 insertions(+), 158 deletions(-)
> 
> As Geert said, include/linux/platform_data/media/ would be nicer for
> consistency with other subsystems.

OK! I have a new series of patches almost ready. I'll be sending it
tomorrow, after addressing your concerns.

> 
> > diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c 
> > b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
> > index ede2bdbb5dd5..44ba1f28bb34 100644
> > --- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
> > +++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
> > @@ -33,7 +33,7 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> 
> This looks like a mistake: the header contains the string 'platform_data',
> but it is not actually a platform data header file in the sense
> that it defines the interface between platform and driver.
> 
> Maybe soc_camera.h is important enough to still leave it as a core
> file in the existing location? Or possibly a separate directory for
> media/soc_camera/*.h

Ok, I'll fix it. 

> 
> > @@ -24,7 +24,7 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> > +#include 
> >  #include 
> >  
> >  #include "cx25840-core.h"
> 
> For this case, I think the original patch to move it into include/media/i2c
> was more logical as it mirrors the file structure. I was mainly talking
> about the platform_data being different from the rest.

cx25840 is not (always) an I2C. On most devices, this is actually an IP
block inside the bridge chipset. 

That's why I didn't include it on patch 1/1. There's one thing to
notice about that, though: while most of the header is describing
the driver interface, it does contain one platform_data in the end:

/* pvr150_workaround activates a workaround for a hardware bug that is
   present in Hauppauge PVR-150 (and possibly PVR-500) cards that have
   certain NTSC tuners (tveeprom tuner model numbers 85, 99 and 112). The
   audio autodetect fails on some channels for these models and the workaround
   is to select the audio standard explicitly. Many thanks to Hauppauge for
   providing this information.
   This platform data only needs to be supplied by the ivtv driver. */
struct cx25840_platform_data {
int pvr150_workaround;
};

While we might split it, I guess it is not worth, specially since
I don't think we'll see any new driver using it.

Also, this is actually a hack used only by the ivtv driver.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] [media] include/media: move platform driver headers to a separate dir

2015-11-13 Thread Arnd Bergmann 
On Friday 13 November 2015 17:13:41 Mauro Carvalho Chehab wrote:
> Em Wed, 11 Nov 2015 21:26:31 +0100
> Arnd Bergmann  escreveu:
> 

>  include/media/{ => drv-intf}/cx2341x.h   | 0
>  include/media/{ => drv-intf}/cx25840.h   | 0
>  include/media/{ => drv-intf}/exynos-fimc.h   | 0
>  include/media/{ => drv-intf}/msp3400.h   | 0
>  include/media/{ => drv-intf}/s3c_camif.h | 0
>  include/media/{ => drv-intf}/saa7146.h   | 0
>  include/media/{ => drv-intf}/saa7146_vv.h| 2 +-
>  include/media/{ => drv-intf}/sh_mobile_ceu.h | 0
>  include/media/{ => drv-intf}/sh_mobile_csi2.h| 0
>  include/media/{ => drv-intf}/sh_vou.h| 0
>  include/media/{ => drv-intf}/si476x.h| 0
>  include/media/{ => drv-intf}/soc_mediabus.h  | 0
>  include/media/{ => drv-intf}/tea575x.h   | 0
>  include/media/i2c/tw9910.h   | 2 +-
>  include/media/{ => platform_data}/gpio-ir-recv.h | 0
>  include/media/{ => platform_data}/ir-rx51.h  | 0
>  include/media/{ => platform_data}/mmp-camera.h   | 0
>  include/media/{ => platform_data}/omap1_camera.h | 0
>  include/media/{ => platform_data}/omap4iss.h | 0
>  include/media/{ => platform_data}/s5p_hdmi.h | 0
>  include/media/{ => platform_data}/si4713.h   | 0
>  include/media/{ => platform_data}/sii9234.h  | 0
>  include/media/{ => platform_data}/smiapp.h   | 0
>  include/media/{ => platform_data}/soc_camera.h   | 0
>  include/media/{ => platform_data}/soc_camera_platform.h  | 2 +-
>  include/media/{ => platform_data}/timb_radio.h   | 0
>  include/media/{ => platform_data}/timb_video.h   | 0
>  sound/pci/es1968.c   | 2 +-
>  sound/pci/fm801.c| 2 +-
>  155 files changed, 158 insertions(+), 158 deletions(-)

As Geert said, include/linux/platform_data/media/ would be nicer for
consistency with other subsystems.

> diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c 
> b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
> index ede2bdbb5dd5..44ba1f28bb34 100644
> --- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
> +++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
> @@ -33,7 +33,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 

This looks like a mistake: the header contains the string 'platform_data',
but it is not actually a platform data header file in the sense
that it defines the interface between platform and driver.

Maybe soc_camera.h is important enough to still leave it as a core
file in the existing location? Or possibly a separate directory for
media/soc_camera/*.h

> @@ -24,7 +24,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  #include "cx25840-core.h"

For this case, I think the original patch to move it into include/media/i2c
was more logical as it mirrors the file structure. I was mainly talking
about the platform_data being different from the rest.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] [media] include/media: move platform driver headers to a separate dir

2015-11-13 Thread Geert Uytterhoeven 
On Fri, Nov 13, 2015 at 8:13 PM, Mauro Carvalho Chehab
 wrote:
>> I think the latter should go into include/linux/platform_data/media/*.h 
>> instead.
>
> Agreed.
>
> Please see the enclosed patch:
>
>
> Subject: [PATCH] [media] include/media: move platform driver headers to a
>  separate dirs
>
> Let's not mix headers used by the core with those headers that
> are needed by some specific platform drivers or by platform data.
>
> This patch was made via this script:
> mkdir include/media/platform mkdir include/media/platform_data
> (cd include/media/; git mv $(grep -l platform_data *.h|grep -v v4l2)

I think include/linux/platform_data/media/, like Arnd suggested,
would be better.

Then we can make it a common goal to empty include/linux/platform_data/ ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] [media] include/media: move platform driver headers to a separate dir

2015-11-11 Thread Mauro Carvalho Chehab 
Let's not mix headers used by the core with those headers that
are needed by some specific platform drivers.

This patch was made via this script:

mkdir include/media/platform
for i in include/media/*.h; do n=`basename $i`;  (for j in $(git grep 
-l $n); do dirname $j; done)|sort|uniq|grep -ve '^.$' > list; num=$(wc -l 
list|cut -d' ' -f1); if [ $num == 1 ]; then if [ "`grep platform list`" != "" 
]; then git mv $i include/media/platform; fi; fi; done
git mv include/media/exynos* include/media/s5p* include/media/omap* 
include/media/soc_* include/media/sh_include/media/platform/
for i in $(find include/media/ -type f); do n=`basename $i`; git grep 
-l $n; done|sort|uniq >files && (echo "for i in \$(cat files); do cat \$i | 
\\"; cd include/media; for j in rc/ i2c/ platform/ common_drv/; do for i in 
$(ls $j); do echo "perl -ne 's,(include 
[\\\"\\<]media/)($i)([\\\"\\>]),\1$j\2\3,; print \$_' |\\"; done; done; echo 
"cat > a && mv a \$i; done") >script && . ./script

Signed-off-by: Mauro Carvalho Chehab 

 rename include/media/{ => platform}/exynos-fimc.h (100%)
 rename include/media/{ => platform}/mmp-camera.h (100%)
 rename include/media/{ => platform}/omap1_camera.h (100%)
 rename include/media/{ => platform}/omap4iss.h (100%)
 rename include/media/{ => platform}/s3c_camif.h (100%)
 rename include/media/{ => platform}/s5p_hdmi.h (100%)
 rename include/media/{ => platform}/sh_mobile_ceu.h (100%)
 rename include/media/{ => platform}/sh_mobile_csi2.h (100%)
 rename include/media/{ => platform}/sh_vou.h (100%)
 rename include/media/{ => platform}/sii9234.h (100%)
 rename include/media/{ => platform}/soc_camera.h (100%)
 rename include/media/{ => platform}/soc_camera_platform.h (98%)
 rename include/media/{ => platform}/soc_mediabus.h (100%)

diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c 
b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index ede2bdbb5dd5..99bd64c69a4f 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/arch/arm/mach-imx/mach-mx27_3ds.c 
b/arch/arm/mach-imx/mach-mx27_3ds.c
index 9ef4640f3660..74d073fd4fa6 100644
--- a/arch/arm/mach-imx/mach-mx27_3ds.c
+++ b/arch/arm/mach-imx/mach-mx27_3ds.c
@@ -31,7 +31,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/arch/arm/mach-imx/mach-mx31_3ds.c 
b/arch/arm/mach-imx/mach-mx31_3ds.c
index 65a0dc06a97c..5e1b9c335103 100644
--- a/arch/arm/mach-imx/mach-mx31_3ds.c
+++ b/arch/arm/mach-imx/mach-mx31_3ds.c
@@ -28,7 +28,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/arch/arm/mach-imx/mach-mx35_3ds.c 
b/arch/arm/mach-imx/mach-mx35_3ds.c
index 7e315f00648d..355b9521f7f0 100644
--- a/arch/arm/mach-imx/mach-mx35_3ds.c
+++ b/arch/arm/mach-imx/mach-mx35_3ds.c
@@ -45,7 +45,7 @@
 
 #include 
 
-#include 
+#include 
 
 #include "3ds_debugboard.h"
 #include "common.h"
diff --git a/arch/arm/mach-imx/mach-pcm037.c b/arch/arm/mach-imx/mach-pcm037.c
index 6d879417db49..02cc07b0a687 100644
--- a/arch/arm/mach-imx/mach-pcm037.c
+++ b/arch/arm/mach-imx/mach-pcm037.c
@@ -35,7 +35,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/arch/arm/mach-imx/mx31moboard-marxbot.c 
b/arch/arm/mach-imx/mx31moboard-marxbot.c
index 2e895a82a6eb..79ae360a9c8f 100644
--- a/arch/arm/mach-imx/mx31moboard-marxbot.c
+++ b/arch/arm/mach-imx/mx31moboard-marxbot.c
@@ -24,7 +24,7 @@
 
 #include 
 
-#include 
+#include 
 
 #include "common.h"
 #include "devices-imx31.h"
diff --git a/arch/arm/mach-imx/mx31moboard-smartbot.c 
b/arch/arm/mach-imx/mx31moboard-smartbot.c
index 89fc35a64448..00bd100df69a 100644
--- a/arch/arm/mach-imx/mx31moboard-smartbot.c
+++ b/arch/arm/mach-imx/mx31moboard-smartbot.c
@@ -23,7 +23,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include "board-mx31moboard.h"
 #include "common.h"
diff --git a/arch/arm/mach-omap1/board-ams-delta.c 
b/arch/arm/mach-omap1/board-ams-delta.c
index a95499ea8706..7adef38f27c2 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -28,7 +28,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/arch/arm/mach-omap1/include/mach/camera.h 
b/arch/arm/mach-omap1/include/mach/camera.h
index 847d00f0bb0a..0df5ebf85de6 100644
--- a/arch/arm/mach-omap1/include/mach/camera.h
+++ b/arch/arm/mach-omap1/include/mach/camera.h
@@ -1,7 +1,7 @@
 #ifndef __ASM_ARCH_CAMERA_H_
 #define __ASM_ARCH_CAMERA_H_
 
-#include 
+#include 
 
 void omap1_camera_init(void *);
 
diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index 9d7072b04045..ae645794ffd0 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -34,7 +34,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include 
 #include 
diff

Re: [PATCH 2/2] [media] include/media: move platform driver headers to a separate dir

2015-11-11 Thread Arnd Bergmann 
On Wednesday 11 November 2015 15:14:48 Mauro Carvalho Chehab wrote:
>  rename include/media/{ => platform}/exynos-fimc.h (100%)
>  rename include/media/{ => platform}/mmp-camera.h (100%)
>  rename include/media/{ => platform}/omap1_camera.h (100%)
>  rename include/media/{ => platform}/omap4iss.h (100%)
>  rename include/media/{ => platform}/s3c_camif.h (100%)
>  rename include/media/{ => platform}/s5p_hdmi.h (100%)
>  rename include/media/{ => platform}/sh_mobile_ceu.h (100%)
>  rename include/media/{ => platform}/sh_mobile_csi2.h (100%)
>  rename include/media/{ => platform}/sh_vou.h (100%)
>  rename include/media/{ => platform}/sii9234.h (100%)
>  rename include/media/{ => platform}/soc_camera.h (100%)
>  rename include/media/{ => platform}/soc_camera_platform.h (98%)
>  rename include/media/{ => platform}/soc_mediabus.h (100%)

This still seems to be a mix of various things. Some of these are interfaces
between drivers, while others declare a foo_platform_data structure that
is used to interface between platform code and the driver.

I think the latter should go into include/linux/platform_data/media/*.h instead.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html