diff mbox series

media: imx258: remove test pattern map from driver

Message ID 1533197820-19176-1-git-send-email-jasonx.z.chen@intel.com (mailing list archive)
State New, archived
Headers show
Series media: imx258: remove test pattern map from driver | expand

Commit Message

Chen, JasonX Z Aug. 2, 2018, 8:17 a.m. UTC
From: "Chen, JasonX Z" <jasonx.z.chen@intel.com>

Test Pattern mode be picked at HAL instead of driver.
do a FLIP when userspace use test pattern mode.
add entity_ops for validating imx258 link.

Signed-off-by: Chen, JasonX Z <jasonx.z.chen@intel.com>
---
 drivers/media/i2c/imx258.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

Comments

Sakari Ailus Aug. 2, 2018, 12:39 p.m. UTC | #1
Hi Jason,

On Thu, Aug 02, 2018 at 04:17:00PM +0800, jasonx.z.chen@intel.com wrote:
> From: "Chen, JasonX Z" <jasonx.z.chen@intel.com>
> 
> Test Pattern mode be picked at HAL instead of driver.
> do a FLIP when userspace use test pattern mode.
> add entity_ops for validating imx258 link.

Hmm. I think this would be changed based on my comments anyway, but please
explain what you're doing and *why*. HAL is not relevant in this context
I'd say.

