Re: [PATCH 3/4] mt9m001: allow setting of bus width from board code
On Wed, 11 Mar 2009, Sascha Hauer wrote:
> This patch removes the phytec specific setting of the bus width
> and switches to the more generic query_bus_param/set_bus_param
> hooks
>
> Signed-off-by: Sascha Hauer
> ---
> drivers/media/video/Kconfig |7 --
> drivers/media/video/mt9m001.c | 130 +---
> 2 files changed, 30 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 19cf3b8..5fc1531 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -728,13 +728,6 @@ config SOC_CAMERA_MT9M001
> This driver supports MT9M001 cameras from Micron, monochrome
> and colour models.
>
> -config MT9M001_PCA9536_SWITCH
> - bool "pca9536 datawidth switch for mt9m001"
> - depends on SOC_CAMERA_MT9M001 && GENERIC_GPIO
> - help
> - Select this if your MT9M001 camera uses a PCA9536 I2C GPIO
> - extender to switch between 8 and 10 bit datawidth modes
> -
> config SOC_CAMERA_MT9M111
> tristate "mt9m111 and mt9m112 support"
> depends on SOC_CAMERA && I2C
> diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
> index c1bf75e..112c612 100644
> --- a/drivers/media/video/mt9m001.c
> +++ b/drivers/media/video/mt9m001.c
> @@ -75,7 +75,6 @@ struct mt9m001 {
> int model; /* V4L2_IDENT_MT9M001* codes from v4l2-chip-ident.h */
> int switch_gpio;
> unsigned char autoexposure;
> - unsigned char datawidth;
> };
>
> static int reg_read(struct soc_camera_device *icd, const u8 reg)
> @@ -181,90 +180,17 @@ static int mt9m001_stop_capture(struct
> soc_camera_device *icd)
> return 0;
> }
>
> -static int bus_switch_request(struct mt9m001 *mt9m001,
> - struct soc_camera_link *icl)
> -{
> -#ifdef CONFIG_MT9M001_PCA9536_SWITCH
> - int ret;
> - unsigned int gpio = icl->gpio;
> -
> - if (gpio_is_valid(gpio)) {
> - /* We have a data bus switch. */
> - ret = gpio_request(gpio, "mt9m001");
> - if (ret < 0) {
> - dev_err(&mt9m001->client->dev, "Cannot get GPIO %u\n",
> - gpio);
> - return ret;
> - }
> -
> - ret = gpio_direction_output(gpio, 0);
> - if (ret < 0) {
> - dev_err(&mt9m001->client->dev,
> - "Cannot set GPIO %u to output\n", gpio);
> - gpio_free(gpio);
> - return ret;
> - }
> - }
> -
> - mt9m001->switch_gpio = gpio;
> -#else
> - mt9m001->switch_gpio = -EINVAL;
> -#endif
> - return 0;
> -}
> -
> -static void bus_switch_release(struct mt9m001 *mt9m001)
> -{
> -#ifdef CONFIG_MT9M001_PCA9536_SWITCH
> - if (gpio_is_valid(mt9m001->switch_gpio))
> - gpio_free(mt9m001->switch_gpio);
> -#endif
> -}
> -
> -static int bus_switch_act(struct mt9m001 *mt9m001, int go8bit)
> -{
> -#ifdef CONFIG_MT9M001_PCA9536_SWITCH
> - if (!gpio_is_valid(mt9m001->switch_gpio))
> - return -ENODEV;
> -
> - gpio_set_value_cansleep(mt9m001->switch_gpio, go8bit);
> - return 0;
> -#else
> - return -ENODEV;
> -#endif
> -}
> -
> -static int bus_switch_possible(struct mt9m001 *mt9m001)
> -{
> -#ifdef CONFIG_MT9M001_PCA9536_SWITCH
> - return gpio_is_valid(mt9m001->switch_gpio);
> -#else
> - return 0;
> -#endif
> -}
> -
> static int mt9m001_set_bus_param(struct soc_camera_device *icd,
>unsigned long flags)
> {
> struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
> - unsigned int width_flag = flags & SOCAM_DATAWIDTH_MASK;
> - int ret;
> + struct soc_camera_link *icl = mt9m001->client->dev.platform_data;
>
> /* Flags validity verified in test_bus_param */
>
> - if ((mt9m001->datawidth != 10 && (width_flag == SOCAM_DATAWIDTH_10)) ||
> - (mt9m001->datawidth != 9 && (width_flag == SOCAM_DATAWIDTH_9)) ||
> - (mt9m001->datawidth != 8 && (width_flag == SOCAM_DATAWIDTH_8))) {
> - /* Well, we actually only can do 10 or 8 bits... */
> - if (width_flag == SOCAM_DATAWIDTH_9)
> - return -EINVAL;
> - ret = bus_switch_act(mt9m001,
> - width_flag == SOCAM_DATAWIDTH_8);
> - if (ret < 0)
> - return ret;
> -
> - mt9m001->datawidth = width_flag == SOCAM_DATAWIDTH_8 ? 8 : 10;
> - }
> + if (icl->set_bus_param)
> + return icl->set_bus_param(&mt9m001->client->dev,
> + flags & SOCAM_DATAWIDTH_MASK);
Not quite. Look at the original code. If no change is requested - just
return 0. If a change is requested, but switching is impossible - return
an error - and this is not, what you are doing in 2/4, please fix. So, you
might still want to preserve ".d
[PATCH 3/4] mt9m001: allow setting of bus width from board code
This patch removes the phytec specific setting of the bus width
and switches to the more generic query_bus_param/set_bus_param
hooks
Signed-off-by: Sascha Hauer
---
drivers/media/video/Kconfig |7 --
drivers/media/video/mt9m001.c | 130 +---
2 files changed, 30 insertions(+), 107 deletions(-)
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 19cf3b8..5fc1531 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -728,13 +728,6 @@ config SOC_CAMERA_MT9M001
This driver supports MT9M001 cameras from Micron, monochrome
and colour models.
-config MT9M001_PCA9536_SWITCH
- bool "pca9536 datawidth switch for mt9m001"
- depends on SOC_CAMERA_MT9M001 && GENERIC_GPIO
- help
- Select this if your MT9M001 camera uses a PCA9536 I2C GPIO
- extender to switch between 8 and 10 bit datawidth modes
-
config SOC_CAMERA_MT9M111
tristate "mt9m111 and mt9m112 support"
depends on SOC_CAMERA && I2C
diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
index c1bf75e..112c612 100644
--- a/drivers/media/video/mt9m001.c
+++ b/drivers/media/video/mt9m001.c
@@ -75,7 +75,6 @@ struct mt9m001 {
int model; /* V4L2_IDENT_MT9M001* codes from v4l2-chip-ident.h */
int switch_gpio;
unsigned char autoexposure;
- unsigned char datawidth;
};
static int reg_read(struct soc_camera_device *icd, const u8 reg)
@@ -181,90 +180,17 @@ static int mt9m001_stop_capture(struct soc_camera_device
*icd)
return 0;
}
-static int bus_switch_request(struct mt9m001 *mt9m001,
- struct soc_camera_link *icl)
-{
-#ifdef CONFIG_MT9M001_PCA9536_SWITCH
- int ret;
- unsigned int gpio = icl->gpio;
-
- if (gpio_is_valid(gpio)) {
- /* We have a data bus switch. */
- ret = gpio_request(gpio, "mt9m001");
- if (ret < 0) {
- dev_err(&mt9m001->client->dev, "Cannot get GPIO %u\n",
- gpio);
- return ret;
- }
-
- ret = gpio_direction_output(gpio, 0);
- if (ret < 0) {
- dev_err(&mt9m001->client->dev,
- "Cannot set GPIO %u to output\n", gpio);
- gpio_free(gpio);
- return ret;
- }
- }
-
- mt9m001->switch_gpio = gpio;
-#else
- mt9m001->switch_gpio = -EINVAL;
-#endif
- return 0;
-}
-
-static void bus_switch_release(struct mt9m001 *mt9m001)
-{
-#ifdef CONFIG_MT9M001_PCA9536_SWITCH
- if (gpio_is_valid(mt9m001->switch_gpio))
- gpio_free(mt9m001->switch_gpio);
-#endif
-}
-
-static int bus_switch_act(struct mt9m001 *mt9m001, int go8bit)
-{
-#ifdef CONFIG_MT9M001_PCA9536_SWITCH
- if (!gpio_is_valid(mt9m001->switch_gpio))
- return -ENODEV;
-
- gpio_set_value_cansleep(mt9m001->switch_gpio, go8bit);
- return 0;
-#else
- return -ENODEV;
-#endif
-}
-
-static int bus_switch_possible(struct mt9m001 *mt9m001)
-{
-#ifdef CONFIG_MT9M001_PCA9536_SWITCH
- return gpio_is_valid(mt9m001->switch_gpio);
-#else
- return 0;
-#endif
-}
-
static int mt9m001_set_bus_param(struct soc_camera_device *icd,
unsigned long flags)
{
struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
- unsigned int width_flag = flags & SOCAM_DATAWIDTH_MASK;
- int ret;
+ struct soc_camera_link *icl = mt9m001->client->dev.platform_data;
/* Flags validity verified in test_bus_param */
- if ((mt9m001->datawidth != 10 && (width_flag == SOCAM_DATAWIDTH_10)) ||
- (mt9m001->datawidth != 9 && (width_flag == SOCAM_DATAWIDTH_9)) ||
- (mt9m001->datawidth != 8 && (width_flag == SOCAM_DATAWIDTH_8))) {
- /* Well, we actually only can do 10 or 8 bits... */
- if (width_flag == SOCAM_DATAWIDTH_9)
- return -EINVAL;
- ret = bus_switch_act(mt9m001,
-width_flag == SOCAM_DATAWIDTH_8);
- if (ret < 0)
- return ret;
-
- mt9m001->datawidth = width_flag == SOCAM_DATAWIDTH_8 ? 8 : 10;
- }
+ if (icl->set_bus_param)
+ return icl->set_bus_param(&mt9m001->client->dev,
+ flags & SOCAM_DATAWIDTH_MASK);
return 0;
}
@@ -274,12 +200,15 @@ static unsigned long mt9m001_query_bus_param(struct
soc_camera_device *icd)
struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
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 |
+ unsigned long
