[PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Antonio Ospite]

Hi,

with the attached patch I can work around a problem I had adding camera
support to the Motorola A780 phone, but I again don't know if it is a
proper fix. Could you help me to find out, please?

Using the same pxa-camera working setup I have for A910 (the other
phone which I am working on) I got incorrect data, see[1]. It turned
out to be because pxa CIF was getting data on pixelclock rising edge
but the sensor hardware on A780 wanted otherwise. So I tried to set
PXA_CAMERA_PCP in pxacamera_platform_data flags, but the sensor bus
config prevented pxa-camera to honour this setting. With the attached
patch I can set PCP high in pxa-camera and get the right data[2] out of
CIF, using this platform config:

struct pxacamera_platform_data a780_pxacamera_platform_data = {
	.init	= a780_pxacamera_init,
	.flags  = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
		PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN |
		PXA_CAMERA_PCP,
	.mclk_10khz = 1000,
};

Now I have many questions:

* Can the same sensor model have different default hardwired values?
  I am referring to IO/Timings differences between mt9m111 on A910
  and A780
* Should I change the sensor setup instead of changing its advertised
  capabilities? Maybe modifying mt9m111 so it can use platform data?
* Is the pxa-camera code dealing with PXA_CAMERA_PCP too conservative?
  Shouldn't PXA_CAMERA_PCP be independent from the specific sensor
  capabilities? it is a valid pxa-camera setting even if it produces
  wrong results with the specific sensor.

Note that the two phones set up a different set of GPIOs as data
lines between mt9m111 and pxa.

Please let me know if you need more details.

Thanks,
   Antonio Ospite

[1] http://people.openezx.org/ao2/a780-not-yet.jpg
[2] http://people.openezx.org/ao2/a780-finally-works.jpg


Pixeldata can be sent on pixelclock falling edge, allow this option in
mt9m111 so pxa-camera can honour PXA_CAMERA_PCP platform data flag.

See pxa-camera.c, pxa_camera_set_bus_param():

	if ((common_flags & SOCAM_PCLK_SAMPLE_RISING) &&
	    (common_flags & SOCAM_PCLK_SAMPLE_FALLING)) {
		if (pcdev->platform_flags & PXA_CAMERA_PCP)
			common_flags &= ~SOCAM_PCLK_SAMPLE_RISING;
		else
			common_flags &= ~SOCAM_PCLK_SAMPLE_FALLING;
	}

...

	if (common_flags & SOCAM_PCLK_SAMPLE_FALLING)
		cicr4 |= CICR4_PCP;


I needed this patch to make camera work properly on Motorola A780 phone.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 76fb0cb..69e103c 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -410,7 +410,8 @@ static int mt9m111_stop_capture(struct soc_camera_device *icd)

 static unsigned long mt9m111_query_bus_param(struct soc_camera_device *icd)
 {
- return SOCAM_MASTER | SOCAM_PCLK_SAMPLE_RISING |
+ return SOCAM_MASTER |
+   SOCAM_PCLK_SAMPLE_RISING | SOCAM_PCLK_SAMPLE_FALLING |
    SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH |
    SOCAM_DATAWIDTH_8;
 }

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Robert Jarzmik]

Antonio Ospite <ospite@studenti.unina.it> writes:

> Now I have many questions:
>
> * Can the same sensor model have different default hardwired values?
>   I am referring to IO/Timings differences between mt9m111 on A910
>   and A780
Technically, yes.
Even the sensor can sometimes be configured to dump its date on falling edge
rather than rising edge. See MT9M111 datasheet, register 0x13a (Output Format
Control 2), bit 9.

> * Should I change the sensor setup instead of changing its advertised
>   capabilities? Maybe modifying mt9m111 so it can use platform data?
Would't it be better to change format negociation instead : patch in mt9m111.c
the mt9m111_query_bus_param() function, add SOCAM_PCLK_SAMPLE_FALLING, and add
necessary handling in the mt9m111_set_bus_param() ? That would be a little
extension of your attached patch ...

