sh/boards/mach-migor/setup.c build error

sh/boards/mach-migor/setup.c build error

[Adrian Bunk]

Commit 81034663159f39d005316b5c139038459cd16721
(V4L/DVB (8687): soc-camera: Move .power and .reset from
 soc_camera host to sensor driver) causes the following build error:

<--  snip  -->

...
  CC      arch/sh/boards/mach-migor/setup.o
arch/sh/boards/mach-migor/setup.c:408: error: unknown field 'enable_camera' specified in initializer
arch/sh/boards/mach-migor/setup.c:408: warning: excess elements in struct initializer
arch/sh/boards/mach-migor/setup.c:408: warning: (near initialization for 'sh_mobile_ceu_info')
arch/sh/boards/mach-migor/setup.c:409: error: unknown field 'disable_camera' specified in initializer
arch/sh/boards/mach-migor/setup.c:409: warning: excess elements in struct initializer
arch/sh/boards/mach-migor/setup.c:409: warning: (near initialization for 'sh_mobile_ceu_info')
make[2]: *** [arch/sh/boards/mach-migor/setup.o] Error 1

<--  snip  -->


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

[PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

Fix Migo-R compile breakage caused by incomplete merge.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

---

Hi Adrian,

please see, if the patch below fixes it. Completely untested. Magnus, 
could you please verify if it also works (of course, if it at least 
compiles:-)) If it doesn't, please fix it along these lines, if it suits 
your needs.

On Tue, 14 Oct 2008, Adrian Bunk wrote:

> Commit 81034663159f39d005316b5c139038459cd16721
> (V4L/DVB (8687): soc-camera: Move .power and .reset from
>  soc_camera host to sensor driver) causes the following build error:
> 
> <--  snip  -->
> 
> ...
>   CC      arch/sh/boards/mach-migor/setup.o
> arch/sh/boards/mach-migor/setup.c:408: error: unknown field 'enable_camera' specified in initializer
> arch/sh/boards/mach-migor/setup.c:408: warning: excess elements in struct initializer
> arch/sh/boards/mach-migor/setup.c:408: warning: (near initialization for 'sh_mobile_ceu_info')
> arch/sh/boards/mach-migor/setup.c:409: error: unknown field 'disable_camera' specified in initializer
> arch/sh/boards/mach-migor/setup.c:409: warning: excess elements in struct initializer
> arch/sh/boards/mach-migor/setup.c:409: warning: (near initialization for 'sh_mobile_ceu_info')
> make[2]: *** [arch/sh/boards/mach-migor/setup.o] Error 1
> 
> <--  snip  -->


diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 714dce9..95459f3 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -312,6 +312,14 @@ static void camera_power_off(void)
 	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
 }
 
+static void camera_power(int mode)
+{
+	if (mode)
+		camera_power_on();
+	else
+		camera_power_off();
+}
+
 #ifdef CONFIG_I2C
 static unsigned char camera_ov772x_magic[]  {
@@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
 	},
 	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
 	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
+	.power = camera_power,
 	.set_capture = ov772x_set_capture,
 };
 
@@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
 static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
 	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
 	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
-	.enable_camera = camera_power_on,
-	.disable_camera = camera_power_off,
 };
 
 static struct resource migor_ceu_resources[] = {
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index 1adc257..5b08873 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -44,11 +44,21 @@ soc_camera_platform_get_info(struct soc_camera_device *icd)
 
 static int soc_camera_platform_init(struct soc_camera_device *icd)
 {
+	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
+
+	if (p->power)
+		p->power(1);
+
 	return 0;
 }
 
 static int soc_camera_platform_release(struct soc_camera_device *icd)
 {
+	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
+
+	if (p->power)
+		p->power(0);
+
 	return 0;
 }
 
diff --git a/include/media/soc_camera_platform.h b/include/media/soc_camera_platform.h
index 851f182..7c81ad3 100644
--- a/include/media/soc_camera_platform.h
+++ b/include/media/soc_camera_platform.h
@@ -9,6 +9,7 @@ struct soc_camera_platform_info {
 	unsigned long format_depth;
 	struct v4l2_pix_format format;
 	unsigned long bus_param;
+	void (*power)(int);
 	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
 };
 

Re: [PATCH] soc-camera: fix compile breakage on SH

[Adrian Bunk]

On Tue, Oct 14, 2008 at 11:53:37PM +0200, Guennadi Liakhovetski wrote:
> Fix Migo-R compile breakage caused by incomplete merge.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> ---
> 
> Hi Adrian,

Hi Guennadi,

> please see, if the patch below fixes it. Completely untested. Magnus, 
> could you please verify if it also works (of course, if it at least 
> compiles:-)) If it doesn't, please fix it along these lines, if it suits 
> your needs.
>...

