The palmas gpadc block has support for monitoring up to 2 ADC channels
and issue an interrupt if they reach past a set threshold. This can be
configured statically with device tree today, but it only gets enabled
when reaching sleep mode. Also, it doesn't look like anyone is using it.
Instead of this one special case, change the code so userspace can
configure the ADC channels to their own needs through the iio events
subsystem. The high and low threshold values can be set for every
channel, but only 2 thresholds can be enabled at a time. Trying to
enable more than 2 thresholds will result in an error.
The configured thresholds will wake up the system from sleep mode if
wakeup is enabled in /sys/devices/.../power/wakeup.
The old platform data was removed.
Thresholds, events, and wakeup were tested on omap5-uevm board. It wakes
up from sleep mode when wakeup is enabled and a threshold is passed. A
userspace tool for monitoring events and adjusting thresholds can be
found at [3].
Patrik Dahlström (7):
iio: adc: palmas: remove adc_wakeupX_data
iio: adc: palmas: replace "wakeup" with "event"
iio: adc: palmas: use iio_event_direction for threshold polarity
iio: adc: palmas: move eventX_enable into palmas_adc_event
iio: adc: palmas: always reset events on unload
iio: adc: palmas: add support for iio threshold events
iio: adc: palmas: don't alter event config on suspend/resume
drivers/iio/adc/palmas_gpadc.c | 550 +++++++++++++++++++++++++++------
include/linux/mfd/palmas.h | 8 -
2 files changed, 455 insertions(+), 103 deletions(-)
base-commit: 37fd83916da2e4cae03d350015c82a67b1b334c4
prerequisite-patch-id: 9b1f55610800b91b721d042bf7f33b58179237d1
prerequisite-patch-id: b0418c707db13f514400956596e9ebe91c25bba0
--
2.25.1
Instead of having high_threshold > 0 as an indicator for upper threshold
event and lower threshold event otherwise, use enum iio_event_direction
instead. This is hopefully less ambiguous.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 36 ++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index 3ef0e1c08c8b..fca455a4285e 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -77,9 +77,9 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
};
struct palmas_adc_event {
- int adc_channel_number;
- int adc_high_threshold;
- int adc_low_threshold;
+ int channel;
+ int raw_thresh;
+ enum iio_event_direction direction;
};
/*
@@ -617,16 +617,21 @@ static int palmas_adc_configure_events(struct palmas_gpadc *adc)
conv = 0;
if (adc->event0_enable) {
+ struct palmas_adc_event *ev = &adc->event0;
int polarity;
- ch0 = adc->event0.adc_channel_number;
+ ch0 = ev->channel;
+ thres = ev->raw_thresh;
conv |= PALMAS_GPADC_AUTO_CTRL_AUTO_CONV0_EN;
- if (adc->event0.adc_high_threshold > 0) {
- thres = adc->event0.adc_high_threshold;
+ switch (ev->direction) {
+ case IIO_EV_DIR_RISING:
polarity = 0;
- } else {
- thres = adc->event0.adc_low_threshold;
+ break;
+ case IIO_EV_DIR_FALLING:
polarity = PALMAS_GPADC_THRES_CONV0_MSB_THRES_CONV0_POL;
+ break;
+ default:
+ return -EINVAL;
}
ret = palmas_write(adc->palmas, PALMAS_GPADC_BASE,
@@ -648,16 +653,21 @@ static int palmas_adc_configure_events(struct palmas_gpadc *adc)
}
if (adc->event1_enable) {
+ struct palmas_adc_event *ev = &adc->event1;
int polarity;
- ch1 = adc->event1.adc_channel_number;
+ ch1 = ev->channel;
+ thres = ev->raw_thresh;
conv |= PALMAS_GPADC_AUTO_CTRL_AUTO_CONV1_EN;
- if (adc->event1.adc_high_threshold > 0) {
- thres = adc->event1.adc_high_threshold;
+ switch (ev->direction) {
+ case IIO_EV_DIR_RISING:
polarity = 0;
- } else {
- thres = adc->event1.adc_low_threshold;
+ break;
+ case IIO_EV_DIR_FALLING:
polarity = PALMAS_GPADC_THRES_CONV1_MSB_THRES_CONV1_POL;
+ break;
+ default:
+ return -EINVAL;
}
ret = palmas_write(adc->palmas, PALMAS_GPADC_BASE,
--
2.25.1
It just makes more sense to have all information regarding adc events in
one place.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index fca455a4285e..f7eb445495b6 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -77,6 +77,7 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
};
struct palmas_adc_event {
+ bool enabled;
int channel;
int raw_thresh;
enum iio_event_direction direction;
@@ -119,8 +120,6 @@ struct palmas_gpadc {
struct completion conv_completion;
struct palmas_adc_event event0;
struct palmas_adc_event event1;
- bool event0_enable;
- bool event1_enable;
int auto_conversion_period;
struct mutex lock;
};
@@ -616,7 +615,7 @@ static int palmas_adc_configure_events(struct palmas_gpadc *adc)
}
conv = 0;
- if (adc->event0_enable) {
+ if (adc->event0.enabled) {
struct palmas_adc_event *ev = &adc->event0;
int polarity;
@@ -652,7 +651,7 @@ static int palmas_adc_configure_events(struct palmas_gpadc *adc)
}
}
- if (adc->event1_enable) {
+ if (adc->event1.enabled) {
struct palmas_adc_event *ev = &adc->event1;
int polarity;
@@ -727,7 +726,7 @@ static int palmas_gpadc_suspend(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct palmas_gpadc *adc = iio_priv(indio_dev);
- int wakeup = adc->event0_enable || adc->event1_enable;
+ int wakeup = adc->event0.enabled || adc->event1.enabled;
int ret;
if (!device_may_wakeup(dev) || !wakeup)
@@ -737,10 +736,10 @@ static int palmas_gpadc_suspend(struct device *dev)
if (ret < 0)
return ret;
- if (adc->event0_enable)
+ if (adc->event0.enabled)
enable_irq_wake(adc->irq_auto_0);
- if (adc->event1_enable)
+ if (adc->event1.enabled)
enable_irq_wake(adc->irq_auto_1);
return 0;
@@ -750,7 +749,7 @@ static int palmas_gpadc_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct palmas_gpadc *adc = iio_priv(indio_dev);
- int wakeup = adc->event0_enable || adc->event1_enable;
+ int wakeup = adc->event0.enabled || adc->event1.enabled;
int ret;
if (!device_may_wakeup(dev) || !wakeup)
@@ -760,10 +759,10 @@ static int palmas_gpadc_resume(struct device *dev)
if (ret < 0)
return ret;
- if (adc->event0_enable)
+ if (adc->event0.enabled)
disable_irq_wake(adc->irq_auto_0);
- if (adc->event1_enable)
+ if (adc->event1.enabled)
disable_irq_wake(adc->irq_auto_1);
return 0;
--
2.25.1
It does not seem to be used by anyone and later patches in this series
are made simpler by first removing this. There is now a lot of dead code
that cannot be reached, until later patches revive it. Arguably, this is
preferred over removing the code only to add it again.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 50 ++++------------------------------
include/linux/mfd/palmas.h | 8 ------
2 files changed, 6 insertions(+), 52 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index 24d7c096e4b8..943ac579eb1f 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -76,6 +76,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
PALMAS_ADC_INFO(IN15, 0, 0, 0, 0, INVALID, INVALID, true),
};
+struct palmas_adc_wakeup_property {
+ int adc_channel_number;
+ int adc_high_threshold;
+ int adc_low_threshold;
+};
+
/*
* struct palmas_gpadc - the palmas_gpadc structure
* @ch0_current: channel 0 current source setting
@@ -492,11 +498,6 @@ static int palmas_gpadc_get_adc_dt_data(struct platform_device *pdev,
return 0;
}
-static void palmas_disable_wakeup(void *dev)
-{
- device_wakeup_disable(dev);
-}
-
static int palmas_gpadc_probe(struct platform_device *pdev)
{
struct palmas_gpadc *adc;
@@ -547,36 +548,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
return dev_err_probe(adc->dev, ret,
"request irq %d failed\n", adc->irq);
- if (gpadc_pdata->adc_wakeup1_data) {
- memcpy(&adc->wakeup1_data, gpadc_pdata->adc_wakeup1_data,
- sizeof(adc->wakeup1_data));
- adc->wakeup1_enable = true;
- adc->irq_auto_0 = platform_get_irq(pdev, 1);
- ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_0,
- NULL, palmas_gpadc_irq_auto,
- IRQF_ONESHOT,
- "palmas-adc-auto-0", adc);
- if (ret < 0)
- return dev_err_probe(adc->dev, ret,
- "request auto0 irq %d failed\n",
- adc->irq_auto_0);
- }
-
- if (gpadc_pdata->adc_wakeup2_data) {
- memcpy(&adc->wakeup2_data, gpadc_pdata->adc_wakeup2_data,
- sizeof(adc->wakeup2_data));
- adc->wakeup2_enable = true;
- adc->irq_auto_1 = platform_get_irq(pdev, 2);
- ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_1,
- NULL, palmas_gpadc_irq_auto,
- IRQF_ONESHOT,
- "palmas-adc-auto-1", adc);
- if (ret < 0)
- return dev_err_probe(adc->dev, ret,
- "request auto1 irq %d failed\n",
- adc->irq_auto_1);
- }
-
/* set the current source 0 (value 0/5/15/20 uA => 0..3) */
if (gpadc_pdata->ch0_current <= 1)
adc->ch0_current = PALMAS_ADC_CH0_CURRENT_SRC_0;
@@ -616,15 +587,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
palmas_gpadc_calibrate(adc, i);
}
- if (adc->wakeup1_enable || adc->wakeup2_enable) {
- device_wakeup_enable(&pdev->dev);
- ret = devm_add_action_or_reset(&pdev->dev,
- palmas_disable_wakeup,
- &pdev->dev);
- if (ret)
- return ret;
- }
-
return 0;
}
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 1e61c7e9f50d..55f22adb1a9e 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -129,12 +129,6 @@ struct palmas_pmic_driver_data {
struct regulator_config config);
};
-struct palmas_adc_wakeup_property {
- int adc_channel_number;
- int adc_high_threshold;
- int adc_low_threshold;
-};
-
struct palmas_gpadc_platform_data {
/* Channel 3 current source is only enabled during conversion */
int ch3_current; /* 0: off; 1: 10uA; 2: 400uA; 3: 800 uA */
@@ -153,8 +147,6 @@ struct palmas_gpadc_platform_data {
int start_polarity;
int auto_conversion_period_ms;
- struct palmas_adc_wakeup_property *adc_wakeup1_data;
- struct palmas_adc_wakeup_property *adc_wakeup2_data;
};
struct palmas_reg_init {
--
2.25.1
The event config is controlled through the IIO events subsystem and
device wakeup is controlled by /sys/devices/.../power/wakeup. Let's keep
those two knobs independent.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index f693d69a10a8..d4e1e80296d0 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -1127,16 +1127,10 @@ static int palmas_gpadc_suspend(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct palmas_gpadc *adc = iio_priv(indio_dev);
- int wakeup = adc->event0.enabled || adc->event1.enabled;
- int ret;
- if (!device_may_wakeup(dev) || !wakeup)
+ if (!device_may_wakeup(dev))
return 0;
- ret = palmas_adc_configure_events(adc);
- if (ret < 0)
- return ret;
-
if (adc->event0.enabled)
enable_irq_wake(adc->irq_auto_0);
@@ -1150,16 +1144,10 @@ static int palmas_gpadc_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct palmas_gpadc *adc = iio_priv(indio_dev);
- int wakeup = adc->event0.enabled || adc->event1.enabled;
- int ret;
- if (!device_may_wakeup(dev) || !wakeup)
+ if (!device_may_wakeup(dev))
return 0;
- ret = palmas_adc_reset_events(adc);
- if (ret < 0)
- return ret;
-
if (adc->event0.enabled)
disable_irq_wake(adc->irq_auto_0);
--
2.25.1
The palmas gpadc block has support for monitoring up to 2 ADC channels
and issue an interrupt if they reach past a set threshold. This change
hooks into the IIO events system and exposes to userspace the ability to
configure these threshold values for each channel, but only allow up to
2 such thresholds to be enabled at any given time. Trying to enable a
third channel will result in an error.
Userspace is expected to input calibrated, as opposed to raw, values as
threshold. However, it is not enough to do the opposite of what is done
when converting the other way around. To account for tolerances in the
ADC, the calculated raw threshold should be adjusted based on the ADC
specifications for the device. These specifications include the integral
nonlinearity (INL), offset, and gain error. To adjust the high
threshold, use the following equation:
(calibrated value + INL) * Gain error + offset = maximum value [1]
Likewise, use the following equation for the low threshold:
(calibrated value - INL) * Gain error - offset = minimum value
The gain error is a combination of gain error, as listed in the
datasheet, and gain error drift due to temperature and supply. The exact
values for these specifications vary between palmas devices. This patch
sets the values found in TWL6035, TWL6037 datasheet.
[1] TI Application Report, SLIA087A, Guide to Using the GPADC in
TPS65903x, TPS65917-Q1, TPS65919-Q1, and TPS65916 Devices.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 438 +++++++++++++++++++++++++++++++--
1 file changed, 414 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index 90fc20205df0..f693d69a10a8 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -20,6 +20,7 @@
#include <linux/completion.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/iio/events.h>
#include <linux/iio/iio.h>
#include <linux/iio/machine.h>
#include <linux/iio/driver.h>
@@ -79,10 +80,14 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
struct palmas_adc_event {
bool enabled;
int channel;
- int raw_thresh;
enum iio_event_direction direction;
};
+struct palmas_gpadc_thresholds {
+ int high;
+ int low;
+};
+
/*
* struct palmas_gpadc - the palmas_gpadc structure
* @ch0_current: channel 0 current source setting
@@ -120,10 +125,31 @@ struct palmas_gpadc {
struct completion conv_completion;
struct palmas_adc_event event0;
struct palmas_adc_event event1;
+ struct palmas_gpadc_thresholds thresholds[PALMAS_ADC_CH_MAX];
int auto_conversion_period;
struct mutex lock;
};
+static struct palmas_adc_event *palmas_gpadc_get_event(struct palmas_gpadc *adc,
+ int adc_chan,
+ enum iio_event_direction dir)
+{
+ if (adc_chan == adc->event0.channel && dir == adc->event0.direction)
+ return &adc->event0;
+
+ if (adc_chan == adc->event1.channel && dir == adc->event1.direction)
+ return &adc->event1;
+
+ return NULL;
+}
+
+static bool palmas_gpadc_channel_is_freerunning(struct palmas_gpadc *adc,
+ int adc_chan)
+{
+ return palmas_gpadc_get_event(adc, adc_chan, IIO_EV_DIR_RISING) ||
+ palmas_gpadc_get_event(adc, adc_chan, IIO_EV_DIR_FALLING);
+}
+
/*
* GPADC lock issue in AUTO mode.
* Impact: In AUTO mode, GPADC conversion can be locked after disabling AUTO
@@ -193,11 +219,24 @@ static irqreturn_t palmas_gpadc_irq(int irq, void *data)
static irqreturn_t palmas_gpadc_irq_auto(int irq, void *data)
{
- struct palmas_gpadc *adc = data;
+ struct iio_dev *indio_dev = data;
+ struct palmas_gpadc *adc = iio_priv(indio_dev);
+ struct palmas_adc_event *ev;
dev_dbg(adc->dev, "Threshold interrupt %d occurs\n", irq);
palmas_disable_auto_conversion(adc);
+ ev = (irq == adc->irq_auto_0) ? &adc->event0 : &adc->event1;
+ if (ev->channel != -1) {
+ enum iio_event_direction dir;
+ u64 code;
+
+ dir = ev->direction;
+ code = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, ev->channel,
+ IIO_EV_TYPE_THRESH, dir);
+ iio_push_event(indio_dev, code, iio_get_time_ns(indio_dev));
+ }
+
return IRQ_HANDLED;
}
@@ -285,6 +324,9 @@ static int palmas_gpadc_read_prepare(struct palmas_gpadc *adc, int adc_chan)
{
int ret;
+ if (palmas_gpadc_channel_is_freerunning(adc, adc_chan))
+ return 0; /* ADC already running */
+
ret = palmas_gpadc_enable(adc, adc_chan, true);
if (ret < 0)
return ret;
@@ -344,28 +386,43 @@ static int palmas_gpadc_start_conversion(struct palmas_gpadc *adc, int adc_chan)
unsigned int val;
int ret;
- init_completion(&adc->conv_completion);
- ret = palmas_update_bits(adc->palmas, PALMAS_GPADC_BASE,
- PALMAS_GPADC_SW_SELECT,
- PALMAS_GPADC_SW_SELECT_SW_START_CONV0,
- PALMAS_GPADC_SW_SELECT_SW_START_CONV0);
- if (ret < 0) {
- dev_err(adc->dev, "SELECT_SW_START write failed: %d\n", ret);
- return ret;
- }
+ if (palmas_gpadc_channel_is_freerunning(adc, adc_chan)) {
+ int event = (adc_chan == adc->event0.channel) ? 0 : 1;
+ unsigned int reg = (event == 0) ?
+ PALMAS_GPADC_AUTO_CONV0_LSB :
+ PALMAS_GPADC_AUTO_CONV1_LSB;
- ret = wait_for_completion_timeout(&adc->conv_completion,
- PALMAS_ADC_CONVERSION_TIMEOUT);
- if (ret == 0) {
- dev_err(adc->dev, "conversion not completed\n");
- return -ETIMEDOUT;
- }
+ ret = palmas_bulk_read(adc->palmas, PALMAS_GPADC_BASE,
+ reg, &val, 2);
+ if (ret < 0) {
+ dev_err(adc->dev, "AUTO_CONV%x_LSB read failed: %d\n",
+ event, ret);
+ return ret;
+ }
+ } else {
+ init_completion(&adc->conv_completion);
+ ret = palmas_update_bits(adc->palmas, PALMAS_GPADC_BASE,
+ PALMAS_GPADC_SW_SELECT,
+ PALMAS_GPADC_SW_SELECT_SW_START_CONV0,
+ PALMAS_GPADC_SW_SELECT_SW_START_CONV0);
+ if (ret < 0) {
+ dev_err(adc->dev, "SELECT_SW_START write failed: %d\n", ret);
+ return ret;
+ }
- ret = palmas_bulk_read(adc->palmas, PALMAS_GPADC_BASE,
- PALMAS_GPADC_SW_CONV0_LSB, &val, 2);
- if (ret < 0) {
- dev_err(adc->dev, "SW_CONV0_LSB read failed: %d\n", ret);
- return ret;
+ ret = wait_for_completion_timeout(&adc->conv_completion,
+ PALMAS_ADC_CONVERSION_TIMEOUT);
+ if (ret == 0) {
+ dev_err(adc->dev, "conversion not completed\n");
+ return -ETIMEDOUT;
+ }
+
+ ret = palmas_bulk_read(adc->palmas, PALMAS_GPADC_BASE,
+ PALMAS_GPADC_SW_CONV0_LSB, &val, 2);
+ if (ret < 0) {
+ dev_err(adc->dev, "SW_CONV0_LSB read failed: %d\n", ret);
+ return ret;
+ }
}
ret = val & 0xFFF;
@@ -390,6 +447,98 @@ static int palmas_gpadc_get_calibrated_code(struct palmas_gpadc *adc,
return val;
}
+/**
+ * The high and low threshold values are calculated based on the advice given
+ * in TI Application Report SLIA087A, "Guide to Using the GPADC in PS65903x,
+ * TPS65917-Q1, TPS65919-Q1, and TPS65916 Devices". This document recommend
+ * taking ADC tolerances into account and is based on the device integral non-
+ * linearity (INL), offset error and gain error:
+ *
+ * raw high threshold = (ideal threshold + INL) * gain error + offset error
+ *
+ * The gain error include both gain error, as specified in the datasheet, and
+ * the gain error drift. These paramenters vary depending on device and whether
+ * the the channel is calibrated (trimmed) or not.
+ */
+static int palmas_gpadc_threshold_with_tolerance(int val, const int INL,
+ const int gain_error,
+ const int offset_error)
+{
+ val = ((val + INL) * (1000 + gain_error)) / 1000 + offset_error;
+
+ return clamp(val, 0, 0xFFF);
+}
+
+/**
+ * The values below are taken from the datasheet of TWL6035, TWL6037.
+ * todo: get max INL, gain error, and offset error from OF.
+ */
+static int palmas_gpadc_get_high_threshold_raw(struct palmas_gpadc *adc,
+ struct palmas_adc_event *ev)
+{
+ const int adc_chan = ev->channel;
+ int val = adc->thresholds[adc_chan].high;
+ /* integral nonlinearity, measured in LSB */
+ const int max_INL = 2;
+ /* measured in LSB */
+ int max_offset_error;
+ /* 0.2% when calibrated */
+ int max_gain_error = 2;
+
+ val = (val * 1000) / adc->adc_info[adc_chan].gain;
+
+ if (adc->adc_info[adc_chan].is_uncalibrated) {
+ /* 2% worse */
+ max_gain_error += 20;
+ max_offset_error = 36;
+ } else {
+ val = (val * adc->adc_info[adc_chan].gain_error +
+ adc->adc_info[adc_chan].offset) /
+ 1000;
+ max_offset_error = 2;
+ }
+
+ return palmas_gpadc_threshold_with_tolerance(val,
+ max_INL,
+ max_gain_error,
+ max_offset_error);
+}
+
+/**
+ * The values below are taken from the datasheet of TWL6035, TWL6037.
+ * todo: get min INL, gain error, and offset error from OF.
+ */
+static int palmas_gpadc_get_low_threshold_raw(struct palmas_gpadc *adc,
+ struct palmas_adc_event *ev)
+{
+ const int adc_chan = ev->channel;
+ int val = adc->thresholds[adc_chan].low;
+ /* integral nonlinearity, measured in LSB */
+ const int min_INL = -2;
+ /* measured in LSB */
+ int min_offset_error;
+ /* -0.6% when calibrated */
+ int min_gain_error = -6;
+
+ val = (val * 1000) / adc->adc_info[adc_chan].gain;
+
+ if (adc->adc_info[adc_chan].is_uncalibrated) {
+ /* 2% worse */
+ min_gain_error -= 20;
+ min_offset_error = -36;
+ } else {
+ val = (val * adc->adc_info[adc_chan].gain_error -
+ adc->adc_info[adc_chan].offset) /
+ 1000;
+ min_offset_error = -2;
+ }
+
+ return palmas_gpadc_threshold_with_tolerance(val,
+ min_INL,
+ min_gain_error,
+ min_offset_error);
+}
+
static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val, int *val2, long mask)
{
@@ -436,8 +585,212 @@ static int palmas_gpadc_read_raw(struct iio_dev *indio_dev,
return ret;
}
+static int palmas_gpadc_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct palmas_gpadc *adc = iio_priv(indio_dev);
+ int adc_chan = chan->channel;
+ int ret = 0;
+
+ if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
+ return -EINVAL;
+
+ mutex_lock(&adc->lock);
+
+ if (palmas_gpadc_get_event(adc, adc_chan, dir)) {
+ ret = 1;
+ }
+
+ mutex_unlock(&adc->lock);
+
+ return ret;
+}
+
+static int palmas_adc_configure_events(struct palmas_gpadc *adc);
+static int palmas_adc_reset_events(struct palmas_gpadc *adc);
+
+static int palmas_gpadc_reconfigure_event_channels(struct palmas_gpadc *adc)
+{
+ return (adc->event0.enabled || adc->event1.enabled) ?
+ palmas_adc_configure_events(adc) :
+ palmas_adc_reset_events(adc);
+}
+
+static int palmas_gpadc_enable_event_config(struct palmas_gpadc *adc,
+ const struct iio_chan_spec *chan,
+ enum iio_event_direction dir)
+{
+ struct palmas_adc_event *ev;
+ int adc_chan = chan->channel;
+
+ if (palmas_gpadc_get_event(adc, adc_chan, dir))
+ /* already enabled */
+ return 0;
+
+ if (adc->event0.channel == -1) {
+ ev = &adc->event0;
+ } else if (adc->event1.channel == -1) {
+ /* event0 has to be the lowest channel */
+ if (adc_chan < adc->event0.channel) {
+ adc->event1 = adc->event0;
+ ev = &adc->event0;
+ } else {
+ ev = &adc->event1;
+ }
+ } else { /* both AUTO channels already in use */
+ dev_warn(adc->dev, "event0 - %d, event1 - %d\n",
+ adc->event0.channel, adc->event1.channel);
+ return -EBUSY;
+ }
+
+ ev->enabled = true;
+ ev->channel = adc_chan;
+ ev->direction = dir;
+
+ return palmas_gpadc_reconfigure_event_channels(adc);
+}
+
+static int palmas_gpadc_disable_event_config(struct palmas_gpadc *adc,
+ const struct iio_chan_spec *chan,
+ enum iio_event_direction dir)
+{
+ int adc_chan = chan->channel;
+ struct palmas_adc_event *ev = palmas_gpadc_get_event(adc, adc_chan, dir);
+
+ if (!ev)
+ return 0;
+
+ if (ev == &adc->event0) {
+ adc->event0 = adc->event1;
+ ev = &adc->event1;
+ }
+
+ ev->enabled = false;
+ ev->channel = -1;
+ ev->direction = IIO_EV_DIR_NONE;
+
+ return palmas_gpadc_reconfigure_event_channels(adc);
+}
+
+static int palmas_gpadc_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ int state)
+{
+ struct palmas_gpadc *adc = iio_priv(indio_dev);
+ int adc_chan = chan->channel;
+ int ret = 0;
+
+ if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
+ return -EINVAL;
+
+ mutex_lock(&adc->lock);
+
+ if (state)
+ ret = palmas_gpadc_enable_event_config(adc, chan, dir);
+ else
+ ret = palmas_gpadc_disable_event_config(adc, chan, dir);
+
+ mutex_unlock(&adc->lock);
+
+ return ret;
+}
+
+static int palmas_gpadc_read_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int *val, int *val2)
+{
+ struct palmas_gpadc *adc = iio_priv(indio_dev);
+ int adc_chan = chan->channel;
+ int ret = 0;
+
+ if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
+ return -EINVAL;
+
+ mutex_lock(&adc->lock);
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ *val = (dir == IIO_EV_DIR_RISING) ?
+ adc->thresholds[adc_chan].high :
+ adc->thresholds[adc_chan].low;
+ ret = IIO_VAL_INT;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ mutex_unlock(&adc->lock);
+
+ return ret;
+}
+
+static int palmas_gpadc_write_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int val, int val2)
+{
+ struct palmas_gpadc *adc = iio_priv(indio_dev);
+ int adc_chan = chan->channel;
+ int ret = 0;
+
+ if (adc_chan > PALMAS_ADC_CH_MAX || type != IIO_EV_TYPE_THRESH)
+ return -EINVAL;
+
+ mutex_lock(&adc->lock);
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ if (val < 0 || val > 0xFFF) {
+ ret = -EINVAL;
+ break;
+ }
+ if (dir == IIO_EV_DIR_RISING)
+ adc->thresholds[adc_chan].high = val;
+ else
+ adc->thresholds[adc_chan].low = val;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (palmas_gpadc_get_event(adc, adc_chan, dir))
+ ret = palmas_gpadc_reconfigure_event_channels(adc);
+
+ mutex_unlock(&adc->lock);
+
+ return ret;
+}
+
static const struct iio_info palmas_gpadc_iio_info = {
.read_raw = palmas_gpadc_read_raw,
+ .read_event_config = palmas_gpadc_read_event_config,
+ .write_event_config = palmas_gpadc_write_event_config,
+ .read_event_value = palmas_gpadc_read_event_value,
+ .write_event_value = palmas_gpadc_write_event_value,
+};
+
+static const struct iio_event_spec palmas_gpadc_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE),
+ }, {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE),
+ },
};
#define PALMAS_ADC_CHAN_IIO(chan, _type, chan_info) \
@@ -448,6 +801,8 @@ static const struct iio_info palmas_gpadc_iio_info = {
BIT(chan_info), \
.indexed = 1, \
.channel = PALMAS_ADC_CH_##chan, \
+ .event_spec = palmas_gpadc_events, \
+ .num_event_specs = ARRAY_SIZE(palmas_gpadc_events) \
}
static const struct iio_chan_spec palmas_gpadc_iio_channel[] = {
@@ -554,6 +909,39 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
return dev_err_probe(adc->dev, ret,
"request irq %d failed\n", adc->irq);
+ adc->irq_auto_0 = platform_get_irq(pdev, 1);
+ if (adc->irq_auto_0 < 0)
+ return dev_err_probe(adc->dev, adc->irq_auto_0,
+ "get auto0 irq failed\n");
+
+ ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_0, NULL,
+ palmas_gpadc_irq_auto, IRQF_ONESHOT,
+ "palmas-adc-auto-0", indio_dev);
+ if (ret < 0)
+ return dev_err_probe(adc->dev, ret,
+ "request auto0 irq %d failed\n",
+ adc->irq_auto_0);
+
+ adc->irq_auto_1 = platform_get_irq(pdev, 2);
+ if (adc->irq_auto_1 < 0)
+ return dev_err_probe(adc->dev, adc->irq_auto_1,
+ "get auto1 irq failed\n");
+
+ ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_1, NULL,
+ palmas_gpadc_irq_auto, IRQF_ONESHOT,
+ "palmas-adc-auto-1", indio_dev);
+ if (ret < 0)
+ return dev_err_probe(adc->dev, ret,
+ "request auto1 irq %d failed\n",
+ adc->irq_auto_1);
+
+ adc->event0.enabled = false;
+ adc->event0.channel = -1;
+ adc->event0.direction = IIO_EV_DIR_NONE;
+ adc->event1.enabled = false;
+ adc->event1.channel = -1;
+ adc->event1.direction = IIO_EV_DIR_NONE;
+
/* set the current source 0 (value 0/5/15/20 uA => 0..3) */
if (gpadc_pdata->ch0_current <= 1)
adc->ch0_current = PALMAS_ADC_CH0_CURRENT_SRC_0;
@@ -631,13 +1019,14 @@ static int palmas_adc_configure_events(struct palmas_gpadc *adc)
int polarity;
ch0 = ev->channel;
- thres = ev->raw_thresh;
conv |= PALMAS_GPADC_AUTO_CTRL_AUTO_CONV0_EN;
switch (ev->direction) {
case IIO_EV_DIR_RISING:
+ thres = palmas_gpadc_get_high_threshold_raw(adc, ev);
polarity = 0;
break;
case IIO_EV_DIR_FALLING:
+ thres = palmas_gpadc_get_low_threshold_raw(adc, ev);
polarity = PALMAS_GPADC_THRES_CONV0_MSB_THRES_CONV0_POL;
break;
default:
@@ -667,13 +1056,14 @@ static int palmas_adc_configure_events(struct palmas_gpadc *adc)
int polarity;
ch1 = ev->channel;
- thres = ev->raw_thresh;
conv |= PALMAS_GPADC_AUTO_CTRL_AUTO_CONV1_EN;
switch (ev->direction) {
case IIO_EV_DIR_RISING:
+ thres = palmas_gpadc_get_high_threshold_raw(adc, ev);
polarity = 0;
break;
case IIO_EV_DIR_FALLING:
+ thres = palmas_gpadc_get_low_threshold_raw(adc, ev);
polarity = PALMAS_GPADC_THRES_CONV1_MSB_THRES_CONV1_POL;
break;
default:
--
2.25.1
The palmas gpadc block has support for monitoring up to 2 ADC channels
and issue an interrupt if they reach past a set threshold. This is
currently used to wake up the system from sleep, but the functionality
is more generic than that. As such, change the naming of functions and
variables to refer to it as events instead, except during suspend and
resume where wakeup still make sense.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 50 +++++++++++++++++-----------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index 943ac579eb1f..3ef0e1c08c8b 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -76,7 +76,7 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
PALMAS_ADC_INFO(IN15, 0, 0, 0, 0, INVALID, INVALID, true),
};
-struct palmas_adc_wakeup_property {
+struct palmas_adc_event {
int adc_channel_number;
int adc_high_threshold;
int adc_low_threshold;
@@ -117,10 +117,10 @@ struct palmas_gpadc {
int irq_auto_1;
struct palmas_gpadc_info *adc_info;
struct completion conv_completion;
- struct palmas_adc_wakeup_property wakeup1_data;
- struct palmas_adc_wakeup_property wakeup2_data;
- bool wakeup1_enable;
- bool wakeup2_enable;
+ struct palmas_adc_event event0;
+ struct palmas_adc_event event1;
+ bool event0_enable;
+ bool event1_enable;
int auto_conversion_period;
struct mutex lock;
};
@@ -590,7 +590,7 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
return 0;
}
-static int palmas_adc_wakeup_configure(struct palmas_gpadc *adc)
+static int palmas_adc_configure_events(struct palmas_gpadc *adc)
{
int adc_period, conv;
int i;
@@ -616,16 +616,16 @@ static int palmas_adc_wakeup_configure(struct palmas_gpadc *adc)
}
conv = 0;
- if (adc->wakeup1_enable) {
+ if (adc->event0_enable) {
int polarity;
- ch0 = adc->wakeup1_data.adc_channel_number;
+ ch0 = adc->event0.adc_channel_number;
conv |= PALMAS_GPADC_AUTO_CTRL_AUTO_CONV0_EN;
- if (adc->wakeup1_data.adc_high_threshold > 0) {
- thres = adc->wakeup1_data.adc_high_threshold;
+ if (adc->event0.adc_high_threshold > 0) {
+ thres = adc->event0.adc_high_threshold;
polarity = 0;
} else {
- thres = adc->wakeup1_data.adc_low_threshold;
+ thres = adc->event0.adc_low_threshold;
polarity = PALMAS_GPADC_THRES_CONV0_MSB_THRES_CONV0_POL;
}
@@ -647,16 +647,16 @@ static int palmas_adc_wakeup_configure(struct palmas_gpadc *adc)
}
}
- if (adc->wakeup2_enable) {
+ if (adc->event1_enable) {
int polarity;
- ch1 = adc->wakeup2_data.adc_channel_number;
+ ch1 = adc->event1.adc_channel_number;
conv |= PALMAS_GPADC_AUTO_CTRL_AUTO_CONV1_EN;
- if (adc->wakeup2_data.adc_high_threshold > 0) {
- thres = adc->wakeup2_data.adc_high_threshold;
+ if (adc->event1.adc_high_threshold > 0) {
+ thres = adc->event1.adc_high_threshold;
polarity = 0;
} else {
- thres = adc->wakeup2_data.adc_low_threshold;
+ thres = adc->event1.adc_low_threshold;
polarity = PALMAS_GPADC_THRES_CONV1_MSB_THRES_CONV1_POL;
}
@@ -695,7 +695,7 @@ static int palmas_adc_wakeup_configure(struct palmas_gpadc *adc)
return ret;
}
-static int palmas_adc_wakeup_reset(struct palmas_gpadc *adc)
+static int palmas_adc_reset_events(struct palmas_gpadc *adc)
{
int ret;
@@ -717,20 +717,20 @@ static int palmas_gpadc_suspend(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct palmas_gpadc *adc = iio_priv(indio_dev);
- int wakeup = adc->wakeup1_enable || adc->wakeup2_enable;
+ int wakeup = adc->event0_enable || adc->event1_enable;
int ret;
if (!device_may_wakeup(dev) || !wakeup)
return 0;
- ret = palmas_adc_wakeup_configure(adc);
+ ret = palmas_adc_configure_events(adc);
if (ret < 0)
return ret;
- if (adc->wakeup1_enable)
+ if (adc->event0_enable)
enable_irq_wake(adc->irq_auto_0);
- if (adc->wakeup2_enable)
+ if (adc->event1_enable)
enable_irq_wake(adc->irq_auto_1);
return 0;
@@ -740,20 +740,20 @@ static int palmas_gpadc_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct palmas_gpadc *adc = iio_priv(indio_dev);
- int wakeup = adc->wakeup1_enable || adc->wakeup2_enable;
+ int wakeup = adc->event0_enable || adc->event1_enable;
int ret;
if (!device_may_wakeup(dev) || !wakeup)
return 0;
- ret = palmas_adc_wakeup_reset(adc);
+ ret = palmas_adc_reset_events(adc);
if (ret < 0)
return ret;
- if (adc->wakeup1_enable)
+ if (adc->event0_enable)
disable_irq_wake(adc->irq_auto_0);
- if (adc->wakeup2_enable)
+ if (adc->event1_enable)
disable_irq_wake(adc->irq_auto_1);
return 0;
--
2.25.1
This prevents leaving the adc in freerunning mode when removing the
driver.
Signed-off-by: Patrik Dahlström <[email protected]>
---
drivers/iio/adc/palmas_gpadc.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
index f7eb445495b6..90fc20205df0 100644
--- a/drivers/iio/adc/palmas_gpadc.c
+++ b/drivers/iio/adc/palmas_gpadc.c
@@ -497,6 +497,13 @@ static int palmas_gpadc_get_adc_dt_data(struct platform_device *pdev,
return 0;
}
+static void palmas_gpadc_reset(void *data)
+{
+ struct palmas_gpadc *adc = data;
+ if (adc->event0.enabled || adc->event1.enabled)
+ palmas_adc_reset_events(adc);
+}
+
static int palmas_gpadc_probe(struct platform_device *pdev)
{
struct palmas_gpadc *adc;
@@ -586,6 +593,10 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
palmas_gpadc_calibrate(adc, i);
}
+ ret = devm_add_action(&pdev->dev, palmas_gpadc_reset, adc);
+ if (ret)
+ return ret;
+
return 0;
}
--
2.25.1
Hi Patrik,
> Am 02.04.2023 um 18:42 schrieb Patrik Dahlström <[email protected]>:
>
> It does not seem to be used by anyone and later patches in this series
> are made simpler by first removing this. There is now a lot of dead code
> that cannot be reached, until later patches revive it. Arguably, this is
> preferred over removing the code only to add it again.
>
> Signed-off-by: Patrik Dahlström <[email protected]>
> ---
> drivers/iio/adc/palmas_gpadc.c | 50 ++++------------------------------
> include/linux/mfd/palmas.h | 8 ------
> 2 files changed, 6 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
> index 24d7c096e4b8..943ac579eb1f 100644
> --- a/drivers/iio/adc/palmas_gpadc.c
> +++ b/drivers/iio/adc/palmas_gpadc.c
> @@ -76,6 +76,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
> PALMAS_ADC_INFO(IN15, 0, 0, 0, 0, INVALID, INVALID, true),
> };
>
> +struct palmas_adc_wakeup_property {
> + int adc_channel_number;
> + int adc_high_threshold;
> + int adc_low_threshold;
> +};
> +
> /*
> * struct palmas_gpadc - the palmas_gpadc structure
> * @ch0_current: channel 0 current source setting
> @@ -492,11 +498,6 @@ static int palmas_gpadc_get_adc_dt_data(struct platform_device *pdev,
> return 0;
> }
>
> -static void palmas_disable_wakeup(void *dev)
something seems to be mixed up here.
There is no palmas_disable_wakeup() upstream that can be removed. So this patch
can not be applied as 1/7 to any upstream kernel.
Please rebase your series on either linus/master or linux-next/master.
BR,
Nikolaus
> -{
> - device_wakeup_disable(dev);
> -}
> -
> static int palmas_gpadc_probe(struct platform_device *pdev)
> {
> struct palmas_gpadc *adc;
> @@ -547,36 +548,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> return dev_err_probe(adc->dev, ret,
> "request irq %d failed\n", adc->irq);
>
> - if (gpadc_pdata->adc_wakeup1_data) {
> - memcpy(&adc->wakeup1_data, gpadc_pdata->adc_wakeup1_data,
> - sizeof(adc->wakeup1_data));
> - adc->wakeup1_enable = true;
> - adc->irq_auto_0 = platform_get_irq(pdev, 1);
> - ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_0,
> - NULL, palmas_gpadc_irq_auto,
> - IRQF_ONESHOT,
> - "palmas-adc-auto-0", adc);
> - if (ret < 0)
> - return dev_err_probe(adc->dev, ret,
> - "request auto0 irq %d failed\n",
> - adc->irq_auto_0);
> - }
> -
> - if (gpadc_pdata->adc_wakeup2_data) {
> - memcpy(&adc->wakeup2_data, gpadc_pdata->adc_wakeup2_data,
> - sizeof(adc->wakeup2_data));
> - adc->wakeup2_enable = true;
> - adc->irq_auto_1 = platform_get_irq(pdev, 2);
> - ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_1,
> - NULL, palmas_gpadc_irq_auto,
> - IRQF_ONESHOT,
> - "palmas-adc-auto-1", adc);
> - if (ret < 0)
> - return dev_err_probe(adc->dev, ret,
> - "request auto1 irq %d failed\n",
> - adc->irq_auto_1);
> - }
> -
> /* set the current source 0 (value 0/5/15/20 uA => 0..3) */
> if (gpadc_pdata->ch0_current <= 1)
> adc->ch0_current = PALMAS_ADC_CH0_CURRENT_SRC_0;
> @@ -616,15 +587,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> palmas_gpadc_calibrate(adc, i);
> }
>
> - if (adc->wakeup1_enable || adc->wakeup2_enable) {
> - device_wakeup_enable(&pdev->dev);
> - ret = devm_add_action_or_reset(&pdev->dev,
> - palmas_disable_wakeup,
> - &pdev->dev);
> - if (ret)
> - return ret;
> - }
> -
> return 0;
> }
>
> diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> index 1e61c7e9f50d..55f22adb1a9e 100644
> --- a/include/linux/mfd/palmas.h
> +++ b/include/linux/mfd/palmas.h
> @@ -129,12 +129,6 @@ struct palmas_pmic_driver_data {
> struct regulator_config config);
> };
>
> -struct palmas_adc_wakeup_property {
> - int adc_channel_number;
> - int adc_high_threshold;
> - int adc_low_threshold;
> -};
> -
> struct palmas_gpadc_platform_data {
> /* Channel 3 current source is only enabled during conversion */
> int ch3_current; /* 0: off; 1: 10uA; 2: 400uA; 3: 800 uA */
> @@ -153,8 +147,6 @@ struct palmas_gpadc_platform_data {
> int start_polarity;
>
> int auto_conversion_period_ms;
> - struct palmas_adc_wakeup_property *adc_wakeup1_data;
> - struct palmas_adc_wakeup_property *adc_wakeup2_data;
> };
>
> struct palmas_reg_init {
> --
> 2.25.1
>
On Mon, Apr 03, 2023 at 06:49:36PM +0200, H. Nikolaus Schaller wrote:
> Hi Patrik,
>
> > Am 02.04.2023 um 18:42 schrieb Patrik Dahlstr?m <[email protected]>:
> >
> > It does not seem to be used by anyone and later patches in this series
> > are made simpler by first removing this. There is now a lot of dead code
> > that cannot be reached, until later patches revive it. Arguably, this is
> > preferred over removing the code only to add it again.
> >
> > Signed-off-by: Patrik Dahlstr?m <[email protected]>
> > ---
> > drivers/iio/adc/palmas_gpadc.c | 50 ++++------------------------------
> > include/linux/mfd/palmas.h | 8 ------
> > 2 files changed, 6 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
> > index 24d7c096e4b8..943ac579eb1f 100644
> > --- a/drivers/iio/adc/palmas_gpadc.c
> > +++ b/drivers/iio/adc/palmas_gpadc.c
> > @@ -76,6 +76,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
> > PALMAS_ADC_INFO(IN15, 0, 0, 0, 0, INVALID, INVALID, true),
> > };
> >
> > +struct palmas_adc_wakeup_property {
> > + int adc_channel_number;
> > + int adc_high_threshold;
> > + int adc_low_threshold;
> > +};
> > +
> > /*
> > * struct palmas_gpadc - the palmas_gpadc structure
> > * @ch0_current: channel 0 current source setting
> > @@ -492,11 +498,6 @@ static int palmas_gpadc_get_adc_dt_data(struct platform_device *pdev,
> > return 0;
> > }
> >
> > -static void palmas_disable_wakeup(void *dev)
>
> something seems to be mixed up here.
>
> There is no palmas_disable_wakeup() upstream that can be removed. So this patch
> can not be applied as 1/7 to any upstream kernel.
>
> Please rebase your series on either linus/master or linux-next/master.
I'm sorry for the confusion. I should have been more clear in the cover
letter.
This series is based on Jonathan Cameron's iio tree[1], plus the patches at
[2] and [3]. The first patch is already part of linux-next and I was under
the impression that [3] would be soon too.
Would it be best to rebase this series on linux-next instead?
[1] git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
[2] https://patchwork.kernel.org/project/linux-iio/patch/[email protected]/
[3] https://patchwork.kernel.org/project/linux-iio/patch/[email protected]/
>
> BR,
> Nikolaus
>
> > -{
> > - device_wakeup_disable(dev);
> > -}
> > -
> > static int palmas_gpadc_probe(struct platform_device *pdev)
> > {
> > struct palmas_gpadc *adc;
> > @@ -547,36 +548,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> > return dev_err_probe(adc->dev, ret,
> > "request irq %d failed\n", adc->irq);
> >
> > - if (gpadc_pdata->adc_wakeup1_data) {
> > - memcpy(&adc->wakeup1_data, gpadc_pdata->adc_wakeup1_data,
> > - sizeof(adc->wakeup1_data));
> > - adc->wakeup1_enable = true;
> > - adc->irq_auto_0 = platform_get_irq(pdev, 1);
> > - ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_0,
> > - NULL, palmas_gpadc_irq_auto,
> > - IRQF_ONESHOT,
> > - "palmas-adc-auto-0", adc);
> > - if (ret < 0)
> > - return dev_err_probe(adc->dev, ret,
> > - "request auto0 irq %d failed\n",
> > - adc->irq_auto_0);
> > - }
> > -
> > - if (gpadc_pdata->adc_wakeup2_data) {
> > - memcpy(&adc->wakeup2_data, gpadc_pdata->adc_wakeup2_data,
> > - sizeof(adc->wakeup2_data));
> > - adc->wakeup2_enable = true;
> > - adc->irq_auto_1 = platform_get_irq(pdev, 2);
> > - ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_1,
> > - NULL, palmas_gpadc_irq_auto,
> > - IRQF_ONESHOT,
> > - "palmas-adc-auto-1", adc);
> > - if (ret < 0)
> > - return dev_err_probe(adc->dev, ret,
> > - "request auto1 irq %d failed\n",
> > - adc->irq_auto_1);
> > - }
> > -
> > /* set the current source 0 (value 0/5/15/20 uA => 0..3) */
> > if (gpadc_pdata->ch0_current <= 1)
> > adc->ch0_current = PALMAS_ADC_CH0_CURRENT_SRC_0;
> > @@ -616,15 +587,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> > palmas_gpadc_calibrate(adc, i);
> > }
> >
> > - if (adc->wakeup1_enable || adc->wakeup2_enable) {
> > - device_wakeup_enable(&pdev->dev);
> > - ret = devm_add_action_or_reset(&pdev->dev,
> > - palmas_disable_wakeup,
> > - &pdev->dev);
> > - if (ret)
> > - return ret;
> > - }
> > -
> > return 0;
> > }
> >
> > diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> > index 1e61c7e9f50d..55f22adb1a9e 100644
> > --- a/include/linux/mfd/palmas.h
> > +++ b/include/linux/mfd/palmas.h
> > @@ -129,12 +129,6 @@ struct palmas_pmic_driver_data {
> > struct regulator_config config);
> > };
> >
> > -struct palmas_adc_wakeup_property {
> > - int adc_channel_number;
> > - int adc_high_threshold;
> > - int adc_low_threshold;
> > -};
> > -
> > struct palmas_gpadc_platform_data {
> > /* Channel 3 current source is only enabled during conversion */
> > int ch3_current; /* 0: off; 1: 10uA; 2: 400uA; 3: 800 uA */
> > @@ -153,8 +147,6 @@ struct palmas_gpadc_platform_data {
> > int start_polarity;
> >
> > int auto_conversion_period_ms;
> > - struct palmas_adc_wakeup_property *adc_wakeup1_data;
> > - struct palmas_adc_wakeup_property *adc_wakeup2_data;
> > };
> >
> > struct palmas_reg_init {
> > --
> > 2.25.1
> >
>
On Tue, 4 Apr 2023 12:33:28 +0200
Patrik Dahlström <[email protected]> wrote:
> On Mon, Apr 03, 2023 at 06:49:36PM +0200, H. Nikolaus Schaller wrote:
> > Hi Patrik,
> >
> > > Am 02.04.2023 um 18:42 schrieb Patrik Dahlström <[email protected]>:
> > >
> > > It does not seem to be used by anyone and later patches in this series
> > > are made simpler by first removing this. There is now a lot of dead code
> > > that cannot be reached, until later patches revive it. Arguably, this is
> > > preferred over removing the code only to add it again.
> > >
> > > Signed-off-by: Patrik Dahlström <[email protected]>
> > > ---
> > > drivers/iio/adc/palmas_gpadc.c | 50 ++++------------------------------
> > > include/linux/mfd/palmas.h | 8 ------
> > > 2 files changed, 6 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/palmas_gpadc.c b/drivers/iio/adc/palmas_gpadc.c
> > > index 24d7c096e4b8..943ac579eb1f 100644
> > > --- a/drivers/iio/adc/palmas_gpadc.c
> > > +++ b/drivers/iio/adc/palmas_gpadc.c
> > > @@ -76,6 +76,12 @@ static struct palmas_gpadc_info palmas_gpadc_info[] = {
> > > PALMAS_ADC_INFO(IN15, 0, 0, 0, 0, INVALID, INVALID, true),
> > > };
> > >
> > > +struct palmas_adc_wakeup_property {
> > > + int adc_channel_number;
> > > + int adc_high_threshold;
> > > + int adc_low_threshold;
> > > +};
> > > +
> > > /*
> > > * struct palmas_gpadc - the palmas_gpadc structure
> > > * @ch0_current: channel 0 current source setting
> > > @@ -492,11 +498,6 @@ static int palmas_gpadc_get_adc_dt_data(struct platform_device *pdev,
> > > return 0;
> > > }
> > >
> > > -static void palmas_disable_wakeup(void *dev)
> >
> > something seems to be mixed up here.
> >
> > There is no palmas_disable_wakeup() upstream that can be removed. So this patch
> > can not be applied as 1/7 to any upstream kernel.
> >
> > Please rebase your series on either linus/master or linux-next/master.
>
> I'm sorry for the confusion. I should have been more clear in the cover
> letter.
>
> This series is based on Jonathan Cameron's iio tree[1], plus the patches at
> [2] and [3]. The first patch is already part of linux-next and I was under
> the impression that [3] would be soon too.
>
> Would it be best to rebase this series on linux-next instead?
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> [2] https://patchwork.kernel.org/project/linux-iio/patch/[email protected]/
> [3] https://patchwork.kernel.org/project/linux-iio/patch/[email protected]/
>
This gets a bit complex because we have a mixture of fixes and updates and the desire
to end up with a fairly simple history in git.
Before [3] can be applied I need to move the togreg tree base to incorporate a newer
version of char-misc/char-misc-next (which is the upstream for IIO pull requests) and
the fix to have made the round trip to get there (which hasn't happened yet).
At that point I'll pick up the above and your series will be on top of that.
So you were fine to base this patch as you did, but you should make sure the
cover letter calls out the required patches that haven't yet made it upstream.
I generally don't mind simple cases of needing to rebase if things are based
on rc1 (don't every use linux-next as it is not a stable base) whilst applying but
in this case the devm change is sufficiently fiddly that you have two obvious choices.
1) Just add it to the start of your patch set (I'm fine with people doing that with
anything I post).
2) Listing the patches that should be applied first in the cover letter.
Jonathan
> >
> > BR,
> > Nikolaus
> >
> > > -{
> > > - device_wakeup_disable(dev);
> > > -}
> > > -
> > > static int palmas_gpadc_probe(struct platform_device *pdev)
> > > {
> > > struct palmas_gpadc *adc;
> > > @@ -547,36 +548,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> > > return dev_err_probe(adc->dev, ret,
> > > "request irq %d failed\n", adc->irq);
> > >
> > > - if (gpadc_pdata->adc_wakeup1_data) {
> > > - memcpy(&adc->wakeup1_data, gpadc_pdata->adc_wakeup1_data,
> > > - sizeof(adc->wakeup1_data));
> > > - adc->wakeup1_enable = true;
> > > - adc->irq_auto_0 = platform_get_irq(pdev, 1);
> > > - ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_0,
> > > - NULL, palmas_gpadc_irq_auto,
> > > - IRQF_ONESHOT,
> > > - "palmas-adc-auto-0", adc);
> > > - if (ret < 0)
> > > - return dev_err_probe(adc->dev, ret,
> > > - "request auto0 irq %d failed\n",
> > > - adc->irq_auto_0);
> > > - }
> > > -
> > > - if (gpadc_pdata->adc_wakeup2_data) {
> > > - memcpy(&adc->wakeup2_data, gpadc_pdata->adc_wakeup2_data,
> > > - sizeof(adc->wakeup2_data));
> > > - adc->wakeup2_enable = true;
> > > - adc->irq_auto_1 = platform_get_irq(pdev, 2);
> > > - ret = devm_request_threaded_irq(&pdev->dev, adc->irq_auto_1,
> > > - NULL, palmas_gpadc_irq_auto,
> > > - IRQF_ONESHOT,
> > > - "palmas-adc-auto-1", adc);
> > > - if (ret < 0)
> > > - return dev_err_probe(adc->dev, ret,
> > > - "request auto1 irq %d failed\n",
> > > - adc->irq_auto_1);
> > > - }
> > > -
> > > /* set the current source 0 (value 0/5/15/20 uA => 0..3) */
> > > if (gpadc_pdata->ch0_current <= 1)
> > > adc->ch0_current = PALMAS_ADC_CH0_CURRENT_SRC_0;
> > > @@ -616,15 +587,6 @@ static int palmas_gpadc_probe(struct platform_device *pdev)
> > > palmas_gpadc_calibrate(adc, i);
> > > }
> > >
> > > - if (adc->wakeup1_enable || adc->wakeup2_enable) {
> > > - device_wakeup_enable(&pdev->dev);
> > > - ret = devm_add_action_or_reset(&pdev->dev,
> > > - palmas_disable_wakeup,
> > > - &pdev->dev);
> > > - if (ret)
> > > - return ret;
> > > - }
> > > -
> > > return 0;
> > > }
> > >
> > > diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> > > index 1e61c7e9f50d..55f22adb1a9e 100644
> > > --- a/include/linux/mfd/palmas.h
> > > +++ b/include/linux/mfd/palmas.h
> > > @@ -129,12 +129,6 @@ struct palmas_pmic_driver_data {
> > > struct regulator_config config);
> > > };
> > >
> > > -struct palmas_adc_wakeup_property {
> > > - int adc_channel_number;
> > > - int adc_high_threshold;
> > > - int adc_low_threshold;
> > > -};
> > > -
> > > struct palmas_gpadc_platform_data {
> > > /* Channel 3 current source is only enabled during conversion */
> > > int ch3_current; /* 0: off; 1: 10uA; 2: 400uA; 3: 800 uA */
> > > @@ -153,8 +147,6 @@ struct palmas_gpadc_platform_data {
> > > int start_polarity;
> > >
> > > int auto_conversion_period_ms;
> > > - struct palmas_adc_wakeup_property *adc_wakeup1_data;
> > > - struct palmas_adc_wakeup_property *adc_wakeup2_data;
> > > };
> > >
> > > struct palmas_reg_init {
> > > --
> > > 2.25.1
> > >
> >