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
|
From 0b6b245f8fcff205f097ce1c1bba4760f8b4f859 Mon Sep 17 00:00:00 2001
From: Naushir Patuck <naush@raspberrypi.com>
Date: Fri, 31 Mar 2023 10:33:51 +0100
Subject: [PATCH] drivers: media: imx708: Tidy-ups to address upstream
review comments
This commit addresses vaious tidy-ups requesed for upstreaming the
IMX708 driver. Notably:
- Remove #define IMX708_NUM_SUPPLIES and use ARRAY_SIZE() directly
- Use dev_err_probe where possible in imx708_probe()
- Fix error handling paths in imx708_probe()
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
drivers/media/i2c/imx708.c | 61 +++++++++++++++++---------------------
1 file changed, 28 insertions(+), 33 deletions(-)
--- a/drivers/media/i2c/imx708.c
+++ b/drivers/media/i2c/imx708.c
@@ -792,8 +792,6 @@ static const char * const imx708_supply_
"VDDL", /* IF (1.8V) supply */
};
-#define IMX708_NUM_SUPPLIES ARRAY_SIZE(imx708_supply_name)
-
/*
* Initialisation delay between XCLR low->high and the moment when the sensor
* can start capture (i.e. can leave software standby), given by T7 in the
@@ -815,7 +813,7 @@ struct imx708 {
u32 xclk_freq;
struct gpio_desc *reset_gpio;
- struct regulator_bulk_data supplies[IMX708_NUM_SUPPLIES];
+ struct regulator_bulk_data supplies[ARRAY_SIZE(imx708_supply_name)];
struct v4l2_ctrl_handler ctrl_handler;
/* V4L2 Controls */
@@ -935,9 +933,10 @@ static int imx708_write_regs(struct imx7
{
struct i2c_client *client = v4l2_get_subdevdata(&imx708->sd);
unsigned int i;
- int ret;
for (i = 0; i < len; i++) {
+ int ret;
+
ret = imx708_write_reg(imx708, regs[i].address, 1, regs[i].val);
if (ret) {
dev_err_ratelimited(&client->dev,
@@ -1025,8 +1024,6 @@ static int imx708_open(struct v4l2_subde
static int imx708_set_exposure(struct imx708 *imx708, unsigned int val)
{
- int ret;
-
val = max(val, imx708->mode->exposure_lines_min);
val -= val % imx708->mode->exposure_lines_step;
@@ -1034,11 +1031,9 @@ static int imx708_set_exposure(struct im
* In HDR mode this will set the longest exposure. The sensor
* will automatically divide the medium and short ones by 4,16.
*/
- ret = imx708_write_reg(imx708, IMX708_REG_EXPOSURE,
- IMX708_REG_VALUE_16BIT,
- val >> imx708->long_exp_shift);
-
- return ret;
+ return imx708_write_reg(imx708, IMX708_REG_EXPOSURE,
+ IMX708_REG_VALUE_16BIT,
+ val >> imx708->long_exp_shift);
}
static void imx708_adjust_exposure_range(struct imx708 *imx708,
@@ -1071,7 +1066,7 @@ static int imx708_set_analogue_gain(stru
static int imx708_set_frame_length(struct imx708 *imx708, unsigned int val)
{
- int ret = 0;
+ int ret;
imx708->long_exp_shift = 0;
@@ -1091,8 +1086,8 @@ static int imx708_set_frame_length(struc
static void imx708_set_framing_limits(struct imx708 *imx708)
{
- unsigned int hblank;
const struct imx708_mode *mode = imx708->mode;
+ unsigned int hblank;
__v4l2_ctrl_modify_range(imx708->pixel_rate,
mode->pixel_rate, mode->pixel_rate,
@@ -1599,7 +1594,7 @@ static int imx708_power_on(struct device
struct imx708 *imx708 = to_imx708(sd);
int ret;
- ret = regulator_bulk_enable(IMX708_NUM_SUPPLIES,
+ ret = regulator_bulk_enable(ARRAY_SIZE(imx708_supply_name),
imx708->supplies);
if (ret) {
dev_err(&client->dev, "%s: failed to enable regulators\n",
@@ -1621,7 +1616,8 @@ static int imx708_power_on(struct device
return 0;
reg_off:
- regulator_bulk_disable(IMX708_NUM_SUPPLIES, imx708->supplies);
+ regulator_bulk_disable(ARRAY_SIZE(imx708_supply_name),
+ imx708->supplies);
return ret;
}
@@ -1632,7 +1628,8 @@ static int imx708_power_off(struct devic
struct imx708 *imx708 = to_imx708(sd);
gpiod_set_value_cansleep(imx708->reset_gpio, 0);
- regulator_bulk_disable(IMX708_NUM_SUPPLIES, imx708->supplies);
+ regulator_bulk_disable(ARRAY_SIZE(imx708_supply_name),
+ imx708->supplies);
clk_disable_unprepare(imx708->xclk);
/* Force reprogramming of the common registers when powered up again. */
@@ -1679,11 +1676,11 @@ static int imx708_get_regulators(struct
struct i2c_client *client = v4l2_get_subdevdata(&imx708->sd);
unsigned int i;
- for (i = 0; i < IMX708_NUM_SUPPLIES; i++)
+ for (i = 0; i < ARRAY_SIZE(imx708_supply_name); i++)
imx708->supplies[i].supply = imx708_supply_name[i];
return devm_regulator_bulk_get(&client->dev,
- IMX708_NUM_SUPPLIES,
+ ARRAY_SIZE(imx708_supply_name),
imx708->supplies);
}
@@ -1956,23 +1953,19 @@ static int imx708_probe(struct i2c_clien
/* Get system clock (xclk) */
imx708->xclk = devm_clk_get(dev, NULL);
- if (IS_ERR(imx708->xclk)) {
- dev_err(dev, "failed to get xclk\n");
- return PTR_ERR(imx708->xclk);
- }
+ if (IS_ERR(imx708->xclk))
+ return dev_err_probe(dev, PTR_ERR(imx708->xclk),
+ "failed to get xclk\n");
imx708->xclk_freq = clk_get_rate(imx708->xclk);
- if (imx708->xclk_freq != IMX708_XCLK_FREQ) {
- dev_err(dev, "xclk frequency not supported: %d Hz\n",
- imx708->xclk_freq);
- return -EINVAL;
- }
+ if (imx708->xclk_freq != IMX708_XCLK_FREQ)
+ return dev_err_probe(dev, -EINVAL,
+ "xclk frequency not supported: %d Hz\n",
+ imx708->xclk_freq);
ret = imx708_get_regulators(imx708);
- if (ret) {
- dev_err(dev, "failed to get regulators\n");
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get regulators\n");
/* Request optional enable pin */
imx708->reset_gpio = devm_gpiod_get_optional(dev, "reset",
@@ -2001,7 +1994,7 @@ static int imx708_probe(struct i2c_clien
/* This needs the pm runtime to be registered. */
ret = imx708_init_controls(imx708);
if (ret)
- goto error_power_off;
+ goto error_pm_runtime;
/* Initialize subdev */
imx708->sd.internal_ops = &imx708_internal_ops;
@@ -2033,9 +2026,11 @@ error_media_entity:
error_handler_free:
imx708_free_controls(imx708);
-error_power_off:
+error_pm_runtime:
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
+
+error_power_off:
imx708_power_off(&client->dev);
return ret;
|