it does compile.

I cannot verify whether it also works.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] soc-camera: fix compile breakage on SH

[Adrian Bunk]

On Wed, Oct 15, 2008 at 06:33:03AM +0300, Adrian Bunk wrote:
> On Tue, Oct 14, 2008 at 11:53:37PM +0200, Guennadi Liakhovetski wrote:
> > Fix Migo-R compile breakage caused by incomplete merge.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > 
> > ---
> > 
> > Hi Adrian,
> 
> Hi Guennadi,
> 
> > please see, if the patch below fixes it. Completely untested. Magnus, 
> > could you please verify if it also works (of course, if it at least 
> > compiles:-)) If it doesn't, please fix it along these lines, if it suits 
> > your needs.
> >...
> 
> it does compile.
>...

But it causes compile breakage elsewhere:

<--  snip  -->

...
  CC      drivers/media/video/soc_camera_platform.o
drivers/media/video/soc_camera_platform.c: In function ‘soc_camera_platform_init’:
drivers/media/video/soc_camera_platform.c:49: error: ‘struct soc_camera_platform_info’ has no member named ‘power’
drivers/media/video/soc_camera_platform.c:50: error: ‘struct soc_camera_platform_info’ has no member named ‘power’
drivers/media/video/soc_camera_platform.c: In function ‘soc_camera_platform_release’:
drivers/media/video/soc_camera_platform.c:59: error: ‘struct soc_camera_platform_info’ has no member named ‘power’
drivers/media/video/soc_camera_platform.c:60: error: ‘struct soc_camera_platform_info’ has no member named ‘power’
make[4]: *** [drivers/media/video/soc_camera_platform.o] Error 1

<--  snip  -->

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] soc-camera: fix compile breakage on SH

[Magnus Damm]

On Wed, Oct 15, 2008 at 2:20 PM, Adrian Bunk <bunk@kernel.org> wrote:
> On Wed, Oct 15, 2008 at 06:33:03AM +0300, Adrian Bunk wrote:
>> On Tue, Oct 14, 2008 at 11:53:37PM +0200, Guennadi Liakhovetski wrote:
>> > Fix Migo-R compile breakage caused by incomplete merge.
>> >
>> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> >
>> > ---
>> >
>> > Hi Adrian,
>>
>> Hi Guennadi,
>>
>> > please see, if the patch below fixes it. Completely untested. Magnus,
>> > could you please verify if it also works (of course, if it at least
>> > compiles:-)) If it doesn't, please fix it along these lines, if it suits
>> > your needs.
>> >...
>>
>> it does compile.
>>...
>
> But it causes compile breakage elsewhere:
>
> <--  snip  -->

Hi guys,

Thanks for working on fixing the breakage. I'd prefer to wait a bit
since there are quite a few pinmux patches queued up that may break if
we merge a fix right now. I can fix it up later on.

Thanks,

/ magnus

Re: [PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

Hi Magnus

On Wed, 15 Oct 2008, Magnus Damm wrote:

> Thanks for working on fixing the breakage. I'd prefer to wait a bit
> since there are quite a few pinmux patches queued up that may break if
> we merge a fix right now. I can fix it up later on.

no, I would not leave the kernel in a non-compilable state even if just 
for one board. Please, test a new version of the patch below. And yes, You 
will have to rebase your patches, sorry. Another thing, could you also, 
please, add a license / copyright header to 
include/media/soc_camera_platform.h?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Subject: [PATCH v2] soc-camera: fix compile breakage on SH

Fix Migo-R compile breakage caused by incomplete merge. Also remove 
redundant soc_camera_platform_info struct definition from 
drivers/media/video/soc_camera_platform.c

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

---

diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 714dce9..95459f3 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -312,6 +312,14 @@ static void camera_power_off(void)
 	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
 }
 
+static void camera_power(int mode)
+{
+	if (mode)
+		camera_power_on();
+	else
+		camera_power_off();
+}
+
 #ifdef CONFIG_I2C
 static unsigned char camera_ov772x_magic[]  {
@@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
 	},
 	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
 	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
+	.power = camera_power,
 	.set_capture = ov772x_set_capture,
 };
 
@@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
 static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
 	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
 	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
