1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
|
From a04966a5f739570c32ac6bcc44c68643bf836780 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Sun, 16 Oct 2022 09:15:13 +0300
Subject: [PATCH] media: i2c: imx290: Simplify error handling when
writing registers
Upstream commit e611f3dac54c.
Error handling for register writes requires checking the error status of
every single write. This makes the code complex, or incorrect when the
checks are omitted. Simplify this by passing a pointer to an error code
to the imx290_write_reg() function, which allows writing multiple
registers in a row and only checking for errors at the end.
While at it, rename imx290_write_reg() to imx290_write() as there's
nothing else than registers to write, and rename imx290_read_reg()
accordingly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/i2c/imx290.c | 86 ++++++++++++++------------------------
1 file changed, 32 insertions(+), 54 deletions(-)
--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -367,7 +367,7 @@ static inline struct imx290 *to_imx290(s
return container_of(_sd, struct imx290, sd);
}
-static int __always_unused imx290_read_reg(struct imx290 *imx290, u32 addr, u32 *value)
+static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value)
{
u8 data[3] = { 0, 0, 0 };
int ret;
@@ -385,17 +385,23 @@ static int __always_unused imx290_read_r
return 0;
}
-static int imx290_write_reg(struct imx290 *imx290, u32 addr, u32 value)
+static int imx290_write(struct imx290 *imx290, u32 addr, u32 value, int *err)
{
u8 data[3] = { value & 0xff, (value >> 8) & 0xff, value >> 16 };
int ret;
+ if (err && *err)
+ return *err;
+
ret = regmap_raw_write(imx290->regmap, addr & IMX290_REG_ADDR_MASK,
data, (addr >> IMX290_REG_SIZE_SHIFT) & 3);
- if (ret < 0)
+ if (ret < 0) {
dev_err(imx290->dev, "%u-bit write to 0x%04x failed: %d\n",
((addr >> IMX290_REG_SIZE_SHIFT) & 3) * 8,
addr & IMX290_REG_ADDR_MASK, ret);
+ if (err)
+ *err = ret;
+ }
return ret;
}
@@ -408,7 +414,7 @@ static int imx290_set_register_array(str
int ret;
for (i = 0; i < num_settings; ++i, ++settings) {
- ret = imx290_write_reg(imx290, settings->reg, settings->val);
+ ret = imx290_write(imx290, settings->reg, settings->val, NULL);
if (ret < 0)
return ret;
}
@@ -419,29 +425,16 @@ static int imx290_set_register_array(str
return 0;
}
-static int imx290_set_gain(struct imx290 *imx290, u32 value)
-{
- int ret;
-
- ret = imx290_write_reg(imx290, IMX290_GAIN, value);
- if (ret)
- dev_err(imx290->dev, "Unable to write gain\n");
-
- return ret;
-}
-
/* Stop streaming */
static int imx290_stop_streaming(struct imx290 *imx290)
{
- int ret;
+ int ret = 0;
- ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x01);
- if (ret < 0)
- return ret;
+ imx290_write(imx290, IMX290_STANDBY, 0x01, &ret);
msleep(30);
- return imx290_write_reg(imx290, IMX290_XMSTA, 0x01);
+ return imx290_write(imx290, IMX290_XMSTA, 0x01, &ret);
}
static int imx290_set_ctrl(struct v4l2_ctrl *ctrl)
@@ -456,25 +449,25 @@ static int imx290_set_ctrl(struct v4l2_c
switch (ctrl->id) {
case V4L2_CID_GAIN:
- ret = imx290_set_gain(imx290, ctrl->val);
+ ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL);
break;
case V4L2_CID_TEST_PATTERN:
if (ctrl->val) {
- imx290_write_reg(imx290, IMX290_BLKLEVEL, 0);
+ imx290_write(imx290, IMX290_BLKLEVEL, 0, &ret);
usleep_range(10000, 11000);
- imx290_write_reg(imx290, IMX290_PGCTRL,
- (u8)(IMX290_PGCTRL_REGEN |
- IMX290_PGCTRL_THRU |
- IMX290_PGCTRL_MODE(ctrl->val)));
+ imx290_write(imx290, IMX290_PGCTRL,
+ (u8)(IMX290_PGCTRL_REGEN |
+ IMX290_PGCTRL_THRU |
+ IMX290_PGCTRL_MODE(ctrl->val)), &ret);
} else {
- imx290_write_reg(imx290, IMX290_PGCTRL, 0x00);
+ imx290_write(imx290, IMX290_PGCTRL, 0x00, &ret);
usleep_range(10000, 11000);
if (imx290->bpp == 10)
- imx290_write_reg(imx290, IMX290_BLKLEVEL,
- 0x3c);
+ imx290_write(imx290, IMX290_BLKLEVEL, 0x3c,
+ &ret);
else /* 12 bits per pixel */
- imx290_write_reg(imx290, IMX290_BLKLEVEL,
- 0xf0);
+ imx290_write(imx290, IMX290_BLKLEVEL, 0xf0,
+ &ret);
}
break;
default:
@@ -695,7 +688,8 @@ static int imx290_start_streaming(struct
return ret;
}
- ret = imx290_write_reg(imx290, IMX290_HMAX, imx290->current_mode->hmax);
+ ret = imx290_write(imx290, IMX290_HMAX, imx290->current_mode->hmax,
+ NULL);
if (ret)
return ret;
@@ -706,14 +700,12 @@ static int imx290_start_streaming(struct
return ret;
}
- ret = imx290_write_reg(imx290, IMX290_STANDBY, 0x00);
- if (ret < 0)
- return ret;
+ imx290_write(imx290, IMX290_STANDBY, 0x00, &ret);
msleep(30);
/* Start streaming */
- return imx290_write_reg(imx290, IMX290_XMSTA, 0x00);
+ return imx290_write(imx290, IMX290_XMSTA, 0x00, &ret);
}
static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
@@ -772,27 +764,13 @@ static int imx290_set_data_lanes(struct
* validated in probe itself
*/
dev_err(imx290->dev, "Lane configuration not supported\n");
- ret = -EINVAL;
- goto exit;
- }
-
- ret = imx290_write_reg(imx290, IMX290_PHY_LANE_NUM, laneval);
- if (ret) {
- dev_err(imx290->dev, "Error setting Physical Lane number register\n");
- goto exit;
- }
-
- ret = imx290_write_reg(imx290, IMX290_CSI_LANE_MODE, laneval);
- if (ret) {
- dev_err(imx290->dev, "Error setting CSI Lane mode register\n");
- goto exit;
+ return -EINVAL;
}
- ret = imx290_write_reg(imx290, IMX290_FR_FDG_SEL, frsel);
- if (ret)
- dev_err(imx290->dev, "Error setting FR/FDG SEL register\n");
+ imx290_write(imx290, IMX290_PHY_LANE_NUM, laneval, &ret);
+ imx290_write(imx290, IMX290_CSI_LANE_MODE, laneval, &ret);
+ imx290_write(imx290, IMX290_FR_FDG_SEL, frsel, &ret);
-exit:
return ret;
}
|