On 12/09/2016 15:56, Lee Jones wrote:

On Mon, 12 Sep 2016, Quentin Schulz wrote:

quoted

On 12/09/2016 11:59, Lee Jones wrote:

quoted

On Mon, 12 Sep 2016, Quentin Schulz wrote:

quoted

On 12/09/2016 11:18, Lee Jones wrote:

quoted

On Thu, 08 Sep 2016, Quentin Schulz wrote:
[...]

quoted

+
+MODULE_DEVICE_TABLE(of, sun4i_gpadc_mfd_of_match);

Place this directly under the table.

quoted

+static struct platform_driver sun4i_gpadc_mfd_driver = {
+	.driver = {
+		.name = "sun4i-adc-mfd",
+		.of_match_table = of_match_ptr(sun4i_gpadc_mfd_of_match),
+	},
+	.probe = sun4i_gpadc_mfd_probe,

No .remove?

No, everything in probe is handled with devm functions.

Don't you need to undo the register write you did?

The regmap_write I use is there to disable all interrupts on hardware
side before the irq_chip handles all interrupts by itself. The
interrupts are not used in the MFD driver.

Thus, I chose to disable the hardware interrupts in the remove function
of drivers using the interrupts (only the IIO yet but the touchscreen
driver later also which will be using a third interrupt). When the MFD
driver is removed, the MFD cells will all be removed, thus calling their
own remove functions, thus disabling hardware interrupts used in each
driver. So the hardware interrupts disabling would be called twice.

This does send some little alarm bells ringing.  I'd normally expect
the .remove function to undo everything you did in .probe.  So, if you
are disabling the IRQs from within the leaf drivers, shouldn't you be
initialising them in the leaf driver's respective .probes?

I use the regmap_write in the MFD driver's probe to disable all
interrupts before requesting irq_chip to guarantee the interrupts are in
a known state, being disabled. It is to insure no interrupt will occur
unwittingly before we want the leaf drivers to handle them.

The disabling of irqs in the remove is handled rather by
devm_regmap_del_irq_chip than by an explicit regmap_write in the
driver's removal function. It performs the exact same thing.

I always use devm functions for requesting either an irq_chip or the
irqs themselves. In that case, when the device is removed, the irqs are
freed on leaf drivers' (where the irqs are requested) removal while the
removal of irq_chip in the MFD driver will also free all irqs mapped to
this irq_chip thanks to devm_regmap_del_irq_chip. Therefore, the
interrupts are disabled by devm functions.

The regmap_update_bits in probe and removal of the ADC driver to disable
irqs are actually redundant because the devm functions already handle
the irqs disabling.

Thanks,
Quentin