Hello Dipen Patel,
The patch 9a75a7cd03c9: "hte: Add Tegra HTE test driver" from Apr 22,
2022, leads to the following Smatch static checker warning:
drivers/hte/hte-tegra194-test.c:157 tegra_hte_test_probe()
warn: missing error code 'ret'
drivers/hte/hte-tegra194-test.c
102 static int tegra_hte_test_probe(struct platform_device *pdev)
103 {
104 int ret = 0;
105 int i, cnt;
106
107 dev_set_drvdata(&pdev->dev, &hte);
108 hte.pdev = &pdev->dev;
109
110 hte.gpio_out = gpiod_get(&pdev->dev, "out", 0);
111 if (IS_ERR(hte.gpio_out)) {
112 dev_err(&pdev->dev, "failed to get gpio out\n");
113 ret = -EINVAL;
It might be better to preserve the error code:
ret = PTR_ERR(hte.gpio_out);
I don't know if gpiod_get() can return -EPROBE_DEFER, but that's
an especially important error code to preserve.
114 goto out;
115 }
116
117 hte.gpio_in = gpiod_get(&pdev->dev, "in", 0);
118 if (IS_ERR(hte.gpio_in)) {
119 dev_err(&pdev->dev, "failed to get gpio in\n");
120 ret = -EINVAL;
121 goto free_gpio_out;
122 }
123
124 ret = gpiod_direction_output(hte.gpio_out, 0);
125 if (ret) {
126 dev_err(&pdev->dev, "failed to set output\n");
127 ret = -EINVAL;
Preserve the error code here as well?
128 goto free_gpio_in;
129 }
130
131 ret = gpiod_direction_input(hte.gpio_in);
132 if (ret) {
133 dev_err(&pdev->dev, "failed to set input\n");
134 ret = -EINVAL;
135 goto free_gpio_in;
136 }
137
138 ret = gpiod_to_irq(hte.gpio_in);
139 if (ret < 0) {
140 dev_err(&pdev->dev, "failed to map GPIO to IRQ: %d\n", ret);
141 ret = -ENXIO;
142 goto free_gpio_in;
143 }
144
145 hte.gpio_in_irq = ret;
146 ret = request_irq(ret, tegra_hte_test_gpio_isr,
147 IRQF_TRIGGER_RISING,
148 "tegra_hte_gpio_test_isr", &hte);
149 if (ret) {
150 dev_err(&pdev->dev, "failed to acquire IRQ\n");
151 ret = -ENXIO;
152 goto free_irq;
153 }
154
155 cnt = of_hte_req_count(hte.pdev);
156 if (cnt < 0)
--> 157 goto free_irq;
Needs a "ret = cnt;" on this error path otherwise it returns success.
158
159 dev_info(&pdev->dev, "Total requested lines:%d\n", cnt);
160
161 hte.desc = devm_kzalloc(hte.pdev, sizeof(*hte.desc) * cnt, GFP_KERNEL);
162 if (!hte.desc) {
163 ret = -ENOMEM;
164 goto free_irq;
165 }
166
167 for (i = 0; i < cnt; i++) {
168 if (i == 0)
169 /*
170 * GPIO hte init, line_id and name will be parsed from
171 * the device tree node. The edge_flag is implicitly
172 * set by request_irq call. Only line_data is needed to be
173 * set.
174 */
175 hte_init_line_attr(&hte.desc[i], 0, 0, NULL,
176 hte.gpio_in);
177 else
178 /*
179 * same comment as above except that IRQ does not need
180 * line data.
181 */
182 hte_init_line_attr(&hte.desc[i], 0, 0, NULL, NULL);
183
184 ret = hte_ts_get(hte.pdev, &hte.desc[i], i);
185 if (ret)
186 goto ts_put;
This is a very interesting thing because we're calling ts_put when
ts_get() failed...
187
188 ret = devm_hte_request_ts_ns(hte.pdev, &hte.desc[i],
189 process_hw_ts, NULL,
190 &hte.desc[i]);
191 if (ret) /* no need to ts_put, request API takes care */
192 goto free_irq;
The comment says that we don't call ts_put but then if the ts_get
fails on the next iteration through the loop, then we will.
193 }
194
195 timer_setup(&hte.timer, gpio_timer_cb, 0);
196 mod_timer(&hte.timer, jiffies + msecs_to_jiffies(5000));
197
198 return 0;
199
200 ts_put:
201 cnt = i;
202 for (i = 0; i < cnt; i++)
203 hte_ts_put(&hte.desc[i]);
204 free_irq:
205 free_irq(hte.gpio_in_irq, &hte);
206 free_gpio_in:
207 gpiod_put(hte.gpio_in);
208 free_gpio_out:
209 gpiod_put(hte.gpio_out);
210 out:
211
212 return ret;
213 }
regards,
dan carpenter
