gpio/vt8500: memory cleanup missing

This driver is missing a .remove callback, and the fail path on probe is incomplete. If an error occurs in vt8500_add_chips, gpio_base is not unmapped. The driver is also ignoring the return value from this function so if a chip fails to register it completes as successful. Replaced pr_err with dev_err in vt8500_add_chips since the device is available. There is also no .remove callback defined so the function is added. Signed-off-by: Tony Prisk <linux@prisktech.co.nz> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

gpio/vt8500: memory cleanup missing
This driver is missing a .remove callback, and the fail path on probe is incomplete. If an error occurs in vt8500_add_chips, gpio_base is not unmapped. The driver is also ignoring the return value from this function so if a chip fails to register it completes as successful. Replaced pr_err with dev_err in vt8500_add_chips since the device is available. There is also no .remove callback defined so the function is added. Signed-off-by: Tony Prisk <linux@prisktech.co.nz> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Tony Prisk · Grant Likely
1 parent 362432aed5
Showing 1 changed file with 49 additions and 12 deletions Side-by-side Diff
drivers/gpio/gpio-vt8500.c
@@ -127,7 +127,13 @@
 	void __iomem	*base;
 };
  
+struct vt8500_data {
+	struct vt8500_gpio_chip *chip;
+	void __iomem *iobase;
+	int num_banks;
+};
  
+
 #define to_vt8500(__chip) container_of(__chip, struct vt8500_gpio_chip, chip)
  
 static int vt8500_gpio_request(struct gpio_chip *chip, unsigned offset)
  
  
  