-	.enable_camera = camera_power_on,
-	.disable_camera = camera_power_off,
 };
 
 static struct resource migor_ceu_resources[] = {
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index 1adc257..bb7a9d4 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -18,15 +18,7 @@
 #include <linux/videodev2.h>
 #include <media/v4l2-common.h>
 #include <media/soc_camera.h>
-
-struct soc_camera_platform_info {
-	int iface;
-	char *format_name;
-	unsigned long format_depth;
-	struct v4l2_pix_format format;
-	unsigned long bus_param;
-	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
-};
+#include <media/soc_camera_platform.h>
 
 struct soc_camera_platform_priv {
 	struct soc_camera_platform_info *info;
@@ -44,11 +36,21 @@ soc_camera_platform_get_info(struct soc_camera_device *icd)
 
 static int soc_camera_platform_init(struct soc_camera_device *icd)
 {
+	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
+
+	if (p->power)
+		p->power(1);
+
 	return 0;
 }
 
 static int soc_camera_platform_release(struct soc_camera_device *icd)
 {
+	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
+
+	if (p->power)
+		p->power(0);
+
 	return 0;
 }
 
diff --git a/include/media/soc_camera_platform.h b/include/media/soc_camera_platform.h
index 851f182..7c81ad3 100644
--- a/include/media/soc_camera_platform.h
+++ b/include/media/soc_camera_platform.h
@@ -9,6 +9,7 @@ struct soc_camera_platform_info {
 	unsigned long format_depth;
 	struct v4l2_pix_format format;
 	unsigned long bus_param;
+	void (*power)(int);
 	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
 };
 

Re: [PATCH] soc-camera: fix compile breakage on SH

[Magnus Damm]

Hi Guennadi,

On Wed, Oct 15, 2008 at 3:41 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi Magnus
>
> On Wed, 15 Oct 2008, Magnus Damm wrote:
>
>> Thanks for working on fixing the breakage. I'd prefer to wait a bit
>> since there are quite a few pinmux patches queued up that may break if
>> we merge a fix right now. I can fix it up later on.
>
> no, I would not leave the kernel in a non-compilable state even if just
> for one board. Please, test a new version of the patch below. And yes, You
> will have to rebase your patches, sorry. Another thing, could you also,
> please, add a license / copyright header to
> include/media/soc_camera_platform.h?

I'm not asking you to keep the board broken forever. It's just a
question of in which order the trees are getting merged. Again, I'd
rather see that this fix is put _on_top_ of the patches that are
already queued up in the SuperH tree. Merging it before doesn't help
anything in my opinion - especially since the change should go though
the SuperH tree anyway.

Feel free to add any header you like. =)

/ magnus

Re: [PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

On Wed, 15 Oct 2008, Magnus Damm wrote:

> Hi Guennadi,
> 
> On Wed, Oct 15, 2008 at 3:41 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > Hi Magnus
> >
> > On Wed, 15 Oct 2008, Magnus Damm wrote:
> >
> >> Thanks for working on fixing the breakage. I'd prefer to wait a bit
> >> since there are quite a few pinmux patches queued up that may break if
> >> we merge a fix right now. I can fix it up later on.
> >
> > no, I would not leave the kernel in a non-compilable state even if just
> > for one board. Please, test a new version of the patch below. And yes, You
> > will have to rebase your patches, sorry. Another thing, could you also,
> > please, add a license / copyright header to
> > include/media/soc_camera_platform.h?
> 
> I'm not asking you to keep the board broken forever. It's just a
> question of in which order the trees are getting merged. Again, I'd
> rather see that this fix is put _on_top_ of the patches that are
> already queued up in the SuperH tree. Merging it before doesn't help
> anything in my opinion - especially since the change should go though
> the SuperH tree anyway.

I think, compilation-breakage fixes should have higher priority than 
further enhancements. Think about bisection. If you now first commit 
several more patches, you make the interval where the tree is not 
compilable longer, and thus the probabiliy that someone hits it in their 
git.bisect higher. That's why I think any compilation breakage should be 
fixed ASAP. And which changes do you mean specifically? This one:

http://marc.info/?l=linux-sh&m\x122346619318532&w=2

Yes, indeed they conflict, but it is trivial to fix. So, I would prefer to 
close the compile-breakage window ASAP, and then trivially update that one 
your patch. Let's see what others say. And as for through which tree it 
should go, if you insist the sh-part going through the sh-tree, then it 
has to be split into two parts - video and sh. Thus extending the 
breakage-window by one commit...

> Feel free to add any header you like. =)

Thanks, but no thanks:-) I cannot add your copyright, at least not without 
your explicit agreement (I think). So, I'd prefer you submit a patch for 
that.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [PATCH] soc-camera: fix compile breakage on SH

[Magnus Damm]