> * Is the pxa-camera code dealing with PXA_CAMERA_PCP too conservative?
>   Shouldn't PXA_CAMERA_PCP be independent from the specific sensor
>   capabilities? it is a valid pxa-camera setting even if it produces
>   wrong results with the specific sensor.
Well, I don't understand something here : you have to configure the sensor to
output data on rising edge, while the PXA is reading them on the falling edge,
am I right ? Would that mean the clock signal is inverted by the hardware ? I
don't really understand that part ...

> @@ -410,7 +410,8 @@ static int mt9m111_stop_capture(struct soc_camera_device *icd)
>
>  static unsigned long mt9m111_query_bus_param(struct soc_camera_device *icd)
>  {
> - return SOCAM_MASTER | SOCAM_PCLK_SAMPLE_RISING |
> + return SOCAM_MASTER |
> +   SOCAM_PCLK_SAMPLE_RISING | SOCAM_PCLK_SAMPLE_FALLING |
>     SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH |
>     SOCAM_DATAWIDTH_8;
>  }
Don't forget mt9m111_set_bus_param(), and add an entry in struct mt9m111 to
remember the setting on suspend/resume. Amend accordingly mt9m111_setup_pixfmt()
with the new field in mt9m111_set_bus_param().

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Guennadi Liakhovetski]

On Fri, 7 Nov 2008, Robert Jarzmik wrote:

> Antonio Ospite <ospite@studenti.unina.it> writes:
> 
> > Now I have many questions:
> >
> > * Can the same sensor model have different default hardwired values?
> >   I am referring to IO/Timings differences between mt9m111 on A910
> >   and A780
> Technically, yes.
> Even the sensor can sometimes be configured to dump its date on falling edge
> rather than rising edge. See MT9M111 datasheet, register 0x13a (Output Format
> Control 2), bit 9.

Also register 0x00a is intersting...

> > * Should I change the sensor setup instead of changing its advertised
> >   capabilities? Maybe modifying mt9m111 so it can use platform data?
> Would't it be better to change format negociation instead : patch in mt9m111.c
> the mt9m111_query_bus_param() function, add SOCAM_PCLK_SAMPLE_FALLING, and add
> necessary handling in the mt9m111_set_bus_param() ? That would be a little
> extension of your attached patch ...

Yes, that would be correct, but, it seems, it would then stop working 
again, see below.

> > * Is the pxa-camera code dealing with PXA_CAMERA_PCP too conservative?
> >   Shouldn't PXA_CAMERA_PCP be independent from the specific sensor
> >   capabilities? it is a valid pxa-camera setting even if it produces
> >   wrong results with the specific sensor.
> Well, I don't understand something here : you have to configure the sensor to
> output data on rising edge, while the PXA is reading them on the falling edge,
> am I right ? Would that mean the clock signal is inverted by the hardware ? I
> don't really understand that part ...

That's also the only explanation I can see here too... I was actually 
wondering as I was developing the framework, if anyone ever would come up 
with an idea to put inverters on any of sync / clock lines or any other 
additional logic. Ok, you can configure inverters with extra platform 
flags, but can we really add enough flags to support any possible 
camera-interface design?... I am not a hardware designer, so, I have no 
idea what other configurations one can think of here. Shall we really add 
flags for inverters on all interface lines and hope noone will ever 
engineer anything more complex than that?

> > @@ -410,7 +410,8 @@ static int mt9m111_stop_capture(struct soc_camera_device *icd)
> >
> >  static unsigned long mt9m111_query_bus_param(struct soc_camera_device *icd)
> >  {
> > - return SOCAM_MASTER | SOCAM_PCLK_SAMPLE_RISING |
> > + return SOCAM_MASTER |
> > +   SOCAM_PCLK_SAMPLE_RISING | SOCAM_PCLK_SAMPLE_FALLING |
> >     SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH |
> >     SOCAM_DATAWIDTH_8;
> >  }
> Don't forget mt9m111_set_bus_param(), and add an entry in struct mt9m111 to
> remember the setting on suspend/resume. Amend accordingly mt9m111_setup_pixfmt()
> with the new field in mt9m111_set_bus_param().

