...

quoted

quoted

+	if (IS_ERR(ams->base))
+		return PTR_ERR(ams->base);
+
+	ams->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(ams->clk))
+		return PTR_ERR(ams->clk);
+	clk_prepare_enable(ams->clk);
+	devm_add_action_or_reset(&pdev->dev, (void  

*)clk_disable_unprepare,  

quoted

+				 ams->clk);
+
+	INIT_DELAYED_WORK(&ams->ams_unmask_work,  

ams_unmask_worker);  

quoted

+	devm_add_action_or_reset(&pdev->dev, (void  

*)cancel_delayed_work,

I'm not keen on casting away the function pointer type.  Normally we'd just
wrap it in a local function, to make it clear it was deliberate and avoid
potential nasty problems if the signature of the function ever changes.

It's 3 lines of boilerplate, but will give me warm fuzzy feelings!
Same for the other case above.  The fact this isn't done in exising kernel code
make this particularly risky.  

Makes sense. I will revert the code back to its original and handle the cases using goto
and inside remove()

Ah.  Not what I meant.  I was suggesting you add a little function locally
that has the right type and in turn calls cancel_delayed_work().

As that directly exposes the actual function calls, any signature change in future
will cause compile breakage (or be picked up any automated tools doing that refactor).

quoted

quoted

+				 &ams->ams_unmask_work);
+
+	ret = ams_init_device(ams);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize AMS\n");
+		return ret;
+	}
+
+	ret = ams_parse_dt(indio_dev, pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "failure in parsing DT\n");
+		return ret;
+	}
+
+	ams_enable_channel_sequence(indio_dev);
+
+	ams->irq = platform_get_irq(pdev, 0);  

platform_get_irq () can return errors, in particular -EPROBE_DEFER so I'd
check that and return before you call devm_request_irq() I'm not sure
devm_request_irq() will not eat that error code.
  

Will fix this in next series.

quoted

quoted

+	ret = devm_request_irq(&pdev->dev, ams->irq, &ams_irq, 0, "ams-  

irq",  

quoted

+			       indio_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register interrupt\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	return iio_device_register(indio_dev); }
+
+static int ams_remove(struct platform_device *pdev) {
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+	iio_device_unregister(indio_dev);  

If this is all you have in remove, then you can use devm_iio_device_register()
in probe() and not need an remove() callback at all.  

I think remove will have more functions since I am getting rid of devm_add_action_or_reset()

See above.

J

quoted

quoted

+
+	return 0;
+}
+  

...