@@ -224,19 +230,32 @@
 static int vt8500_add_chips(struct platform_device *pdev, void __iomem *base,
 				const struct vt8500_gpio_data *data)
 {
+	struct vt8500_data *priv;
 	struct vt8500_gpio_chip *vtchip;
 	struct gpio_chip *chip;
 	int i;
 	int pin_cnt = 0;
  
-	vtchip = devm_kzalloc(&pdev->dev,
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct vt8500_data), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	priv->chip = devm_kzalloc(&pdev->dev,
 			sizeof(struct vt8500_gpio_chip) * data->num_banks,
 			GFP_KERNEL);
-	if (!vtchip) {
-		pr_err("%s: failed to allocate chip memory\n", __func__);
+	if (!priv->chip) {
+		dev_err(&pdev->dev, "failed to allocate chip memory\n");
 		return -ENOMEM;
 	}
  
+	priv->iobase = base;
+	priv->num_banks = data->num_banks;
+	platform_set_drvdata(pdev, priv);
+
+	vtchip = priv->chip;
+
 	for (i = 0; i < data->num_banks; i++) {
 		vtchip[i].base = base;
 		vtchip[i].regs = &data->banks[i];
  
  
  
  
  
  
  
  
@@ -273,36 +292,54 @@
  
 static int vt8500_gpio_probe(struct platform_device *pdev)
 {
+	int ret;
 	void __iomem *gpio_base;
-	struct device_node *np;
+	struct resource *res;
 	const struct of_device_id *of_id =
 				of_match_device(vt8500_gpio_dt_ids, &pdev->dev);
  
 	if (!of_id) {
-		dev_err(&pdev->dev, "Failed to find gpio controller\n");
+		dev_err(&pdev->dev, "No matching driver data\n");
 		return -ENODEV;
 	}
  
-	np = pdev->dev.of_node;
-	if (!np) {
-		dev_err(&pdev->dev, "Missing GPIO description in devicetree\n");
-		return -EFAULT;
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Unable to get IO resource\n");
+		return -ENODEV;
 	}
  
-	gpio_base = of_iomap(np, 0);
+	gpio_base = devm_request_and_ioremap(&pdev->dev, res);
 	if (!gpio_base) {
 		dev_err(&pdev->dev, "Unable to map GPIO registers\n");
-		of_node_put(np);
 		return -ENOMEM;
 	}
  
-	vt8500_add_chips(pdev, gpio_base, of_id->data);
+	ret = vt8500_add_chips(pdev, gpio_base, of_id->data);
  
+	return ret;
+}
+
+static int vt8500_gpio_remove(struct platform_device *pdev)
+{
+	int i;
+	int ret;
+	struct vt8500_data *priv = platform_get_drvdata(pdev);
+	struct vt8500_gpio_chip *vtchip = priv->chip;
+
+	for (i = 0; i < priv->num_banks; i++) {
+		ret = gpiochip_remove(&vtchip[i].chip);
+		if (ret)
+			dev_warn(&pdev->dev, "gpiochip_remove returned %d\n",
+				 ret);
+	}
+
 	return 0;
 }
  
 static struct platform_driver vt8500_gpio_driver = {
 	.probe		= vt8500_gpio_probe,
+	.remove		= vt8500_gpio_remove,
 	.driver		= {
 		.name	= "vt8500-gpio",
 		.owner	= THIS_MODULE,
...	...	@@ -127,7 +127,13 @@
127	127	void __iomem *base;
128	128	};
129	129
	130	+struct vt8500_data {
	131	+ struct vt8500_gpio_chip *chip;
	132	+ void __iomem *iobase;
	133	+ int num_banks;
	134	+};
130	135
	136	+
131	137	#define to_vt8500(__chip) container_of(__chip, struct vt8500_gpio_chip, chip)
132	138
133	139	static int vt8500_gpio_request(struct gpio_chip *chip, unsigned offset)
134	140
135	141
136	142
...	...	@@ -224,19 +230,32 @@
224	230	static int vt8500_add_chips(struct platform_device pdev, void __iomem base,
225	231	const struct vt8500_gpio_data *data)
226	232	{
	233	+ struct vt8500_data *priv;
227	234	struct vt8500_gpio_chip *vtchip;
228	235	struct gpio_chip *chip;
229	236	int i;
230	237	int pin_cnt = 0;
231	238
232		- vtchip = devm_kzalloc(&pdev->dev,
	239	+ priv = devm_kzalloc(&pdev->dev, sizeof(struct vt8500_data), GFP_KERNEL);
	240	+ if (!priv) {
	241	+ dev_err(&pdev->dev, "failed to allocate memory\n");
	242	+ return -ENOMEM;
	243	+ }
	244	+
	245	+ priv->chip = devm_kzalloc(&pdev->dev,
233	246	sizeof(struct vt8500_gpio_chip) * data->num_banks,
234	247	GFP_KERNEL);
235		- if (!vtchip) {
236		- pr_err("%s: failed to allocate chip memory\n", __func__);
	248	+ if (!priv->chip) {
	249	+ dev_err(&pdev->dev, "failed to allocate chip memory\n");
237	250	return -ENOMEM;
238	251	}
239	252
	253	+ priv->iobase = base;
	254	+ priv->num_banks = data->num_banks;
	255	+ platform_set_drvdata(pdev, priv);
	256	+
	257	+ vtchip = priv->chip;
	258	+
240	259	for (i = 0; i < data->num_banks; i++) {
241	260	vtchip[i].base = base;
242	261	vtchip[i].regs = &data->banks[i];
243	262
244	263
245	264
246	265
247	266
248	267
249	268
250	269
...	...	@@ -273,36 +292,54 @@
273	292
274	293	static int vt8500_gpio_probe(struct platform_device *pdev)
275	294	{
	295	+ int ret;
276	296	void __iomem *gpio_base;
277		- struct device_node *np;
	297	+ struct resource *res;
278	298	const struct of_device_id *of_id =
279	299	of_match_device(vt8500_gpio_dt_ids, &pdev->dev);
280	300
281	301	if (!of_id) {
282		- dev_err(&pdev->dev, "Failed to find gpio controller\n");
	302	+ dev_err(&pdev->dev, "No matching driver data\n");
283	303	return -ENODEV;
284	304	}
285	305
286		- np = pdev->dev.of_node;
287		- if (!np) {
288		- dev_err(&pdev->dev, "Missing GPIO description in devicetree\n");
289		- return -EFAULT;
	306	+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	307	+ if (!res) {
	308	+ dev_err(&pdev->dev, "Unable to get IO resource\n");
	309	+ return -ENODEV;
290	310	}
291	311
292		- gpio_base = of_iomap(np, 0);
	312	+ gpio_base = devm_request_and_ioremap(&pdev->dev, res);
293	313	if (!gpio_base) {
294	314	dev_err(&pdev->dev, "Unable to map GPIO registers\n");
295		- of_node_put(np);
296	315	return -ENOMEM;
297	316	}
298	317
299		- vt8500_add_chips(pdev, gpio_base, of_id->data);
	318	+ ret = vt8500_add_chips(pdev, gpio_base, of_id->data);
300	319
	320	+ return ret;
	321	+}
	322	+
	323	+static int vt8500_gpio_remove(struct platform_device *pdev)
	324	+{
	325	+ int i;
	326	+ int ret;
	327	+ struct vt8500_data *priv = platform_get_drvdata(pdev);
	328	+ struct vt8500_gpio_chip *vtchip = priv->chip;
	329	+
	330	+ for (i = 0; i < priv->num_banks; i++) {
	331	+ ret = gpiochip_remove(&vtchip[i].chip);
	332	+ if (ret)
	333	+ dev_warn(&pdev->dev, "gpiochip_remove returned %d\n",
	334	+ ret);
	335	+ }
	336	+
301	337	return 0;
302	338	}
303	339
304	340	static struct platform_driver vt8500_gpio_driver = {
305	341	.probe = vt8500_gpio_probe,
	342	+ .remove = vt8500_gpio_remove,
306	343	.driver = {
307	344	.name = "vt8500-gpio",
308	345	.owner = THIS_MODULE,