I think, it currently works thanks to this code in pxa_camera.c:

	if ((common_flags & SOCAM_PCLK_SAMPLE_RISING) &&
	    (common_flags & SOCAM_PCLK_SAMPLE_FALLING)) {
		if (pcdev->platform_flags & PXA_CAMERA_PCP)
			common_flags &= ~SOCAM_PCLK_SAMPLE_RISING;
		else
			common_flags &= ~SOCAM_PCLK_SAMPLE_FALLING;
	}

i.e., if both sensor and host support both polarities take what's 
suggested by the platform. That's, probably, why Antonio's patch helped. 
But, if you also add flag handling to mt9m111_set_bus_param(), it will 
invert the pixel clock too, and it will stop working again... It's a pity 
we'll, probably, never see schematics of the phone:-)

So, shall we add inverter flags?

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

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 5253 bytes --]

On Sat, 8 Nov 2008 21:36:46 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Fri, 7 Nov 2008, Robert Jarzmik wrote:
> 
> > Antonio Ospite <ospite@studenti.unina.it> writes:
> > 
> > > Now I have many questions:
> > >
> > > * Can the same sensor model have different default hardwired values?
> > >   I am referring to IO/Timings differences between mt9m111 on A910
> > >   and A780
> > Technically, yes.
> > Even the sensor can sometimes be configured to dump its date on falling edge
> > rather than rising edge. See MT9M111 datasheet, register 0x13a (Output Format
> > Control 2), bit 9.
> 
> Also register 0x00a is intersting...
>

Well, the two sensors have indeed almost the same factory config, in
particular are equal:

ROW_SPEED(0x00a): 0x0011
OUTPUT_FORMAT_CTRL2_A(0x13a): 0x0200
OUTPUT_FORMAT_CTRL2_B(0x19b): 0x0200

only some _reserved_ registers have different factory defaults. I can
provide mt9m111 registers dump of both phones if you are interested.

> > > * Should I change the sensor setup instead of changing its advertised
> > >   capabilities? Maybe modifying mt9m111 so it can use platform data?
> > Would't it be better to change format negociation instead : patch in mt9m111.c
> > the mt9m111_query_bus_param() function, add SOCAM_PCLK_SAMPLE_FALLING, and add
> > necessary handling in the mt9m111_set_bus_param() ? That would be a little
> > extension of your attached patch ...
> 
> Yes, that would be correct, but, it seems, it would then stop working 
> again, see below.
>

I guess so.

> > > * Is the pxa-camera code dealing with PXA_CAMERA_PCP too conservative?
> > >   Shouldn't PXA_CAMERA_PCP be independent from the specific sensor
> > >   capabilities? it is a valid pxa-camera setting even if it produces
> > >   wrong results with the specific sensor.
> > Well, I don't understand something here : you have to configure the sensor to
> > output data on rising edge, while the PXA is reading them on the falling edge,
> > am I right ? Would that mean the clock signal is inverted by the hardware ? I
> > don't really understand that part ...
> 
> That's also the only explanation I can see here too... I was actually 
> wondering as I was developing the framework, if anyone ever would come up 
> with an idea to put inverters on any of sync / clock lines or any other 
> additional logic. Ok, you can configure inverters with extra platform 
> flags, but can we really add enough flags to support any possible 
> camera-interface design?... I am not a hardware designer, so, I have no 
> idea what other configurations one can think of here. Shall we really add 
> flags for inverters on all interface lines and hope noone will ever 
> engineer anything more complex than that?
>

A smarter solution would be preferred indeed.