On Wed, Oct 15, 2008 at 5:26 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Wed, 15 Oct 2008, Magnus Damm wrote:
>> On Wed, Oct 15, 2008 at 3:41 PM, Guennadi Liakhovetski
>> <g.liakhovetski@gmx.de> wrote:
>> > On Wed, 15 Oct 2008, Magnus Damm wrote:
>> >
>> >> Thanks for working on fixing the breakage. I'd prefer to wait a bit
>> >> since there are quite a few pinmux patches queued up that may break if
>> >> we merge a fix right now. I can fix it up later on.
>> >
>> > no, I would not leave the kernel in a non-compilable state even if just
>> > for one board. Please, test a new version of the patch below. And yes, You
>> > will have to rebase your patches, sorry. Another thing, could you also,
>> > please, add a license / copyright header to
>> > include/media/soc_camera_platform.h?
>>
>> I'm not asking you to keep the board broken forever. It's just a
>> question of in which order the trees are getting merged. Again, I'd
>> rather see that this fix is put _on_top_ of the patches that are
>> already queued up in the SuperH tree. Merging it before doesn't help
>> anything in my opinion - especially since the change should go though
>> the SuperH tree anyway.
>
> I think, compilation-breakage fixes should have higher priority than
> further enhancements. Think about bisection. If you now first commit
> several more patches, you make the interval where the tree is not
> compilable longer, and thus the probabiliy that someone hits it in their
> git.bisect higher. That's why I think any compilation breakage should be
> fixed ASAP. And which changes do you mean specifically? This one:
>
> http://marc.info/?l=linux-sh&m\x122346619318532&w=2
>
> Yes, indeed they conflict, but it is trivial to fix. So, I would prefer to
> close the compile-breakage window ASAP, and then trivially update that one
> your patch. Let's see what others say. And as for through which tree it
> should go, if you insist the sh-part going through the sh-tree, then it
> has to be split into two parts - video and sh. Thus extending the
> breakage-window by one commit...

Yeah, that one plus a patch for the smc91x platform data and another
one for mmc (which needs updating anyway). So maybe it's not such a
big deal. And I see your point with closing the window ASAP to do
damage control. Otoh I wonder how big difference it will be extending
the breakage window with one commit - there must be zillions of
commits in after the breakage already.

Paul, any strong feelings regarding merging things though the SuperH tree?

>> Feel free to add any header you like. =)
>
> Thanks, but no thanks:-) I cannot add your copyright, at least not without
> your explicit agreement (I think). So, I'd prefer you submit a patch for
> that.

I wonder if it's a large enough bit sequence to actually copyright. =)
But sure, I'll do that.

Is this .29 material, or will there be a second v4l round with trivial
driver changes for .28?

I've already posted some vivi patches and two simple patches for the
sh_mobile_ceu driver - sorry about the timing - and i have one more
sh_mobile_ceu patch outstanding. Also, I think one of my coworkers may
post a soc_camera driver for ov772x chips soon too.

Is there any chance that can get included in .28?

Thank you!

/ magnus

Re: [PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

On Wed, 15 Oct 2008, Magnus Damm wrote:

> Yeah, that one plus a patch for the smc91x platform data and another
> one for mmc (which needs updating anyway). So maybe it's not such a
> big deal. And I see your point with closing the window ASAP to do
> damage control. Otoh I wonder how big difference it will be extending
> the breakage window with one commit - there must be zillions of
> commits in after the breakage already.

So, let's close it ASAP.

> Paul, any strong feelings regarding merging things though the SuperH tree?

Hm, I think, soc_camera_platform parts would have to go through v4l tree. 
Unless we ask Andrew / Linus apply a fix directly to minimize the window, 
after you test it of course:-)

> > Thanks, but no thanks:-) I cannot add your copyright, at least not without
> > your explicit agreement (I think). So, I'd prefer you submit a patch for
> > that.
> 
> I wonder if it's a large enough bit sequence to actually copyright. =)
> But sure, I'll do that.

Good, thanks.

> Is this .29 material, or will there be a second v4l round with trivial
> driver changes for .28?
> 
> I've already posted some vivi patches and two simple patches for the
> sh_mobile_ceu driver - sorry about the timing - and i have one more
> sh_mobile_ceu patch outstanding. Also, I think one of my coworkers may
> post a soc_camera driver for ov772x chips soon too.
> 
> Is there any chance that can get included in .28?

I think there is still a chane, if we are fast enough. In any case I 
queued your sh_mobile_ceu patches, will try to push them on to Mauro 
latest this weekend.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

[PATCH v2] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

Fix Migo-R compile breakage caused by incomplete merge. Also remove 
redundant soc_camera_platform_info struct definition from 
drivers/media/video/soc_camera_platform.c

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Tested-by: Magnus Damm <damm@igel.co.jp>

---

Paul, Mauro, as this patch closes a compile-breakage, I'd like to get it 
upstream asap. What would be the fastest way: get your both Acked-by and 
send it directly to Andrew / Linus, or an Acked-by from one of you and 
let the other one merge (you can decide who does what:-)) Splitting it 
into two patches and merging separately seems suboptimal to me in this 
case.

Thanks
Guennadi

diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 714dce9..95459f3 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -312,6 +312,14 @@ static void camera_power_off(void)
 	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
 }
 