> 
> Signed-off-by: Chen, JasonX Z <jasonx.z.chen@intel.com>
> ---
>  drivers/media/i2c/imx258.c | 28 ++++++++--------------------
>  1 file changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 31a1e22..71f9875 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -62,11 +62,6 @@
>  
>  /* Test Pattern Control */
>  #define IMX258_REG_TEST_PATTERN		0x0600
> -#define IMX258_TEST_PATTERN_DISABLE	0
> -#define IMX258_TEST_PATTERN_SOLID_COLOR	1
> -#define IMX258_TEST_PATTERN_COLOR_BARS	2
> -#define IMX258_TEST_PATTERN_GREY_COLOR	3
> -#define IMX258_TEST_PATTERN_PN9		4
>  
>  /* Orientation */
>  #define REG_MIRROR_FLIP_CONTROL		0x0101
> @@ -504,20 +499,12 @@ struct imx258_mode {
>  
>  static const char * const imx258_test_pattern_menu[] = {
>  	"Disabled",
> -	"Color Bars",
>  	"Solid Color",
> +	"Color Bars",
>  	"Grey Color Bars",
>  	"PN9"
>  };
>  
> -static const int imx258_test_pattern_val[] = {
> -	IMX258_TEST_PATTERN_DISABLE,
> -	IMX258_TEST_PATTERN_COLOR_BARS,
> -	IMX258_TEST_PATTERN_SOLID_COLOR,
> -	IMX258_TEST_PATTERN_GREY_COLOR,
> -	IMX258_TEST_PATTERN_PN9,
> -};
> -
>  /* Configurations for supported link frequencies */
>  #define IMX258_LINK_FREQ_634MHZ	633600000ULL
>  #define IMX258_LINK_FREQ_320MHZ	320000000ULL
> @@ -752,7 +739,6 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
>  		container_of(ctrl->handler, struct imx258, ctrl_handler);
>  	struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
>  	int ret = 0;
> -

I think this newline is where it should be.

>  	/*
>  	 * Applying V4L2 control value only happens
>  	 * when power is up for streaming
> @@ -778,13 +764,10 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
>  	case V4L2_CID_TEST_PATTERN:
>  		ret = imx258_write_reg(imx258, IMX258_REG_TEST_PATTERN,
>  				IMX258_REG_VALUE_16BIT,
> -				imx258_test_pattern_val[ctrl->val]);
> -
> +				ctrl->val);
>  		ret = imx258_write_reg(imx258, REG_MIRROR_FLIP_CONTROL,
>  				IMX258_REG_VALUE_08BIT,
> -				ctrl->val == imx258_test_pattern_val
> -				[IMX258_TEST_PATTERN_DISABLE] ?
> -				REG_CONFIG_MIRROR_FLIP :
> +				!ctrl->val?REG_CONFIG_MIRROR_FLIP :

Spaces around "?".

>  				REG_CONFIG_FLIP_TEST_PATTERN);
>  		break;
>  	default:
> @@ -1105,6 +1088,10 @@ static int imx258_identify_module(struct imx258 *imx258)
>  	.pad = &imx258_pad_ops,
>  };
>  
> +static const struct media_entity_operations imx258_subdev_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,

The sensor only has a source pad while the link validate is only needed for
sink pads.

> +};
> +
>  static const struct v4l2_subdev_internal_ops imx258_internal_ops = {
>  	.open = imx258_open,
>  };
> @@ -1250,6 +1237,7 @@ static int imx258_probe(struct i2c_client *client)
>  
>  	/* Initialize subdev */
>  	imx258->sd.internal_ops = &imx258_internal_ops;
> +	imx258->sd.entity.ops  = &imx258_subdev_entity_ops;
>  	imx258->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  	imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 31a1e22..71f9875 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -62,11 +62,6 @@ 
 
 /* Test Pattern Control */
 #define IMX258_REG_TEST_PATTERN		0x0600
-#define IMX258_TEST_PATTERN_DISABLE	0
-#define IMX258_TEST_PATTERN_SOLID_COLOR	1
-#define IMX258_TEST_PATTERN_COLOR_BARS	2
-#define IMX258_TEST_PATTERN_GREY_COLOR	3
-#define IMX258_TEST_PATTERN_PN9		4
 
 /* Orientation */
 #define REG_MIRROR_FLIP_CONTROL		0x0101
@@ -504,20 +499,12 @@  struct imx258_mode {
 
 static const char * const imx258_test_pattern_menu[] = {
 	"Disabled",
-	"Color Bars",
 	"Solid Color",
+	"Color Bars",
 	"Grey Color Bars",
 	"PN9"
 };
 
-static const int imx258_test_pattern_val[] = {
-	IMX258_TEST_PATTERN_DISABLE,
-	IMX258_TEST_PATTERN_COLOR_BARS,
-	IMX258_TEST_PATTERN_SOLID_COLOR,
-	IMX258_TEST_PATTERN_GREY_COLOR,
-	IMX258_TEST_PATTERN_PN9,
-};
-
 /* Configurations for supported link frequencies */
 #define IMX258_LINK_FREQ_634MHZ	633600000ULL
 #define IMX258_LINK_FREQ_320MHZ	320000000ULL
@@ -752,7 +739,6 @@  static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
 		container_of(ctrl->handler, struct imx258, ctrl_handler);
 	struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
 	int ret = 0;
-
 	/*
 	 * Applying V4L2 control value only happens
 	 * when power is up for streaming
@@ -778,13 +764,10 @@  static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_TEST_PATTERN:
 		ret = imx258_write_reg(imx258, IMX258_REG_TEST_PATTERN,
 				IMX258_REG_VALUE_16BIT,
-				imx258_test_pattern_val[ctrl->val]);
-
+				ctrl->val);
 		ret = imx258_write_reg(imx258, REG_MIRROR_FLIP_CONTROL,
 				IMX258_REG_VALUE_08BIT,
-				ctrl->val == imx258_test_pattern_val
-				[IMX258_TEST_PATTERN_DISABLE] ?
-				REG_CONFIG_MIRROR_FLIP :
+				!ctrl->val?REG_CONFIG_MIRROR_FLIP :
 				REG_CONFIG_FLIP_TEST_PATTERN);
 		break;
 	default:
@@ -1105,6 +1088,10 @@  static int imx258_identify_module(struct imx258 *imx258)
 	.pad = &imx258_pad_ops,
 };
 
+static const struct media_entity_operations imx258_subdev_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
 static const struct v4l2_subdev_internal_ops imx258_internal_ops = {
 	.open = imx258_open,
 };
@@ -1250,6 +1237,7 @@  static int imx258_probe(struct i2c_client *client)
 
 	/* Initialize subdev */
 	imx258->sd.internal_ops = &imx258_internal_ops;
+	imx258->sd.entity.ops  = &imx258_subdev_entity_ops;
 	imx258->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	imx258->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;