> > > @@ -410,7 +410,8 @@ static int mt9m111_stop_capture(struct soc_camera_device *icd)
> > >
> > >  static unsigned long mt9m111_query_bus_param(struct soc_camera_device *icd)
> > >  {
> > > - return SOCAM_MASTER | SOCAM_PCLK_SAMPLE_RISING |
> > > + return SOCAM_MASTER |
> > > +   SOCAM_PCLK_SAMPLE_RISING | SOCAM_PCLK_SAMPLE_FALLING |
> > >     SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH |
> > >     SOCAM_DATAWIDTH_8;
> > >  }
[...]
> 
> I think, it currently works thanks to this code in pxa_camera.c:
> 
> 	if ((common_flags & SOCAM_PCLK_SAMPLE_RISING) &&
> 	    (common_flags & SOCAM_PCLK_SAMPLE_FALLING)) {
> 		if (pcdev->platform_flags & PXA_CAMERA_PCP)
> 			common_flags &= ~SOCAM_PCLK_SAMPLE_RISING;
> 		else
> 			common_flags &= ~SOCAM_PCLK_SAMPLE_FALLING;
> 	}
> 
> i.e., if both sensor and host support both polarities take what's 
> suggested by the platform. That's, probably, why Antonio's patch helped. 
> But, if you also add flag handling to mt9m111_set_bus_param(), it will 
> invert the pixel clock too, and it will stop working again... It's a pity 
> we'll, probably, never see schematics of the phone:-)
>

A real pity. I don't have access to those schematic either as you may
guess.

> So, shall we add inverter flags?
>

Would you accept this change instead?

--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -845,7 +845,7 @@ static int pxa_camera_set_bus_param(struct
soc_camera_device *icd, __u32 pixfmt) cicr4 |= CICR4_PCLK_EN;
  if (pcdev->platform_flags & PXA_CAMERA_MCLK_EN)
    cicr4 |= CICR4_MCLK_EN;
- if (common_flags & SOCAM_PCLK_SAMPLE_FALLING)
+ if (pcdev->platform_flags & PXA_CAMERA_PCP)
    cicr4 |= CICR4_PCP;
  if (common_flags & SOCAM_HSYNC_ACTIVE_LOW)
    cicr4 |= CICR4_HSP;

and maybe for other cicr4 bits too, in the spirit of using the SOCAM_
defines only for icd set_bus_param() but still giving preference to
platform data for host settings.

It is kind of tricky I know, but it would allow to overcome unexpected
hardware setups.

Regards,
   Antonio

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Guennadi Liakhovetski]

(please, use reply-to-all, thanks)

On Sun, 9 Nov 2008, Antonio Ospite wrote:

> > So, shall we add inverter flags?
> 
> Would you accept this change instead?
> 
> --- a/drivers/media/video/pxa_camera.c
> +++ b/drivers/media/video/pxa_camera.c
> @@ -845,7 +845,7 @@ static int pxa_camera_set_bus_param(struct
> soc_camera_device *icd, __u32 pixfmt) cicr4 |= CICR4_PCLK_EN;
>   if (pcdev->platform_flags & PXA_CAMERA_MCLK_EN)
>     cicr4 |= CICR4_MCLK_EN;
> - if (common_flags & SOCAM_PCLK_SAMPLE_FALLING)
> + if (pcdev->platform_flags & PXA_CAMERA_PCP)
>     cicr4 |= CICR4_PCP;
>   if (common_flags & SOCAM_HSYNC_ACTIVE_LOW)
>     cicr4 |= CICR4_HSP;
> 
> and maybe for other cicr4 bits too, in the spirit of using the SOCAM_
> defines only for icd set_bus_param() but still giving preference to
> platform data for host settings.
> 
> It is kind of tricky I know, but it would allow to overcome unexpected
> hardware setups.

I would prefer not to disregard camera flags. If we don't find a better 
solution, I would introduce platform inverter flags, and, I think, we 
better put them in camera platform data - not host platform data, to 
provide a finer granularity. In the end, inverters can also be located on 
camera boards, then you plug-in a different camera and, if your 
inverter-flags were in host platform data, it doesn't work again.

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

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> I would prefer not to disregard camera flags. If we don't find a better 
> solution, I would introduce platform inverter flags, and, I think, we 
> better put them in camera platform data - not host platform data, to 
> provide a finer granularity. In the end, inverters can also be located on 
> camera boards, then you plug-in a different camera and, if your 
> inverter-flags were in host platform data, it doesn't work again.
I'm of the same opinion.

I was thinking of another case : imagine the host needs to be configured on
rising edge, and camera on falling edge. Your patch wouldn't cover that devious
case.

I can't think of a better solution than an inverter flag as well. As this would
be very board specific, let it go in something board code sets up.

That's how it's already done for inverted gpio Vbus sensing in the USB stack for
the pxa for example.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 1576 bytes --]

On Mon, 10 Nov 2008 20:06:34 +0100
Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > I would prefer not to disregard camera flags. If we don't find a better 
> > solution, I would introduce platform inverter flags, and, I think, we 
> > better put them in camera platform data - not host platform data, to 
> > provide a finer granularity. In the end, inverters can also be located on 
> > camera boards, then you plug-in a different camera and, if your 
> > inverter-flags were in host platform data, it doesn't work again.
>
> I'm of the same opinion.
> 
> I was thinking of another case : imagine the host needs to be configured on
> rising edge, and camera on falling edge. Your patch wouldn't cover that devious
> case.
> 
> I can't think of a better solution than an inverter flag as well. As this would
> be very board specific, let it go in something board code sets up.
> 
> That's how it's already done for inverted gpio Vbus sensing in the USB stack for
> the pxa for example.
> 

Ok, I hope you'll find time to add the proper solution some day, since I
don't think I can do it correctly with my current knowledge.

Thanks for the interest.

Best regards,
   Antonio Ospite

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Guennadi Liakhovetski]

Hi Antonio,

On Wed, 12 Nov 2008, Antonio Ospite wrote:

> On Mon, 10 Nov 2008 20:06:34 +0100
> Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> 
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> > 
> > > I would prefer not to disregard camera flags. If we don't find a better 
> > > solution, I would introduce platform inverter flags, and, I think, we 
> > > better put them in camera platform data - not host platform data, to 
> > > provide a finer granularity. In the end, inverters can also be located on 
> > > camera boards, then you plug-in a different camera and, if your 
> > > inverter-flags were in host platform data, it doesn't work again.
> >
> > I'm of the same opinion.
> > 
> > I was thinking of another case : imagine the host needs to be configured on
> > rising edge, and camera on falling edge. Your patch wouldn't cover that devious
> > case.
> > 
> > I can't think of a better solution than an inverter flag as well. As this would
> > be very board specific, let it go in something board code sets up.
> > 
> > That's how it's already done for inverted gpio Vbus sensing in the USB stack for
> > the pxa for example.
> > 
> 
> Ok, I hope you'll find time to add the proper solution some day, since I
> don't think I can do it correctly with my current knowledge.

Could you test the patch below? It applies on top of all my patches I 
pushed today plus a couple more that are still to be pushed... But maybe 
you can apply it to linux-next manually. You just need the parts for 
soc_camera.h and for mt9m111. And then you need to add to your struct 
soc_camera_link in platform data:

	.flags = SOCAM_SENSOR_INVERT_PCLK,

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


diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
index 0bcfef7..b58f0f8 100644
--- a/drivers/media/video/mt9m001.c
+++ b/drivers/media/video/mt9m001.c
@@ -272,17 +272,16 @@ static int mt9m001_set_bus_param(struct soc_camera_device *icd,
 static unsigned long mt9m001_query_bus_param(struct soc_camera_device *icd)
 {
 	struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
-	unsigned int width_flag = SOCAM_DATAWIDTH_10;
+	struct soc_camera_link *icl = mt9m001->client->dev.platform_data;
+	/* MT9M001 has all capture_format parameters fixed */
+	unsigned long flags = SOCAM_DATAWIDTH_10 | SOCAM_PCLK_SAMPLE_RISING |
+		SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH |
+		SOCAM_MASTER;
 
 	if (bus_switch_possible(mt9m001))
-		width_flag |= SOCAM_DATAWIDTH_8;
+		flags |= SOCAM_DATAWIDTH_8;
 
-	/* MT9M001 has all capture_format parameters fixed */
-	return SOCAM_PCLK_SAMPLE_RISING |
-		SOCAM_HSYNC_ACTIVE_HIGH |
-		SOCAM_VSYNC_ACTIVE_HIGH |
-		SOCAM_MASTER |
-		width_flag;
+	return soc_camera_apply_sensor_flags(icl, flags);
 }
 
 static int mt9m001_set_fmt(struct soc_camera_device *icd,
@@ -578,6 +577,7 @@ static int mt9m001_set_control(struct soc_camera_device *icd, struct v4l2_contro
 static int mt9m001_video_probe(struct soc_camera_device *icd)
 {
 	struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
+	struct soc_camera_link *icl = mt9m001->client->dev.platform_data;
 	s32 data;
 	int ret;
 
@@ -600,7 +600,7 @@ static int mt9m001_video_probe(struct soc_camera_device *icd)
 	case 0x8421:
 		mt9m001->model = V4L2_IDENT_MT9M001C12ST;
 		icd->formats = mt9m001_colour_formats;
-		if (mt9m001->client->dev.platform_data)
+		if (gpio_is_valid(icl->gpio))
 			icd->num_formats = ARRAY_SIZE(mt9m001_colour_formats);
 		else
 			icd->num_formats = 1;
@@ -608,7 +608,7 @@ static int mt9m001_video_probe(struct soc_camera_device *icd)
 	case 0x8431:
 		mt9m001->model = V4L2_IDENT_MT9M001C12STM;
 		icd->formats = mt9m001_monochrome_formats;
-		if (mt9m001->client->dev.platform_data)
+		if (gpio_is_valid(icl->gpio))
 			icd->num_formats = ARRAY_SIZE(mt9m001_monochrome_formats);
 		else
 			icd->num_formats = 1;
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index b4a238f..b0e6046 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -415,9 +415,13 @@ static int mt9m111_stop_capture(struct soc_camera_device *icd)
 
 static unsigned long mt9m111_query_bus_param(struct soc_camera_device *icd)
 {
-	return SOCAM_MASTER | SOCAM_PCLK_SAMPLE_RISING |
+	struct mt9m111 *mt9m111 = container_of(icd, struct mt9m111, icd);
+	struct soc_camera_link *icl = mt9m111->client->dev.platform_data;
+	unsigned long flags = SOCAM_MASTER | SOCAM_PCLK_SAMPLE_RISING |
 		SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_HIGH |
 		SOCAM_DATAWIDTH_8;
+
+	return soc_camera_apply_sensor_flags(icl, flags);
 }
 
 static int mt9m111_set_bus_param(struct soc_camera_device *icd, unsigned long f)
diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 3a39f02..3b3a6a0 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -273,6 +273,7 @@ static int mt9v022_set_bus_param(struct soc_camera_device *icd,
 				 unsigned long flags)
 {
 	struct mt9v022 *mt9v022 = container_of(icd, struct mt9v022, icd);
+	struct soc_camera_link *icl = mt9v022->client->dev.platform_data;
 	unsigned int width_flag = flags & SOCAM_DATAWIDTH_MASK;
 	int ret;
 	u16 pixclk = 0;
@@ -296,6 +297,8 @@ static int mt9v022_set_bus_param(struct soc_camera_device *icd,
 		mt9v022->datawidth = width_flag == SOCAM_DATAWIDTH_8 ? 8 : 10;
 	}
 
+	flags = soc_camera_apply_sensor_flags(icl, flags);
+
 	if (flags & SOCAM_PCLK_SAMPLE_RISING)
 		pixclk |= 0x10;
 
@@ -690,6 +693,7 @@ static int mt9v022_set_control(struct soc_camera_device *icd,
 static int mt9v022_video_probe(struct soc_camera_device *icd)
 {
 	struct mt9v022 *mt9v022 = container_of(icd, struct mt9v022, icd);
+	struct soc_camera_link *icl = mt9v022->client->dev.platform_data;
 	s32 data;
 	int ret;
 
@@ -725,7 +729,7 @@ static int mt9v022_video_probe(struct soc_camera_device *icd)
 		ret = reg_write(icd, MT9V022_PIXEL_OPERATION_MODE, 4 | 0x11);
 		mt9v022->model = V4L2_IDENT_MT9V022IX7ATC;
 		icd->formats = mt9v022_colour_formats;
-		if (mt9v022->client->dev.platform_data)
+		if (gpio_is_valid(icl->gpio))
 			icd->num_formats = ARRAY_SIZE(mt9v022_colour_formats);
 		else
 			icd->num_formats = 1;
@@ -733,7 +737,7 @@ static int mt9v022_video_probe(struct soc_camera_device *icd)
 		ret = reg_write(icd, MT9V022_PIXEL_OPERATION_MODE, 0x11);
 		mt9v022->model = V4L2_IDENT_MT9V022IX7ATM;
 		icd->formats = mt9v022_monochrome_formats;
-		if (mt9v022->client->dev.platform_data)
+		if (gpio_is_valid(icl->gpio))
 			icd->num_formats = ARRAY_SIZE(mt9v022_monochrome_formats);
 		else
 			icd->num_formats = 1;
diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index c3e7139..0209462 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -706,12 +706,12 @@ static int ov772x_set_bus_param(struct soc_camera_device *icd,
 static unsigned long ov772x_query_bus_param(struct soc_camera_device *icd)
 {
 	struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
-
-	return  SOCAM_PCLK_SAMPLE_RISING |
-		SOCAM_HSYNC_ACTIVE_HIGH  |
-		SOCAM_VSYNC_ACTIVE_HIGH  |
-		SOCAM_MASTER             |
+	struct soc_camera_link *icl = priv->client->dev.platform_data;
+	unsigned long flags = SOCAM_PCLK_SAMPLE_RISING | SOCAM_MASTER |
+		SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_HSYNC_ACTIVE_HIGH |
 		priv->info->buswidth;
+
+	return soc_camera_apply_sensor_flags(icl, flags);
 }
 
 static int ov772x_get_chip_id(struct soc_camera_device *icd,
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index d1f7241..549d0c2 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -81,11 +81,19 @@ struct soc_camera_host_ops {
 	unsigned int (*poll)(struct file *, poll_table *);
 };
 
+#define SOCAM_SENSOR_INVERT_PCLK	(1 << 0)
+#define SOCAM_SENSOR_INVERT_MCLK	(1 << 1)
+#define SOCAM_SENSOR_INVERT_HSYNC	(1 << 2)
+#define SOCAM_SENSOR_INVERT_VSYNC	(1 << 3)
+#define SOCAM_SENSOR_INVERT_DATA	(1 << 4)
+
 struct soc_camera_link {
 	/* Camera bus id, used to match a camera and a bus */
 	int bus_id;
 	/* GPIO number to switch between 8 and 10 bit modes */
 	unsigned int gpio;
+	/* Per camera SOCAM_SENSOR_* bus flags */
+	unsigned long flags;
 	/* Optional callbacks to power on or off and reset the sensor */
 	int (*power)(struct device *, int);
 	int (*reset)(struct device *);
@@ -206,4 +214,26 @@ static inline unsigned long soc_camera_bus_param_compatible(
 	return (!hsync || !vsync || !pclk) ? 0 : common_flags;
 }
 
+/**
+ * Return flags modified according to SOCAM_SENSOR_* platform configuration
+ *
+ * @icl:	camera platform parameters
+ * @flags:	flags to be inverted according to platform configuration
+ * @return:	resulting flags
+ */
+static inline unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl,
+							  unsigned long flags)
+{
+	if (icl->flags & SOCAM_SENSOR_INVERT_HSYNC)
+		flags ^= SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_HSYNC_ACTIVE_LOW;
+
+	if (icl->flags & SOCAM_SENSOR_INVERT_VSYNC)
+		flags ^= SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_VSYNC_ACTIVE_LOW;
+
+	if (icl->flags & SOCAM_SENSOR_INVERT_PCLK)
+		flags ^= SOCAM_PCLK_SAMPLE_RISING | SOCAM_PCLK_SAMPLE_FALLING;
+
+	return flags;
+}
+
 #endif

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 1548 bytes --]

On Mon, 1 Dec 2008 21:54:17 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> Hi Antonio,
> 
> On Wed, 12 Nov 2008, Antonio Ospite wrote:
> 
> > On Mon, 10 Nov 2008 20:06:34 +0100
> > Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > 
> >
> > > 
> > > I can't think of a better solution than an inverter flag as well. As this would
> > > be very board specific, let it go in something board code sets up.
> > > 
> > > That's how it's already done for inverted gpio Vbus sensing in the USB stack for
> > > the pxa for example.
> > > 
> > 
> > Ok, I hope you'll find time to add the proper solution some day, since I
> > don't think I can do it correctly with my current knowledge.
> 
> Could you test the patch below? It applies on top of all my patches I 
> pushed today plus a couple more that are still to be pushed... But maybe 
> you can apply it to linux-next manually. You just need the parts for 
> soc_camera.h and for mt9m111. And then you need to add to your struct 
> soc_camera_link in platform data:
> 
> 	.flags = SOCAM_SENSOR_INVERT_PCLK,
> 

Thanks Guennadi, I can't test it before week-end. If by then you have a
single patchset, please let me know.

Regards,
   Antonio Ospite

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH, RFC] mt9m111: allow data to be received on pixelclock falling edge?

[Antonio Ospite]

[-- Attachment #1.1: Type: text/plain, Size: 2332 bytes --]

On Mon, 1 Dec 2008 21:54:17 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> Hi Antonio,
> 
> On Wed, 12 Nov 2008, Antonio Ospite wrote:
> 
> > On Mon, 10 Nov 2008 20:06:34 +0100
> > Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> > 
> > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> > > 
> > > > I would prefer not to disregard camera flags. If we don't find a better 
> > > > solution, I would introduce platform inverter flags, and, I think, we 
> > > > better put them in camera platform data - not host platform data, to 
> > > > provide a finer granularity. In the end, inverters can also be located on 
> > > > camera boards, then you plug-in a different camera and, if your 
> > > > inverter-flags were in host platform data, it doesn't work again.
> > >
> > > I'm of the same opinion.
> > > 
> > > I was thinking of another case : imagine the host needs to be configured on
> > > rising edge, and camera on falling edge. Your patch wouldn't cover that devious
> > > case.
> > > 
> > > I can't think of a better solution than an inverter flag as well. As this would
> > > be very board specific, let it go in something board code sets up.
> > > 
> > > That's how it's already done for inverted gpio Vbus sensing in the USB stack for
> > > the pxa for example.
> > > 
> > 
> > Ok, I hope you'll find time to add the proper solution some day, since I
> > don't think I can do it correctly with my current knowledge.
> 
> Could you test the patch below? It applies on top of all my patches I 
> pushed today plus a couple more that are still to be pushed... But maybe 
> you can apply it to linux-next manually. You just need the parts for 
> soc_camera.h and for mt9m111. And then you need to add to your struct 
> soc_camera_link in platform data:
> 
> 	.flags = SOCAM_SENSOR_INVERT_PCLK,
> 
> Thanks
> Guennadi

Sorry for the absurdly late reply :)

I had the chanche to test this today and it works OK.

Thanks once again,
   Antonio Ospite

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

  Web site: http://www.studenti.unina.it/~ospite
Public key: http://www.studenti.unina.it/~ospite/aopubkey.asc

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 164 bytes --]

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list