+static void camera_power(int mode)
+{
+	if (mode)
+		camera_power_on();
+	else
+		camera_power_off();
+}
+
 #ifdef CONFIG_I2C
 static unsigned char camera_ov772x_magic[]  {
@@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
 	},
 	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
 	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
+	.power = camera_power,
 	.set_capture = ov772x_set_capture,
 };
 
@@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
 static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
 	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
 	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
-	.enable_camera = camera_power_on,
-	.disable_camera = camera_power_off,
 };
 
 static struct resource migor_ceu_resources[] = {
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index 1adc257..bb7a9d4 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -18,15 +18,7 @@
 #include <linux/videodev2.h>
 #include <media/v4l2-common.h>
 #include <media/soc_camera.h>
-
-struct soc_camera_platform_info {
-	int iface;
-	char *format_name;
-	unsigned long format_depth;
-	struct v4l2_pix_format format;
-	unsigned long bus_param;
-	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
-};
+#include <media/soc_camera_platform.h>
 
 struct soc_camera_platform_priv {
 	struct soc_camera_platform_info *info;
@@ -44,11 +36,21 @@ soc_camera_platform_get_info(struct soc_camera_device *icd)
 
 static int soc_camera_platform_init(struct soc_camera_device *icd)
 {
+	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
+
+	if (p->power)
+		p->power(1);
+
 	return 0;
 }
 
 static int soc_camera_platform_release(struct soc_camera_device *icd)
 {
+	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
+
+	if (p->power)
+		p->power(0);
+
 	return 0;
 }
 
diff --git a/include/media/soc_camera_platform.h b/include/media/soc_camera_platform.h
index 851f182..7c81ad3 100644
--- a/include/media/soc_camera_platform.h
+++ b/include/media/soc_camera_platform.h
@@ -9,6 +9,7 @@ struct soc_camera_platform_info {
 	unsigned long format_depth;
 	struct v4l2_pix_format format;
 	unsigned long bus_param;
+	void (*power)(int);
 	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
 };
 

Re: [PATCH v2] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

On Wed, 15 Oct 2008, Guennadi Liakhovetski wrote:

> Fix Migo-R compile breakage caused by incomplete merge. Also remove 
> redundant soc_camera_platform_info struct definition from 
> drivers/media/video/soc_camera_platform.c

Ok, I got no acks to this one, so, let's drop it, I'll submit its two 
parts to the v4l and sh lists and maintainers independently.

Thanks
Guennadi

> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Tested-by: Magnus Damm <damm@igel.co.jp>
> 
> ---
> 
> Paul, Mauro, as this patch closes a compile-breakage, I'd like to get it 
> upstream asap. What would be the fastest way: get your both Acked-by and 
> send it directly to Andrew / Linus, or an Acked-by from one of you and 
> let the other one merge (you can decide who does what:-)) Splitting it 
> into two patches and merging separately seems suboptimal to me in this 
> case.
> 
> Thanks
> Guennadi
> 
> diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> index 714dce9..95459f3 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -312,6 +312,14 @@ static void camera_power_off(void)
>  	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
>  }
>  
> +static void camera_power(int mode)
> +{
> +	if (mode)
> +		camera_power_on();
> +	else
> +		camera_power_off();
> +}
> +
>  #ifdef CONFIG_I2C
>  static unsigned char camera_ov772x_magic[] >  {
> @@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
>  	},
>  	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
>  	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
> +	.power = camera_power,
>  	.set_capture = ov772x_set_capture,
>  };
>  
> @@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
>  static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
>  	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
>  	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
> -	.enable_camera = camera_power_on,
> -	.disable_camera = camera_power_off,
>  };
>  
>  static struct resource migor_ceu_resources[] = {
> diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
> index 1adc257..bb7a9d4 100644
> --- a/drivers/media/video/soc_camera_platform.c
> +++ b/drivers/media/video/soc_camera_platform.c
> @@ -18,15 +18,7 @@
>  #include <linux/videodev2.h>
>  #include <media/v4l2-common.h>
>  #include <media/soc_camera.h>
> -
> -struct soc_camera_platform_info {
> -	int iface;
> -	char *format_name;
> -	unsigned long format_depth;
> -	struct v4l2_pix_format format;
> -	unsigned long bus_param;
> -	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
> -};
> +#include <media/soc_camera_platform.h>
>  
>  struct soc_camera_platform_priv {
>  	struct soc_camera_platform_info *info;
> @@ -44,11 +36,21 @@ soc_camera_platform_get_info(struct soc_camera_device *icd)
>  
>  static int soc_camera_platform_init(struct soc_camera_device *icd)
>  {
> +	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
> +
> +	if (p->power)
> +		p->power(1);
> +
>  	return 0;
>  }
>  
>  static int soc_camera_platform_release(struct soc_camera_device *icd)
>  {
> +	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
> +
> +	if (p->power)
> +		p->power(0);
> +
>  	return 0;
>  }
>  
> diff --git a/include/media/soc_camera_platform.h b/include/media/soc_camera_platform.h
> index 851f182..7c81ad3 100644
> --- a/include/media/soc_camera_platform.h
> +++ b/include/media/soc_camera_platform.h
> @@ -9,6 +9,7 @@ struct soc_camera_platform_info {
>  	unsigned long format_depth;
>  	struct v4l2_pix_format format;
>  	unsigned long bus_param;
> +	void (*power)(int);
>  	int (*set_capture)(struct soc_camera_platform_info *info, int enable);
>  };
>  
> 
> --
> video4linux-list mailing list
> Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

[PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

Fix Migo-R compile breakage caused by incomplete merge.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Tested-by: Magnus Damm <damm@igel.co.jp>
---

Paul, as I explained in my reply to my own patch to fix this compile 
breakage, as it didn't get Acks soon enough as I hoped, I've splitted it 
into two parts, this is the sh-part. It depends on patch "soc-camera: move 
sensor power management to soc_camera_platform.c", posted simultaneously 
to the V4L tree. But even if you commit it before the v4l-patch gets 
merged, it won't get worth - it will just stay broken until the other one 
gets merged too:-)

Thanks
Guennadi

 arch/sh/boards/mach-migor/setup.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 714dce9..95459f3 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -312,6 +312,14 @@ static void camera_power_off(void)
 	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
 }
 
+static void camera_power(int mode)
+{
+	if (mode)
+		camera_power_on();
+	else
+		camera_power_off();
+}
+
 #ifdef CONFIG_I2C
 static unsigned char camera_ov772x_magic[]  {
@@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
 	},
 	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
 	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
+	.power = camera_power,
 	.set_capture = ov772x_set_capture,
 };
 
