Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On Sun, 21 May 2017 21:06:32 +0200 Corentin Labbewrote: > On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: > > For the sake of DT binding stability, this IIO driver is a child of an > > MFD driver for Allwinner A10, A13 and A31 because there already exists a > > DT binding for this IP. The MFD driver has a DT node but the IIO driver > > does not. > > > > The IIO device registers the temperature sensor in the thermal framework > > using the DT node of the parent, the MFD device, so the thermal > > framework could match the phandle to the MFD device in the DT and the > > struct device used to register in the thermal framework. > > > > devm_thermal_zone_of_sensor_register was previously used to register the > > thermal sensor with the parent struct device of the IIO device, > > representing the MFD device. By doing so, we registered actually the > > parent in the devm routine and not the actual IIO device. > > > > This lead to the devm unregister function not being called when the IIO > > module driver is removed. It resulted in the thermal framework still > > polling the get_temp function of the IIO module while the device doesn't > > exist anymore, thus generated a kernel panic. > > > > Use the non-devm function instead and do the unregister manually in the > > remove function. > > > > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") > > > > Signed-off-by: Quentin Schulz > > Reported-by: Corentin Labbe > > --- > > Tested-by: Corentin Labbe Thanks. I'm afraid this has gone into a non rebasing tree (and upstream for that matter) so I can't now apply this. We will have to fall back on the email record. Thanks Jonathan > > Thanks
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On Sun, 21 May 2017 21:06:32 +0200 Corentin Labbe wrote: > On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: > > For the sake of DT binding stability, this IIO driver is a child of an > > MFD driver for Allwinner A10, A13 and A31 because there already exists a > > DT binding for this IP. The MFD driver has a DT node but the IIO driver > > does not. > > > > The IIO device registers the temperature sensor in the thermal framework > > using the DT node of the parent, the MFD device, so the thermal > > framework could match the phandle to the MFD device in the DT and the > > struct device used to register in the thermal framework. > > > > devm_thermal_zone_of_sensor_register was previously used to register the > > thermal sensor with the parent struct device of the IIO device, > > representing the MFD device. By doing so, we registered actually the > > parent in the devm routine and not the actual IIO device. > > > > This lead to the devm unregister function not being called when the IIO > > module driver is removed. It resulted in the thermal framework still > > polling the get_temp function of the IIO module while the device doesn't > > exist anymore, thus generated a kernel panic. > > > > Use the non-devm function instead and do the unregister manually in the > > remove function. > > > > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") > > > > Signed-off-by: Quentin Schulz > > Reported-by: Corentin Labbe > > --- > > Tested-by: Corentin Labbe Thanks. I'm afraid this has gone into a non rebasing tree (and upstream for that matter) so I can't now apply this. We will have to fall back on the email record. Thanks Jonathan > > Thanks
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: > For the sake of DT binding stability, this IIO driver is a child of an > MFD driver for Allwinner A10, A13 and A31 because there already exists a > DT binding for this IP. The MFD driver has a DT node but the IIO driver > does not. > > The IIO device registers the temperature sensor in the thermal framework > using the DT node of the parent, the MFD device, so the thermal > framework could match the phandle to the MFD device in the DT and the > struct device used to register in the thermal framework. > > devm_thermal_zone_of_sensor_register was previously used to register the > thermal sensor with the parent struct device of the IIO device, > representing the MFD device. By doing so, we registered actually the > parent in the devm routine and not the actual IIO device. > > This lead to the devm unregister function not being called when the IIO > module driver is removed. It resulted in the thermal framework still > polling the get_temp function of the IIO module while the device doesn't > exist anymore, thus generated a kernel panic. > > Use the non-devm function instead and do the unregister manually in the > remove function. > > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") > > Signed-off-by: Quentin Schulz> Reported-by: Corentin Labbe > --- Tested-by: Corentin Labbe Thanks
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: > For the sake of DT binding stability, this IIO driver is a child of an > MFD driver for Allwinner A10, A13 and A31 because there already exists a > DT binding for this IP. The MFD driver has a DT node but the IIO driver > does not. > > The IIO device registers the temperature sensor in the thermal framework > using the DT node of the parent, the MFD device, so the thermal > framework could match the phandle to the MFD device in the DT and the > struct device used to register in the thermal framework. > > devm_thermal_zone_of_sensor_register was previously used to register the > thermal sensor with the parent struct device of the IIO device, > representing the MFD device. By doing so, we registered actually the > parent in the devm routine and not the actual IIO device. > > This lead to the devm unregister function not being called when the IIO > module driver is removed. It resulted in the thermal framework still > polling the get_temp function of the IIO module while the device doesn't > exist anymore, thus generated a kernel panic. > > Use the non-devm function instead and do the unregister manually in the > remove function. > > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") > > Signed-off-by: Quentin Schulz > Reported-by: Corentin Labbe > --- Tested-by: Corentin Labbe Thanks
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On 18/05/17 07:51, Maxime Ripard wrote: On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: For the sake of DT binding stability, this IIO driver is a child of an MFD driver for Allwinner A10, A13 and A31 because there already exists a DT binding for this IP. The MFD driver has a DT node but the IIO driver does not. The IIO device registers the temperature sensor in the thermal framework using the DT node of the parent, the MFD device, so the thermal framework could match the phandle to the MFD device in the DT and the struct device used to register in the thermal framework. devm_thermal_zone_of_sensor_register was previously used to register the thermal sensor with the parent struct device of the IIO device, representing the MFD device. By doing so, we registered actually the parent in the devm routine and not the actual IIO device. This lead to the devm unregister function not being called when the IIO module driver is removed. It resulted in the thermal framework still polling the get_temp function of the IIO module while the device doesn't exist anymore, thus generated a kernel panic. Use the non-devm function instead and do the unregister manually in the remove function. Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") Signed-off-by: Quentin SchulzReported-by: Corentin Labbe Reviewed-by: Maxime Ripard Maxime Thanks to all involved with tracking this one down. Jonathan
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On 18/05/17 07:51, Maxime Ripard wrote: On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: For the sake of DT binding stability, this IIO driver is a child of an MFD driver for Allwinner A10, A13 and A31 because there already exists a DT binding for this IP. The MFD driver has a DT node but the IIO driver does not. The IIO device registers the temperature sensor in the thermal framework using the DT node of the parent, the MFD device, so the thermal framework could match the phandle to the MFD device in the DT and the struct device used to register in the thermal framework. devm_thermal_zone_of_sensor_register was previously used to register the thermal sensor with the parent struct device of the IIO device, representing the MFD device. By doing so, we registered actually the parent in the devm routine and not the actual IIO device. This lead to the devm unregister function not being called when the IIO module driver is removed. It resulted in the thermal framework still polling the get_temp function of the IIO module while the device doesn't exist anymore, thus generated a kernel panic. Use the non-devm function instead and do the unregister manually in the remove function. Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") Signed-off-by: Quentin Schulz Reported-by: Corentin Labbe Reviewed-by: Maxime Ripard Maxime Thanks to all involved with tracking this one down. Jonathan
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: > For the sake of DT binding stability, this IIO driver is a child of an > MFD driver for Allwinner A10, A13 and A31 because there already exists a > DT binding for this IP. The MFD driver has a DT node but the IIO driver > does not. > > The IIO device registers the temperature sensor in the thermal framework > using the DT node of the parent, the MFD device, so the thermal > framework could match the phandle to the MFD device in the DT and the > struct device used to register in the thermal framework. > > devm_thermal_zone_of_sensor_register was previously used to register the > thermal sensor with the parent struct device of the IIO device, > representing the MFD device. By doing so, we registered actually the > parent in the devm routine and not the actual IIO device. > > This lead to the devm unregister function not being called when the IIO > module driver is removed. It resulted in the thermal framework still > polling the get_temp function of the IIO module while the device doesn't > exist anymore, thus generated a kernel panic. > > Use the non-devm function instead and do the unregister manually in the > remove function. > > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") > > Signed-off-by: Quentin Schulz> Reported-by: Corentin Labbe Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [PATCH v2] iio: adc: sun4i-gpadc-iio: fix parent device being used in devm function
On Thu, May 18, 2017 at 08:36:07AM +0200, Quentin Schulz wrote: > For the sake of DT binding stability, this IIO driver is a child of an > MFD driver for Allwinner A10, A13 and A31 because there already exists a > DT binding for this IP. The MFD driver has a DT node but the IIO driver > does not. > > The IIO device registers the temperature sensor in the thermal framework > using the DT node of the parent, the MFD device, so the thermal > framework could match the phandle to the MFD device in the DT and the > struct device used to register in the thermal framework. > > devm_thermal_zone_of_sensor_register was previously used to register the > thermal sensor with the parent struct device of the IIO device, > representing the MFD device. By doing so, we registered actually the > parent in the devm routine and not the actual IIO device. > > This lead to the devm unregister function not being called when the IIO > module driver is removed. It resulted in the thermal framework still > polling the get_temp function of the IIO module while the device doesn't > exist anymore, thus generated a kernel panic. > > Use the non-devm function instead and do the unregister manually in the > remove function. > > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC") > > Signed-off-by: Quentin Schulz > Reported-by: Corentin Labbe Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature