On Wed, 8 Oct 2008 21:01:03 +0200 Bart Visscher <[EMAIL PROTECTED]> wrote:
> Hello everyone,
>
Hi Bart, pleased to hear from you.
Please, the next time make sure your email client wraps long lines
properly ;)
> I have been working on the camera support for the A780. The result is
> attached, the second patch, mt9m111.patch is the code i took from the
> V4L branch. Should i clean it up for including it in the EZX branch?
> The current version of the patch is only usefull for testing.
>
I've just given it a try, thanks for your work.
Did you make any changes to the sensor driver code?
About inclusion in openezx tree, I think we could wait the mt9m111
driver hit mainline, this should not be too far in the future, the
first 2.6.28 merge window will be opened pretty soon, and the v4l tree
should be merged then.
In the meantime we can concentrate cleaning up the ezx-camera patch.
> The first patch, ezx-camera.patch, are the changes needed to activate
> the camera on a A780 phone. I'm not sure what to do with the GPIO pin
> names, suggestions are welcome.
Let's give a look at the code below, comments inlined.
> I tested with the program 'motion',
> but the resulting pictures have the wrong colours and are split or
> something. But it does show the right shapes so the picture comes from
> the camera sensor.
>
Good anyway, let's find out what's wrong. Have you compared your
pxa-camera setup with the one from motorola 2.4? Actually the code
latest code we have shows a driver for mt9v111 sensor, I don't know if
there is any valuable difference with mt9m111.
> If you have some idea, send them to me, and i'll give them a try.
>
> Bart
> [ezx-camera.patch text/x-diff (2,6KB)]
> diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
> index 2b415e5..5b3e4b5 100644
> --- a/arch/arm/mach-pxa/ezx.c
> +++ b/arch/arm/mach-pxa/ezx.c
> @@ -23,6 +23,8 @@
> #include <linux/spi/spi.h>
> #include <linux/spi/mmc_spi.h>
>
> +#include <media/soc_camera.h>
> +
> #include <asm/setup.h>
> #include <asm/arch/pxafb.h>
> #include <asm/arch/ohci.h>
> @@ -36,6 +38,7 @@
> #include <asm/arch/udc.h>
> #include <asm/arch/pxa27x-udc.h>
> #include <asm/arch/pxa2xx_spi.h>
> +#include <asm/arch/camera.h>
> #include <asm/mach-types.h>
> #include <asm/mach/arch.h>
>
> @@ -540,8 +543,8 @@ static unsigned long gen1_pin_config[] __initdata = {
> GPIO94_CIF_DD_5,
> GPIO17_CIF_DD_6,
> GPIO108_CIF_DD_7,
> - GPIO50_GPIO, /* CAM_EN */
> - GPIO19_GPIO, /* CAM_RST */
> + GPIO50_GPIO|MFP_DIR_OUT, /* CAM_EN */
> + GPIO19_GPIO|MFP_DIR_OUT, /* CAM_RST */
>
> /* EMU */
> GPIO120_GPIO, /* EMU_MUX1 */
> @@ -703,6 +706,50 @@ static void __init ezx_fixup(struct machine_desc *desc,
> struct tag *tags,
> }
>
> #ifdef CONFIG_MACH_EZX_A780
> +static int a780_pxacamera_init(struct device *dev)
> +{
> + gpio_set_value(MFP_PIN_GPIO50, 0); // CAM_EN
I think you can either use GPIO50_GPIO or mfp_to_gpio(MFP_PIN_GPIO50)
here, so just choose one, I think the first form is more appropriate.
And please remove C++ style comments, use C comments
The same comments holds for the other gpio_set_value() calls.
> + gpio_set_value(MFP_PIN_GPIO19, 1); // CAM_RST
> + return 0;
> +}
> +
>
> +static int a780_pxacamera_power(struct device *dev, int on)
> +{
> + gpio_set_value(MFP_PIN_GPIO50, on?0:1); // CAM_EN
I'd put some space around the conditional expression here.
> + return 0;
> +}
> +
> +static int a780_pxacamera_reset(struct device *dev)
> +{
> + gpio_set_value(MFP_PIN_GPIO19, 1); // CAM_RST
> + return 0;
> +}
> +
> +/*
> + * CICR4: PCLK_EN: Pixel clock is supplied by the sensor
> + * MCLK_EN: Master clock is generated by PXA
> + * PCP: Data sampled on the falling edge of pixel clock
> + */
> +struct pxacamera_platform_data a780_pxacamera_platform_data = {
> + .init = a780_pxacamera_init,
> + .flags = PXA_CAMERA_MASTER |
> + PXA_CAMERA_DATAWIDTH_8 | PXA_CAMERA_DATAWIDTH_10 |
> + PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN/* | PXA_CAMERA_PCP*/,
> + .mclk_10khz = 1000,
> +};
Motorola code seems to only use DATAWIDTH_8, I don't know if that makes
any difference tho.
> +static struct soc_camera_link iclink = {
> + .bus_id = 0, /* Must match with the camera ID */
> + .power = a780_pxacamera_power,
> + .reset = a780_pxacamera_reset,
> +};
> +
when you say "camera ID", do you mean the ID of the bus which the
_sensor_ is attached to?
> +static struct i2c_board_info __initdata a780_i2c_board_info[] = {
> + {
> + I2C_BOARD_INFO("mt9m111", 0x5d),
> + .platform_data = &iclink,
> + },
> +};
> +
> static void __init a780_init(void)
> {
> pxa2xx_mfp_config(ARRAY_AND_SIZE(ezx_pin_config));
> @@ -730,6 +777,12 @@ static void __init a780_init(void)
> pxa_set_keypad_info(&a780_keypad_platform_data);
> #endif
>
> +#if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
> + pxa_set_camera_info(&a780_pxacamera_platform_data);
> +
> + i2c_register_board_info(0, ARRAY_AND_SIZE(a780_i2c_board_info));
> +#endif
> +
> platform_add_devices(ARRAY_AND_SIZE(devices));
> }
>
>
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
pgpaMlAQn5EjG.pgp
Description: PGP signature