@@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
 static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
 	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
 	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
-	.enable_camera = camera_power_on,
-	.disable_camera = camera_power_off,
 };
 
 static struct resource migor_ceu_resources[] = {
-- 
1.5.4

Re: [PATCH] soc-camera: fix compile breakage on SH

[Paul Mundt]

On Fri, Oct 17, 2008 at 01:16:40AM +0200, Guennadi Liakhovetski wrote:
> diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> index 714dce9..95459f3 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -312,6 +312,14 @@ static void camera_power_off(void)
>  	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
>  }
>  
> +static void camera_power(int mode)
> +{
> +	if (mode)
> +		camera_power_on();
> +	else
> +		camera_power_off();
> +}
> +
>  #ifdef CONFIG_I2C
>  static unsigned char camera_ov772x_magic[] >  {
> @@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
>  	},
>  	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
>  	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
> +	.power = camera_power,
>  	.set_capture = ov772x_set_capture,
>  };
>  
> @@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
>  static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
>  	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
>  	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
> -	.enable_camera = camera_power_on,
> -	.disable_camera = camera_power_off,
>  };
>  
I don't like this. Keeping the enable and disable_camera interfaces
is really the way we want to go, evident by the fact that your new power
callback is forced to call in to one or the other anyways. I would rather
see these moved in to struct soc_camera_platform_info, rather than having
every single board in existence have to model that if (mode) on; else off
crap.

Also, there is not much point in splitting these changes out. They are
coupled, and splitting them out only causes confusion (especially across
a bisect).

Re: [PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

On Mon, 20 Oct 2008, Paul Mundt wrote:

> On Fri, Oct 17, 2008 at 01:16:40AM +0200, Guennadi Liakhovetski wrote:
> > diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> > index 714dce9..95459f3 100644
> > --- a/arch/sh/boards/mach-migor/setup.c
> > +++ b/arch/sh/boards/mach-migor/setup.c
> > @@ -312,6 +312,14 @@ static void camera_power_off(void)
> >  	ctrl_outb(ctrl_inb(PORT_PTDR) & ~0x08, PORT_PTDR);
> >  }
> >  
> > +static void camera_power(int mode)
> > +{
> > +	if (mode)
> > +		camera_power_on();
> > +	else
> > +		camera_power_off();
> > +}
> > +
> >  #ifdef CONFIG_I2C
> >  static unsigned char camera_ov772x_magic[] > >  {
> > @@ -391,6 +399,7 @@ static struct soc_camera_platform_info ov772x_info = {
> >  	},
> >  	.bus_param =  SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
> >  	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8,
> > +	.power = camera_power,
> >  	.set_capture = ov772x_set_capture,
> >  };
> >  
> > @@ -405,8 +414,6 @@ static struct platform_device migor_camera_device = {
> >  static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
> >  	.flags = SOCAM_MASTER | SOCAM_DATAWIDTH_8 | SOCAM_PCLK_SAMPLE_RISING \
> >  	| SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH,
> > -	.enable_camera = camera_power_on,
> > -	.disable_camera = camera_power_off,
> >  };
> >  
> I don't like this. Keeping the enable and disable_camera interfaces
> is really the way we want to go, evident by the fact that your new power
> callback is forced to call in to one or the other anyways. I would rather
> see these moved in to struct soc_camera_platform_info, rather than having
> every single board in existence have to model that if (mode) on; else off
> crap.

The v4l counterpart of this patch has already been accepted. And since 
this is a compilation breakage fix, I think it would be better to apply it 
as is now, and then we can discuss better APIs.

Eventually, these power callback should disappear from struct 
soc_camera_platform_info and it should instead inclde struct 
soc_camera_link, which already has it. Then we can split "power" into a 
disable / enable pair.

> Also, there is not much point in splitting these changes out. They are
> coupled, and splitting them out only causes confusion (especially across
> a bisect).

This is why I originally have sent it as a single patch. But since I have 
got no acks I decided it would be easier to split it and have parts 
committed separately.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [PATCH] soc-camera: fix compile breakage on SH

[Paul Mundt]

On Mon, Oct 20, 2008 at 09:02:26AM +0200, Guennadi Liakhovetski wrote:
> On Mon, 20 Oct 2008, Paul Mundt wrote:
> > I don't like this. Keeping the enable and disable_camera interfaces
> > is really the way we want to go, evident by the fact that your new power
> > callback is forced to call in to one or the other anyways. I would rather
> > see these moved in to struct soc_camera_platform_info, rather than having
> > every single board in existence have to model that if (mode) on; else off
> > crap.
> 
> The v4l counterpart of this patch has already been accepted. And since 
> this is a compilation breakage fix, I think it would be better to apply it 
> as is now, and then we can discuss better APIs.
> 
I don't see how you can call this a fix, since it doesn't actually fix
the situation unless both parts are applied at the same time.

> Eventually, these power callback should disappear from struct 
> soc_camera_platform_info and it should instead inclde struct 
> soc_camera_link, which already has it. Then we can split "power" into a 
> disable / enable pair.
> 
That sounds fine.

> > Also, there is not much point in splitting these changes out. They are
> > coupled, and splitting them out only causes confusion (especially across
> > a bisect).
> 
> This is why I originally have sent it as a single patch. But since I have 
> got no acks I decided it would be easier to split it and have parts 
> committed separately.

I am not interested in merging something that is broken until the v4l
tree syncs up. If the v4l part is already merged, then we don't have a
lot of choice in what to do on the platform side. I will take your
original patch that touches both the platform and the v4l code and merge
that, then later when the v4l code tries to sync up git merging should do
the right thing.

In the future, if you do not immediately get acks, please do not assume
it is because there is a problem with your approach and that you should
suddenly try random different approaches until you start getting
feedback. In my case I was traveling and was way behind on email.

Splitting patches up that depend on each other is not acceptable, since
you introduce breakage as soon as you change the API and fail to update
the users at the same time. Subsystems should absolutely not be merging
those sorts of changes, since this causes nothing but pain for the users
that only find out about the changes when the build blows up, as seems to
have been the case in the initial merge also.

Re: [PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

On Mon, 20 Oct 2008, Paul Mundt wrote:

> On Mon, Oct 20, 2008 at 09:02:26AM +0200, Guennadi Liakhovetski wrote:
> > 
> > The v4l counterpart of this patch has already been accepted. And since 
> > this is a compilation breakage fix, I think it would be better to apply it 
> > as is now, and then we can discuss better APIs.
> > 
> I don't see how you can call this a fix, since it doesn't actually fix
> the situation unless both parts are applied at the same time.

It will be fixed _after_ both parts are committed, even if not at the same 
time.

> > This is why I originally have sent it as a single patch. But since I have 
> > got no acks I decided it would be easier to split it and have parts 
> > committed separately.
> 
> I am not interested in merging something that is broken until the v4l
> tree syncs up. If the v4l part is already merged, then we don't have a
> lot of choice in what to do on the platform side.

Last I checked - just before sending the previous email - it was not yet 
committed, but it is racy, you know:-)

> I will take your
> original patch that touches both the platform and the v4l code and merge
> that, then later when the v4l code tries to sync up git merging should do
> the right thing.

Will it? ok, didn't know that. Then it's ok indeed.

> In the future, if you do not immediately get acks, please do not assume
> it is because there is a problem with your approach and that you should
> suddenly try random different approaches until you start getting
> feedback. In my case I was traveling and was way behind on email.

