Hi there, > Am 19.03.2023 um 16:32 schrieb Jonathan Cameron <jic23@xxxxxxxxxx>: > > On Sat, 18 Mar 2023 20:22:53 +0100 > Patrik Dahlström <risca@xxxxxxxxxxxxxx> wrote: > >> On Sat, Mar 18, 2023 at 04:30:33PM +0000, Jonathan Cameron wrote: >>> On Mon, 13 Mar 2023 21:50:29 +0100 >>> Patrik Dahlström <risca@xxxxxxxxxxxxxx> wrote: >>> >>>> Calling dev_to_iio_dev() on a platform device pointer is undefined and >>>> will make adc NULL. >>>> >>>> Signed-off-by: Patrik Dahlström <risca@xxxxxxxxxxxxxx> >>> >>> Hi Patrik, >>> >>> Looks good so applied to the fixes-togreg branch of iio.git. >>> >>> Whilst we are here, this would be a trivial driver to take fully device >>> managed. The only slightly messy bit is that it would need >>> a devm_add_action_or_reset() + custom callback to handle the >>> device_wakeup_enable(). >>> >>> On the off chance you can test it I'll send a patch in a few mins. >>> Note that will depend on this one going up stream first and that >>> I haven't done more than build test it. >> I got the patch and it looks good, but it will take a few days before I >> have the time to test it. >> >> I have some more patches coming for this driver to configure the adc >> thresholds from userspace, Yes, that is a useful feature. >> employing the iio channel event subsystem, but >> they need a bit more work. In particular, to ensure backwards compatibility >> with the adc_wakeupX_data platform data. However, I don't see this platform >> data being used by anyone. >> How important is it to retain support for adc_wakeupX_data? > > It's a somewhat unusual feature, so I doubt it was implemented without someone > needing it. However as you observe there is no upstream user. > > As it is causing you problems, I'd just rip out the palmas_adc_platform_data > completely and see if anyone objects. You can do that as a standalone patch > prior to posting your events stuff if you like. Or hopefully > H. Nikolaus Schaller might be able to give us some background on why > that feature is there but not used. I also have no idea. The original author was Pradeep Goudagunta <pgoudagunta@xxxxxxxxxx> and I just copied it from https://android.googlesource.com/kernel/tegra/+/aaabb2e045f31e5a970109ffdaae900dd403d17e/drivers/staging/iio/adc/palmas_gpadc.c polished the code and made it compile & work some years ago. So it may have been useful in a some Tegra Android kernel from 2013. Maybe it is for special power management (at least that is how I interpret the "wakeup"). But I found some hint which device it is good for: https://lore.kernel.org/all/1379509642-19227-2-git-send-email-ldewangan@xxxxxxxxxx/T/ "PALMAS PMIC is used on Dalmore platform." https://docs.nvidia.com/gameworks/content/devices/dalmore_devkit_main.htm And there seems to be an upstream DTS: arch/arm/boot/dts/tegra114-dalmore.dts but without gpadc support. That is quite common that upstream DTS are incomplete so we can't deduce that there are no users of a feature. BR, Nikolaus > >>> >>> Thanks, >>> >>> Jonathan >> >> Thank you for going the extra mile :) > > No problem. I jumped on the opportunity to get it tested - takes way longer > than writing a little patch like that ;) > > Jonathan > >>> >>> >>>> --- >>>> drivers/iio/adc/palmas_gpadc.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c >>>> index 61e80bf3d05e..6db6f3bc768a 100644 >>>> --- a/drivers/iio/adc/palmas_gpadc.c >>>> +++ b/drivers/iio/adc/palmas_gpadc.c >>>> @@ -638,7 +638,7 @@ static int palmas_gpadc_probe(struct platform_device *pdev) >>>> >>>> static int palmas_gpadc_remove(struct platform_device *pdev) >>>> { >>>> - struct iio_dev *indio_dev = dev_to_iio_dev(&pdev->dev); >>>> + struct iio_dev *indio_dev = dev_get_drvdata(&pdev->dev); >>>> struct palmas_gpadc *adc = iio_priv(indio_dev); >>>> >>>> if (adc->wakeup1_enable || adc->wakeup2_enable) >>> >