I knew that, and the reason I splitted it was not because I assumed it was 
bad, but because the initial intention to get it fixed quickly didn't work 
out, then I decided to go the "normal" path.

> Splitting patches up that depend on each other is not acceptable, since
> you introduce breakage as soon as you change the API and fail to update
> the users at the same time. Subsystems should absolutely not be merging
> those sorts of changes, since this causes nothing but pain for the users
> that only find out about the changes when the build blows up, as seems to
> have been the case in the initial merge also.

I don't think all subsystem maintainers would agree with you here. I have 
been explicitly asked before to split such patches and just explain in the 
dependent patch, that it should be applied after the one it depends upon.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [PATCH] soc-camera: fix compile breakage on SH

[Paul Mundt]

On Mon, Oct 20, 2008 at 03:00:25PM +0200, Guennadi Liakhovetski wrote:
> On Mon, 20 Oct 2008, Paul Mundt wrote:
> > I don't see how you can call this a fix, since it doesn't actually fix
> > the situation unless both parts are applied at the same time.
> 
> It will be fixed _after_ both parts are committed, even if not at the same 
> time.
> 
_after_ = broken bisection, so therefore it is as good as no fix.

> > > This is why I originally have sent it as a single patch. But since I have 
> > > got no acks I decided it would be easier to split it and have parts 
> > > committed separately.
> > 
> > I am not interested in merging something that is broken until the v4l
> > tree syncs up. If the v4l part is already merged, then we don't have a
> > lot of choice in what to do on the platform side.
> 
> Last I checked - just before sending the previous email - it was not yet 
> committed, but it is racy, you know:-)
> 
Then I will merge the original in my tree and we can then focus on
getting the API changed properly, and making an incremental transition
that doesn't break all of the existing users in the process.

> > I will take your
> > original patch that touches both the platform and the v4l code and merge
> > that, then later when the v4l code tries to sync up git merging should do
> > the right thing.
> 
> Will it? ok, didn't know that. Then it's ok indeed.
> 
Yes, unfortunately this results in reproduced metadata, but it is a
necessary evil. This sort of situation often arises with platform data
API changes, so you see it quite frequently in architecture code. This
sort of bump in the workflow is necessary in order not to break the tree
however.

> > Splitting patches up that depend on each other is not acceptable, since
> > you introduce breakage as soon as you change the API and fail to update
> > the users at the same time. Subsystems should absolutely not be merging
> > those sorts of changes, since this causes nothing but pain for the users
> > that only find out about the changes when the build blows up, as seems to
> > have been the case in the initial merge also.
> 
> I don't think all subsystem maintainers would agree with you here. I have 
> been explicitly asked before to split such patches and just explain in the 
> dependent patch, that it should be applied after the one it depends upon.
> 
Yeah, the subsystem maintainers that don't have their platform broken
because of the API changes would tend not to have a strong opinion one
way or the other. In the case where there are lots of users all over the
place, one usually makes a best effort to get those all converted at the
same time, but obviously some fall through the cracks, and we deal with
that breakage as it happens. In this case there are very very few users,
so this simply should not have happened.

If subsystem people want to argue that breaking bisection is perfectly
acceptable, lets move this to linux-kernel and make sure Linus is on the
Cc.

Making incremental dependent patches is perfectly fine, as long as you do
not break the state of the tree in between any of those incremental
changes. If you are doing an API change you do need to get the user
updates in the same update, if you are not providing backwards
compatability for the old behaviour. It is never acceptable to willfully
break the tree in the name of having something split out.

Re: [PATCH] soc-camera: fix compile breakage on SH

[Guennadi Liakhovetski]

On Mon, 20 Oct 2008, Paul Mundt wrote:

> > Last I checked - just before sending the previous email - it was not yet 
> > committed, but it is racy, you know:-)
> > 
> Then I will merge the original in my tree and we can then focus on
> getting the API changed properly, and making an incremental transition
> that doesn't break all of the existing users in the process.

Shall I ask Mauro to not merge the v4l part? It might still work...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [PATCH] soc-camera: fix compile breakage on SH

[Paul Mundt]

On Mon, Oct 20, 2008 at 07:44:09PM +0200, Guennadi Liakhovetski wrote:
> On Mon, 20 Oct 2008, Paul Mundt wrote:
> 
> > > Last I checked - just before sending the previous email - it was not yet 
> > > committed, but it is racy, you know:-)
> > > 
> > Then I will merge the original in my tree and we can then focus on
> > getting the API changed properly, and making an incremental transition
> > that doesn't break all of the existing users in the process.
> 
> Shall I ask Mauro to not merge the v4l part? It might still work...
> 
Too late now, so I'll just merge the SH part separately. Please just try
to keep this in mind in the future so we do not have the same problem
again.